Skip to content
This repository has been archived by the owner on Feb 16, 2022. It is now read-only.

Recommended to use interface instead of struct in xxxManager #226

Closed
starsz opened this issue Jul 9, 2021 · 4 comments
Closed

Recommended to use interface instead of struct in xxxManager #226

starsz opened this issue Jul 9, 2021 · 4 comments
Labels
breaking-change enhancement New feature or request

Comments

@starsz
Copy link

starsz commented Jul 9, 2021

Description

Hello, I found that the xxxManager was defined as a struct.
And it was referenced in management.

type Management struct {
// Client manages Auth0 Client (also known as Application) resources.
Client *ClientManager
// ClientGrant manages Auth0 ClientGrant resources.
ClientGrant *ClientGrantManager
// ResourceServer manages Auth0 Resource Server (also known as API)
// resources.
ResourceServer *ResourceServerManager
// Connection manages Auth0 Connection resources.
Connection *ConnectionManager
// CustomDomain manages Auth0 Custom Domains.
CustomDomain *CustomDomainManager
// Grant manages Auth0 Grants.
Grant *GrantManager
// Log reads Auth0 Logs.
Log *LogManager

auth0/management/user.go

Lines 280 to 282 in 0ed82d2

type UserManager struct {
*Management
}

Affected Resources

  • management.Xxx
  • management.XxxManager

Potential Sample Code

I recommend to use interface instead of struct in xxxManager
Like this:

type UserManager interface{
 Create(u *User, opts ...RequestOption) error
 Read(id string, opts ...RequestOption) (u *User, err error)
 Update(id string, u *User, opts ...RequestOption) (err error) 
 List(opts ...RequestOption) (ul *UserList, err error) 
...
}

So that we can mock the Management.User and do some unit tests.

The same as other managers.

@starsz starsz added the enhancement New feature or request label Jul 9, 2021
@alexkappa
Copy link
Contributor

Hi @starsz, I find the proposal to adopt interfaces reasonable and your use case valid. Its a large undertaking however and a potentially backwards incompatible change (so a major version bump may be warranted).

Just to make sure you have explored the option, did you try to pass your own implementation of http.Client? Its not exactly the same, but you can mock the HTTP layer by providing a custom client.

@emmaLP
Copy link

emmaLP commented Jan 20, 2022

@alexkappa do you have an example where injecting a mock http client has been done for auth0 when unit testing?

@mutefiRe
Copy link

mutefiRe commented Feb 8, 2022

Just to make sure you have explored the option, did you try to pass your own implementation of http.Client? Its not exactly the same, but you can mock the HTTP layer by providing a custom client.

func WithClient(client *http.Client) ManagementOption {
	return func(m *Management) {
		m.http = client
	}
}

You can't pass an own implementation of http.Client either because the function uses the http.Client struct type instead of an interface around the Do method. Is there any other way to inject it?

@sergiught
Copy link
Collaborator

Closed and cloned this issue to auth0/go-auth0#17

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants