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

Singleton ApiContext with multi-thread code #103

Closed
penumbra23 opened this issue Jun 19, 2018 · 1 comment
Closed

Singleton ApiContext with multi-thread code #103

penumbra23 opened this issue Jun 19, 2018 · 1 comment

Comments

@penumbra23
Copy link

In some 0.13.x update, the BunqContext class was introduced, implementing the Singleton pattern on the ApiContext.

Every API call goes in some way like this:

var apiClient = new ApiClient(GetApiContext());   // GetApiContext() - fetches the Singleton instance
var response = apiClient.Get(...);

The potential issue (and the one I got now in my web project) is in multi-threaded and multi-user code where I need to make API calls with different ApiContext instances, i.e. different API keys.
One scenario is: user A loads the context, then user B loads his context, then user A makes a call with the context loaded from B (because it's a global static variable inside the SDK).

The workaround would be to lock my methods with a static objects, but then I have to do it for every method with the same Bunq API call and spaghetti code arises.

Why did you introduce the singleton ApiContext and left the earlier approach? How can I easily and in 'a more dev-friendly way' solve this issue?

@OGKevin
Copy link
Contributor

OGKevin commented Jun 20, 2018

@gpufreak The API was created with private/personal use in mind and not to create apps that can handle multiple API keys. Which means an app should always use 1 API key. Whit this in mind, bunq update 7 introduced a refactor that would make the SDK's more easy to use. Before you had to constantly create request bodies and pass the API context. With that update, we removed this.

Now with this in mind, the above issue was not considered.

So, to solve this you could either rethink your app/integration, don't use mutli-node or suggest a PR that fixes this and still keeps it simple.

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

2 participants