bchavez / Coinbase

:moneybag: A .NET/C# implementation of the Coinbase API.
https://developers.coinbase.com/api/v2
MIT License
170 stars 93 forks source link

Api Maintains state when reusing endpoint #35

Closed granthoff1107 closed 6 years ago

granthoff1107 commented 6 years ago

The Api Endpoint maintain the old URL Segments from past requests:

        var apiConfig = new ApiKeyConfig();
        apiConfig.ApiKey = apiKey;
        apiConfig.ApiSecret = apiSecret;

        var coinbaseApi = new CoinbaseApi(apiConfig);
        var apiAccounts = await coinbaseApi.Accounts.ListAccountsAsync();
        var apiAccount = apiAccounts.Data.FirstOrDefault(x => x.Name == "ETH Wallet");
        var apiAddresses = await coinbaseApi.Addresses.ListAddressesAsync(apiAccount.Id);
        var apiAddress = apiAddresses.Data.FirstOrDefault();
        var transactions = await coinbaseApi.Transactions.ListTransactionsAsync(apiAccount.Id);

In this senario transactions will 404 because Transactions internally uses the Accounts api,

     return this.AccountsEndpoint
        .AppendPathSegments(accountId, "transactions")
        .WithClient(this)
        .GetJsonAsync<PagedResponse<Transaction>>(cancellationToken);

AppendPathSegments will cause the internal endpoints to maintains the state, so the previous requests will have the old path segments. This seems like a larger architecture change but it probably could be solved by changing the Endpoint types from Url to Func

and then just initializing like so:

     this.AccountsEndpoint = () => this.config.ApiUrl.AppendPathSegment("accounts");
     this.PaymentMethodsEndpoint = () => this.config.ApiUrl.AppendPathSegment("payment-methods");
     //...
bchavez commented 6 years ago

Hi Grant,

My apologies. It was a total misunderstanding on my part. I thought each call to AppendPathSegment created a new URL object.

devenv_2344

But apparently, it creates a new URL object only from string and subsequent calls mutate the URL object if it already exists.

This should be fixed in v5.0.0-beta-4.

https://www.nuget.org/packages/Coinbase/5.0.0-beta-4

Thanks again for testing! Sorry for the frustration. :crying_cat_face: Hopefully I got it right this time. :)

Thanks, Brian

:hourglass_flowing_sand: :mag: "But I still haven't found what I'm for..."

granthoff1107 commented 6 years ago

No worries, they should have made their api stateless, i would have thought the same thing especially how they worded it.

I’m just glad at how quicker we’re making the progress on the new beta. I’ll put it some more testing tomorrow

On Wed, Nov 7, 2018 at 11:42 PM Brian Chavez notifications@github.com wrote:

Hi Grant,

My apologies. It was a total misunderstanding on my part. I thought each call to AppendPathSegment created a new URL object.

[image: devenv_2344] https://user-images.githubusercontent.com/478118/48178163-244d0300-e2cd-11e8-9c29-a2b0abb4f965.png

But apparently, it creates a new URL object only from string and mutates if an object already exists.

This should be fixed in v5.0.0-beta-4.

https://www.nuget.org/packages/Coinbase/5.0.0-beta-4

Thanks again for testing! Sorry for the frustration. 😿 Hopefully I got it right this time. :)

Thanks, Brian

⏳ 🔍 "But I still haven't found what I'm for..." https://www.youtube.com/watch?v=wdCJRybAtso

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bchavez/Coinbase/issues/35#issuecomment-436871653, or mute the thread https://github.com/notifications/unsubscribe-auth/AMsMnUOQQXWsiM4m_pDnwvMIRPTcCG6Nks5us7ZIgaJpZM4YT83J .