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

Fields on Client struct changed from interfaces to structs in 0.1.6 #171

Open
DWSR opened this issue Jan 12, 2025 · 2 comments
Open

Fields on Client struct changed from interfaces to structs in 0.1.6 #171

DWSR opened this issue Jan 12, 2025 · 2 comments
Labels
bug Something isn't working

Comments

@DWSR
Copy link

DWSR commented Jan 12, 2025

Scenario & Reproduction Steps

In this change (included in 0.1.6), the types of the fields on the Client struct were changed from interfaces to concrete types. This breaks any unit or integration tests which rely on providing mock implementations of the various fields in order to avoid calling a live 1Password service (related: #98).

As an example, I wrote a MockResolver so that I could test my code like this.

Given that this file is auto-generated, I expect this is due to a change in the generation code.

Actual Behavior

The fields are structs, not interfaces

Expected Behavior

The fields should be interfaces.

SDK version

0.1.6

Additional information

No response

@DWSR DWSR added the bug Something isn't working label Jan 12, 2025
@DWSR DWSR changed the title Fields on Client struct changed from interfaces to structs Fields on Client struct changed from interfaces to structs in 0.1.6 Jan 12, 2025
@hculea
Copy link
Member

hculea commented Jan 13, 2025

Hey @DWSR, thank you for bringing this up!

Making sure that our SDKs are testable is a priority for us. It looks like, in making this change, we did not consider the use-cases for custom implementations of the APIs within the SDKs.

Thanks for sharing your implementation of the MockResolver - we're going to look into making this switch back to the top-level fields of the client being injectable interfaces and we will keep you posted with our decisions and work on this.

In the meantime, given you only use one function within the SDK, would a workaround such as wrapping the entire 1Password client in an interface with multiple implementations in your code be suitable?

For example:

type SecretsStore interface {
  Resolve(context ctx.Context, reference string) (string, error)
}

type OnePasswordStore struct {
  client *onepassword.Client
}

func (o *OnePasswordStore) Resolve(context ctx.Context, reference string) (string, error) {
  return o.client.Secrets.Resolve(context, reference)
}

type MockStore struct {
  // your mocked fields
}

func (o *MockStore) Resolve(context ctx.Context, reference string) (string, error) {
  // your mock implementation
}

@DWSR
Copy link
Author

DWSR commented Feb 2, 2025

Yes, I was able to do what you suggested here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants