Nerzal / gocloak

golang keycloak client
Apache License 2.0
1.01k stars 275 forks source link

GoCloak is not an interface like documentation describe it #438

Open ymohl-cl opened 1 year ago

ymohl-cl commented 1 year ago

Is your feature request related to a problem? Please describe.

The go doc reference for gocloak package describe the client like a GoCloak interface. It seems to be a good approach to abstract the core Client.

The interface assume a succefull instantiate struct and a strong API Contract.

A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'm always frustrated when i manipulate the core struct instance with the tentation for changing the internal fields.

Whats is the motivation to expose the struct instead of Interface ?

Describe the solution you'd like

The change to an interface cause an error inside the options parameters for the NewClient method. The solution should to have an internal type option.

I purpose you to make this rollback change.

i didn't find any resources about this topics ;)

MaxBreida commented 1 year ago

The change has been done to adhere to the accept interfaces, return structs principle.

https://github.com/golang/go/wiki/CodeReviewComments#interfaces https://dave.cheney.net/2016/08/20/solid-go-design https://mycodesmells.com/post/accept-interfaces-return-struct-in-go

Although yes, maybe the readme should be updated to not get people confused on this

joshuai96 commented 11 months ago

I think the interface should be part of the libary and can adhere to the principal. Checks should be made that the implemented struct methods satisfy the interface. This would make mocking, for testing pruposes, easier, by having an "offical" and up to date interface to mock against.

WilSimpson commented 6 months ago

GoCloak being a struct breaks the accept interfaces, return structs principal. It requires passing GoCloak as a struct when using it.