Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GoCloak is not an interface like documentation describe it #438

Open
ymohl-cl opened this issue Jul 26, 2023 · 3 comments
Open

GoCloak is not an interface like documentation describe it #438

ymohl-cl opened this issue Jul 26, 2023 · 3 comments

Comments

@ymohl-cl
Copy link

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
Copy link

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
Copy link

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
Copy link

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

rcaught added a commit to rcaught/gocloak that referenced this issue Aug 15, 2024
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 15, 2024
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 16, 2024
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 16, 2024
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 18, 2024
Fixes Nerzal#438

Generates interface from struct and verify implementation
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 18, 2024
Fixes Nerzal#438

Generates interface from struct and verify implementation
rcaught added a commit to rcaught/gocloak that referenced this issue Aug 19, 2024
Related Nerzal#438

Generates interface from struct and verifies implementation. This doesn't modify `NewClient` as that requires a larger refactor.
Nerzal pushed a commit that referenced this issue Nov 5, 2024
* chore: update workflows and lint fixes

* chore: update golang version and dependencies

* feat: provide generated and verified interface

Related #438

Generates interface from struct and verifies implementation. This doesn't modify `NewClient` as that requires a larger refactor.

* test: fix message to match logic

* test: fix up codeQL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants