bchavez / Coinbase

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

Remove deprecated endpoints and enhanced transaction information by expanding `Buy` and `Sell` resources #89

Open astrohart opened 8 months ago

astrohart commented 8 months ago

Removed the following endpoints, as they are totally offline (calling them gives a HTTP 410 Gone response:

and their associated unit tests.

Update the List Transactions and Show Transaction methods in the Transactions endpoint, along with the Transaction model, to expand Buy and Sell resources inline.

astrohart commented 8 months ago

I can't get AppVeyor to work

astrohart commented 8 months ago

Hello @bchavez good morning. Thank you for allowing me to be a contributor to your code. I wanted to write to you about this pull request because I have two points I'd like to discuss:

Please note, I was trying to upgrade the version of .NET and the NuGet packages because using the existing versions was making Visual Studio throw all kinds of errors and warnings, given that various versions of .NET have been EOL'd since when you yourself last published this code and now. Partly was just a way to bring the code up to the "modern times," so I could more effectively help contribute.

To address point 1, AppVeyor:

I am thinking if you install the latest .NET Core 7 SDK on the AppVeyor instance (however that works) and upgrade FAKE and other libraries to the compatible versions, then AppVeyor should work. The code builds on my own machine just fine. But I can't be 100% sure, only because I've never really used AppVeyor.

To address point 2, lack of unit testing:

astrohart commented 8 months ago

@bchavez image Figure 1. Failing unit tests.

1046 hours MST on 01-05-2024.xml.txt File 1. Unit test results as an XML file.

As of 10:45 hours Mountain time on 01/05/2024, Figure 1 shows my current unit testing results. It appears I have a ways to go yet. I will keep this thread updated as I progress.

astrohart commented 8 months ago

The way the unit tests are written, they are not very useful to discovering the root cause of exceptions. I'd like to make more use of the NUnit methods to do the tests on a more fine-grained level by slightly re-writing them.

For instance, when running the debugger on the AccountTests.can_list_accounts test, I set a breakpoint at the opening curly brace. I then use Step Over repeatedly. When I get to the call of ListAccountsAsync, the debugger suddenly exits, reporting the unit test as failing with an exception.

I propose changing the unit tests in the following manner. First, I wish to update the body of the ServerTest.SetupServerPagedResponse method to print out the JSON that it's expecting to send back as a response, to the console, so I can see what I am expecting to get:

      protected void SetupServerPagedResponse(string pageJson, string dataJson)
      {
         var json = @"{
    ""pagination"": {pageJson},
    ""data"": [{dataJson}]
}
".Replace("{dataJson}", dataJson)
            .Replace("{pageJson}", pageJson);

         server.RespondWith(json);

         Console.WriteLine("Anticipated server response:");
         Console.WriteLine("---");
         Console.WriteLine();
         Console.WriteLine(json);
         Console.WriteLine();
         Console.WriteLine("---");
      }

Listing 1. The new implementation of the ServerTest.SetupServerPagedResponse method.

I then propose to adapt the unit-test method AccountTests.can_list_accounts to read as the following:

      [Test]
      public void can_list_accounts()
      {
         SetupServerPagedResponse(PaginationJson, $"{Account1},{Account2}");

         PagedResponse<Account> accounts = default;

         Assert.DoesNotThrowAsync(async () => accounts = await client.Accounts.ListAccountsAsync(new PaginationOptions { Limit = 25}));

         var truth = new PagedResponse<Account>
            {
               Pagination = PaginationModel,
               Data = new[] {Account1Model, Account2Model}
            };

         truth.Should().BeEquivalentTo(accounts);

         server.ShouldHaveCalled("https://api.coinbase.com/v2/accounts")
            .WithVerb(HttpMethod.Get);
      }

Listing 2. The new body of the can_list_accounts test. I am making use of the NUnit method, Assert.DoesNotThrowAsync, and am passing in a PaginationOptions to the ListAccountsAsync method, where before, there was no argument being passed.

The unit test is still failing. Results:


  Assert.That(code, new ThrowsNothingConstraint())
  Expected: No Exception to be thrown
  But was:  <Flurl.Http.FlurlParsingException: Response could not be deserialized to JSON: GET https://api.coinbase.com/v2/accounts?limit=25 ---> System.Text.Json.JsonException: The JSON value could not be converted to Coinbase.Models.SortOrder. Path: $.pagination.order | LineNumber: 6 | BytePositionInLine: 19.
   at System.Text.Json.ThrowHelper.ThrowJsonException(String message)
   at System.Text.Json.Serialization.Converters.EnumConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](JsonReaderState& readerState, Boolean isFinalBlock, ReadOnlySpan`1 buffer, JsonSerializerOptions options, ReadStack& state, JsonConverter converterBase)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.ReadAll[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at Flurl.Http.Configuration.DefaultJsonSerializer.Deserialize[T](Stream stream)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
   --- End of inner exception stack trace ---
   at Flurl.Http.FlurlClient.<HandleExceptionAsync>d__28.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.ResponseExtensions.<ReceiveJson>d__0`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Coinbase.Tests.Endpoints.AccountTests.<>c__DisplayClass0_0.<<can_list_accounts>b__0>d.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 20
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.ExceptionHelper.RecordException(Delegate parameterlessDelegate, String parameterName)>

   at Coinbase.Tests.Endpoints.AccountTests.can_list_accounts() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 20

Anticipated server response:
---

{
    "pagination": 
   {
    "ending_before": null,
    "starting_after": null,
    "limit": 25,
    "order": "desc",
    "previous_uri": null,
    "next_uri": "/v2/accounts?&limit=25&starting_after=5d5aed5f-b7c0-5585-a3dd-a7ed9ef0e414"
  },
    "data": [{
      "id": "58542935-67b5-56e1-a3f9-42686e07fa40",
      "name": "My Vault",
      "primary": false,
      "type": "vault",
      "currency": {"code":"BTC"},
      "balance": {
        "amount": "4.00000000",
        "currency": "BTC"
      },
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-01-31T20:49:02Z",
      "resource": "account",
      "resource_path": "/v2/accounts/58542935-67b5-56e1-a3f9-42686e07fa40",
      "ready": true
    },{
      "id": "2bbf394c-193b-5b2a-9155-3b4732659ede",
      "name": "My Wallet",
      "primary": true,
      "type": "wallet",
      "currency": {"code":"BTC"},
      "balance": {
        "amount": "39.59000000",
        "currency": "BTC"
      },
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-01-31T20:49:02Z",
      "resource": "account",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede"
    }]
}

---

Listing 3. Unit test results from a test conducted at approximately 11:16 hours MST on 01/05/2024.

It appears as if it's complaining because the order property of the pagination is not being supplied. So I will supply the default value, desc, which is also part of the anticipated server response as shown in the bottom of Listing 3, and see if that works. The unit test has been rewritten to read:

      [Test]
      public void can_list_accounts()
      {
         SetupServerPagedResponse(PaginationJson, $"{Account1},{Account2}");

         PagedResponse<Account> accounts = default;

         Assert.DoesNotThrowAsync(async () =>
            accounts = await client.Accounts.ListAccountsAsync(
               new PaginationOptions
               {
                  Limit = 25, Order = "desc"
               })
            );

         var truth = new PagedResponse<Account>
            {
               Pagination = PaginationModel,
               Data = new[] {Account1Model, Account2Model}
            };

         truth.Should().BeEquivalentTo(accounts);

         server.ShouldHaveCalled("https://api.coinbase.com/v2/accounts")
            .WithVerb(HttpMethod.Get);
      }

Listing 4. The new body of the unit test AccountTests.can_list_accounts.

I've added line breaks for clarity. The only change between this and Listing 2 is that I am passing desc for the PaginationOptions.Order property.

Oh! It's telling me that it cannot convert the value of order in the response to the SortOrder enum. Yes, I understand now. I actually encountered this while working on the code of a client for Coinbase Advanced Trade. I know the solve. According to a StackOverflow post I read (and forgive me, but I can't recall the link), I need to add some attributes to the Coinbase.Models.SortOrder enum members. It's not enough to just put [JsonConverter(typeof(StringEnumConverter))] on the Order property of Coinbase.Model.Pagination class. You also have to use the System.ComponentModel.DescriptionAttribute and System.Runtime.Serialization.EnumMemberAttribute on the enum members to tell the serializer what text values they correspond to, as in:

using System.Runtime.Serialization;
using System.ComponentModel;

 public enum SortOrder
 {
    [Description("desc"), EnumMember(Value = "desc")]
    Desc,

    [Description("asc"), EnumMember(Value = "asc")]
    Asc
 }

Listing 5. The SortOrder enum, with its members properly decorated.

Now let's run the unit test again. The results are:


  Assert.That(code, new ThrowsNothingConstraint())
  Expected: No Exception to be thrown
  But was:  <Flurl.Http.FlurlParsingException: Response could not be deserialized to JSON: GET https://api.coinbase.com/v2/accounts?limit=25&order=desc ---> System.Text.Json.JsonException: The JSON value could not be converted to Coinbase.Models.SortOrder. Path: $.pagination.order | LineNumber: 6 | BytePositionInLine: 19.
   at System.Text.Json.ThrowHelper.ThrowJsonException(String message)
   at System.Text.Json.Serialization.Converters.EnumConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](JsonReaderState& readerState, Boolean isFinalBlock, ReadOnlySpan`1 buffer, JsonSerializerOptions options, ReadStack& state, JsonConverter converterBase)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.ReadAll[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at Flurl.Http.Configuration.DefaultJsonSerializer.Deserialize[T](Stream stream)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
   --- End of inner exception stack trace ---
   at Flurl.Http.FlurlClient.<HandleExceptionAsync>d__28.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.ResponseExtensions.<ReceiveJson>d__0`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Coinbase.Tests.Endpoints.AccountTests.<>c__DisplayClass0_0.<<can_list_accounts>b__0>d.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 21
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.ExceptionHelper.RecordException(Delegate parameterlessDelegate, String parameterName)>

   at Coinbase.Tests.Endpoints.AccountTests.can_list_accounts() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 20

Anticipated server response:
---

{
    "pagination": 
   {
    "ending_before": null,
    "starting_after": null,
    "limit": 25,
    "order": "desc",
    "previous_uri": null,
    "next_uri": "/v2/accounts?&limit=25&starting_after=5d5aed5f-b7c0-5585-a3dd-a7ed9ef0e414"
  },
    "data": [{
      "id": "58542935-67b5-56e1-a3f9-42686e07fa40",
      "name": "My Vault",
      "primary": false,
      "type": "vault",
      "currency": {"code":"BTC"},
      "balance": {
        "amount": "4.00000000",
        "currency": "BTC"
      },
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-01-31T20:49:02Z",
      "resource": "account",
      "resource_path": "/v2/accounts/58542935-67b5-56e1-a3f9-42686e07fa40",
      "ready": true
    },{
      "id": "2bbf394c-193b-5b2a-9155-3b4732659ede",
      "name": "My Wallet",
      "primary": true,
      "type": "wallet",
      "currency": {"code":"BTC"},
      "balance": {
        "amount": "39.59000000",
        "currency": "BTC"
      },
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-01-31T20:49:02Z",
      "resource": "account",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede"
    }]
}

---

Listing 6. Results of the still-failing unit test.

One thing I did notice, in the Coinbase.Models.Pagination class, is that there is no JsonProperty attribute on the Order property to tell it that that is the property that corresponds to the property that is named order in the response JSON. Let me update the Coinbase.Models.Pagination class appropriately.

I am starting with:

public class Pagination : Json
{
    /* ... */

   [JsonConverter(typeof(StringEnumConverter))]
   public SortOrder Order { get; set; }

    /* ... */
}

Listing 7. The existing Coinbase.Models.Pagination.Order property.

and changing it to this:

public class Pagination : Json
{
    /* ... */

   [JsonProperty("order"), JsonConverter(typeof(StringEnumConverter))]
   public SortOrder Order { get; set; }

    /* ... */
}

Listing 8. The new version of the Coinbase.Models.Pagination.Order property.

Upon rebuilding everything and then running the unit test once again, I am obtaining exactly the same failure as shown in Listing 6. I am not sure why this is occurring. I will have to think some more.

astrohart commented 8 months ago

RE:

  1. https://stackoverflow.com/questions/57265440/problem-with-json-deserialization-of-html-string-using-json-net

See the StackOverflow link above. It's something that helped me.

Aha! Finally nailed it. It turns out, in your concrete CoinbaseClient class, the ListAccountsAsync method must have the async keyword applied to it, and not simply return a Task<PagedResponse<Account>> but be async Task<PagedResponse<Account>>; furthermore, it must have a new body:

/// <summary>
/// Lists current user’s accounts to which the authentication method has access to.
/// </summary>
async Task<PagedResponse<Account>> IAccountsEndpoint.ListAccountsAsync(
   PaginationOptions pagination,
   CancellationToken cancellationToken
   )
{
   var responseBody = await Request(
         AccountsEndpoint
            .WithPagination(pagination)
         )
      .GetStringAsync(cancellationToken: cancellationToken);
   if( string.IsNullOrWhiteSpace(responseBody) ) return new PagedResponse<Account>();

   return JsonConvert.DeserializeObject<PagedResponse<Account>>(responseBody);
}

Listing 1. The new implementation of the IAccountsEndpoint.ListAccountsAsync method.

Forgive the line breaks in Listing 1. They were put in for readability. I have not altered the line breaks in the original code.

This is bizarre, and the reason is, I am confused as to why we can't just call GetJson<...> on the response, but apparently, the proper way to do it is to call GetStringAsync on the Request and then check it for null or blank. If so, then return a PagedResponse<Account>() object.


NOTE: I personally, don't like returning null from methods on failure if I can help it because then it breaks calling code. Instead, I think it's better to just new up a new PagedResponse<Account> that represents the empty response. This way, folks who iterate through it will find zero elements in the response. Salt and pepper to taste, but I propose to leave this change in.


If our string response is not blank, then deserialize it by hand using Newtonsoft.

This approach caused the unit test to pass. I propose making similar changes throughout the software.

astrohart commented 8 months ago

UPDATE

I thought it would be a good idea to update the AccountTests.can_list_account unit tests to run some additional Asserts on the data returned by the ListAccountsAsync method; namely:

  1. Assert that the accounts variable is not null
  2. Assert that the accounts.Data property has a Length of 2

The changes:

public void can_list_accounts()
{
   /* ... */

   Assert.DoesNotThrowAsync(...);

   Assert.That(accounts, Is.Not.Null);

   Assert.That(accounts.Data.Length == 2);

   /* ... */
}

Listing 1. Updates to the AccountTests.can_list_account unit test.

Given these new Asserts, the unit test still passes.

astrohart commented 8 months ago

At 11:58 hours MST on 01/05/2024, I got the AccountTests.get_an_account to pass (formerly failing) without changing the body of the test itself. However, I did update the body of the CoinbaseClient.GetAccountAsync method to use the new "get a string response, and then deserialize it" technique:

/// <summary>
/// Show current user’s account. To access the primary account for a given currency, a currency string (BTC or ETH) can be used instead of the account id in the URL.
/// </summary>
async Task<Response<Account>> IAccountsEndpoint.GetAccountAsync(string accountId, CancellationToken cancellationToken)
{
   var responseBody = await Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId)).GetStringAsync(cancellationToken: cancellationToken);
   if (string.IsNullOrWhiteSpace(responseBody))
      return new Response<Account>();

   return JsonConvert.DeserializeObject<Response<Account>>(responseBody);
}

Listing 1. The new implementation of the CoinbaseClient.GetAccountAsync.

I propose that this change be adopted. It'll be included in my next PR commit.

astrohart commented 8 months ago

Moving along to making the AccountTests.can_set_account_as_primary unit test pass, there were two changes I had to make to get it to work.

First, if the operation does not succeed, I propose we return the result of calling CoinbaseClient.GetAccountAsync() on the input to CoinbaseClient.SetAccountAsPrimaryAsync() so that, at least, the caller has valid account data (even if it could not be set as primary).

To do this, I had to change the signature of GetAccountAsync from an explicit implementation, i.e.:

async Task<Response<Account>> IAccountsEndpoint.GetAccountAsync(string accountId, CancellationToken cancellationToken)

to

public async Task<Response<Account>> GetAccountAsync(string accountId, CancellationToken cancellationToken)

This is because, apparently, C# does not allow one explicitly-implemented interface member to call another, without the explicit implementation being removed and the called method being made public. This does not change how a caller sees the method from outside the CoinbaseClient class; rather, it simply allows me to do the following:

      /// <summary>
      /// Promote an account as primary account.
      /// </summary>
      async Task<Response<Account>> IAccountsEndpoint.SetAccountAsPrimaryAsync(string accountId, CancellationToken cancellationToken)
      {
         using var response = Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId, "primary"))
            .PostJsonAsync(null, cancellationToken: cancellationToken);
         var responseBody = await response.ReceiveString();
         if( string.IsNullOrWhiteSpace(responseBody) )
            return await GetAccountAsync(accountId, cancellationToken);

         return JsonConvert.DeserializeObject<Response<Account>>(responseBody);
      }

Listing 1. The new version of the CoinbaseClient.SetAccountAsPrimaryAsync method that passes its unit test.

Note the line if (string.IsNullOrWhiteSpace(responseBody)) return await GetAccountAsync(accountId, cancellationToken). I put this here so that, in case the call does not work for some reason, the method is idempotent (i.e., the caller gets back the Account whose accountId they supplied corresponds to. At least then, caller code won't break.

The XML documentation should, IMHO, be updated to remark about that:

      /// <remarks>If the operation does not work, a <c>Response&lt;Account&gt;</c>
      /// reference is returned having the account data, but it is not set to
      /// <c>primary</c>.</remarks>

Listing 2. XML documentation to be added to the method.

astrohart commented 8 months ago

Now, on to making the AccountTests.can_update_account unit test pass. I suspect, what I have to do is alter the code of the CoinbaseClient.UpdateAccountAsync method to be in a similar vein to the above.

Upon running the failing unit test, I am getting the following result:


Flurl.Http.FlurlParsingException : Response could not be deserialized to JSON: POST https://api.coinbase.com/v2/accounts/82de7fcd-db72-5085-8ceb-bee19303080b
  ----> System.Text.Json.JsonException : The JSON value could not be converted to System.Decimal. Path: $.data.balance.amount | LineNumber: 8 | BytePositionInLine: 28.
  ----> System.InvalidOperationException : Cannot get the value of a token type 'String' as a number.
   at Flurl.Http.FlurlClient.<HandleExceptionAsync>d__28.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.ResponseExtensions.<ReceiveJson>d__0`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Coinbase.Tests.Endpoints.AccountTests.<can_update_account>d__3.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 95
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)
--JsonException
   at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](JsonReaderState& readerState, Boolean isFinalBlock, ReadOnlySpan`1 buffer, JsonSerializerOptions options, ReadStack& state, JsonConverter converterBase)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.ReadAll[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at Flurl.Http.Configuration.DefaultJsonSerializer.Deserialize[T](Stream stream)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
--InvalidOperationException
   at System.Text.Json.Utf8JsonReader.TryGetDecimal(Decimal& value)
   at System.Text.Json.Utf8JsonReader.GetDecimal()
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)

Anticipated server response:
---

{
    "data": {
    "id": "82de7fcd-db72-5085-8ceb-bee19303080b",
    "name": "New account name",
    "primary": true,
    "type": "wallet",
    "currency": {"code":"BTC"},
    "balance": {
      "amount": "0.00000000",
      "currency": "BTC"
    },
    "created_at": "2015-03-31T15:21:58-07:00",
    "updated_at": "2015-03-31T15:21:58-07:00",
    "resource": "account",
    "resource_path": "/v2/accounts/82de7fcd-db72-5085-8ceb-bee19303080b"
  }
}

---

Listing 1. Results of the failing unit test AccountTests.can_update_account.

The current body of the CoinbaseClient.UpdateAccountAsync method is as follows:

/// <summary>
/// Modifies user’s account.
/// </summary>
Task<Response<Account>> IAccountsEndpoint.UpdateAccountAsync(string accountId, UpdateAccount updateAccount, CancellationToken cancellationToken)
{
   return Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId))
          .PostJsonAsync(updateAccount, cancellationToken: cancellationToken)
          .ReceiveJson<Response<Account>>();
}

Listing 2. The current body of the CoinbaseClient.UpdateAccountAsync method.

I propose changing the method to be:

/// <summary>
/// Modifies user’s account.
/// </summary>
/// <remarks>Callers must check the result of this method to see
/// whether the update actually occurred, as this method returns
/// an instance of the <c>Account</c> model corresponding to the
/// specified <paramref name="accountId"/> regardless of whether
/// the update succeeded.</remarks>
async Task<Response<Account>> IAccountsEndpoint.UpdateAccountAsync(string accountId, UpdateAccount updateAccount, CancellationToken cancellationToken)
{
   using var response = Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId))
      .PostJsonAsync(updateAccount, cancellationToken: cancellationToken);
   var responseBody = await response.ReceiveString();
   if( string.IsNullOrWhiteSpace(responseBody) )
      return await GetAccountAsync(accountId, cancellationToken);

   return JsonConvert.DeserializeObject<Response<Account>>(responseBody);
}

Listing 3. The new body of the CoinbaseClient.UpdateAccountAsync method.

The unit test failed. I got the message:


Flurl.Http.Testing.HttpTestException : Expected any calls to be made with URL pattern https://api.coinbase.com/v2/accounts/82de7fcd-db72-5085-8ceb-bee19303080b and verb PUT, but no matching calls were made.
   at Flurl.Http.Testing.HttpCallAssertion.Assert(Nullable`1 count)
   at Flurl.Http.Testing.HttpCallAssertion.With(Func`2 match, String descrip)
   at Coinbase.Tests.Endpoints.AccountTests.<can_update_account>d__3.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AccountTests.cs:line 105
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

Anticipated server response:
---

{
    "data": {
    "id": "82de7fcd-db72-5085-8ceb-bee19303080b",
    "name": "New account name",
    "primary": true,
    "type": "wallet",
    "currency": {"code":"BTC"},
    "balance": {
      "amount": "0.00000000",
      "currency": "BTC"
    },
    "created_at": "2015-03-31T15:21:58-07:00",
    "updated_at": "2015-03-31T15:21:58-07:00",
    "resource": "account",
    "resource_path": "/v2/accounts/82de7fcd-db72-5085-8ceb-bee19303080b"
  }
}

---

Listing 4. Message from the failing unit test.

I thought maybe I need to use PutJsonAsync instead of PostJsonAsync:

/// <summary>
/// Modifies user’s account.
/// </summary>
/// <remarks>Callers must check the result of this method to see
/// whether the update actually occurred, as this method returns
/// an instance of the <c>Account</c> model corresponding to the
/// specified <paramref name="accountId"/> regardless of whether
/// the update succeeded.</remarks>
async Task<Response<Account>> IAccountsEndpoint.UpdateAccountAsync(string accountId, UpdateAccount updateAccount, CancellationToken cancellationToken)
{
   using var response = Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId))
      .PutJsonAsync(updateAccount, cancellationToken: cancellationToken);
   var responseBody = await response.ReceiveString();
   if( string.IsNullOrWhiteSpace(responseBody) )
      return await GetAccountAsync(accountId, cancellationToken);

   return JsonConvert.DeserializeObject<Response<Account>>(responseBody);
}

Listing 5. Variant of the CoinbaseClient.UpdateAccountAsync using the PutJsonAsync Flurl method.

The unit test passes.

astrohart commented 8 months ago

Commit: a7988dc3ebad09089e2f5b4331270887e14ae3f5

image Figure 1. All AccountTests now pass for both .net 4.62 and .net 7.0.

The commit with the SHA listed above has been pushed to this PR. It consists of only those changes that made the AccountTests all pass when none of them were before.

astrohart commented 8 months ago

Commit: fa9ef05d2931c4876bca97603931ba8e8a278c1e

image Figure 1. None of the AddressTests pass.

Now, I note that none of the AddressTests pass. Let's look at them one by one. I will condense my work log into one post (this post).


Unit Test: AddressTests.can_list_addresses


At 12:38 hours MST on 01/05/2024, the test fails. The failure message is as follows:

Flurl.Http.FlurlParsingException : Response could not be deserialized to JSON: GET https://api.coinbase.com/v2/accounts/ffff/addresses
  ----> System.Text.Json.JsonException : The JSON value could not be converted to Coinbase.Models.SortOrder. Path: $.pagination.order | LineNumber: 6 | BytePositionInLine: 19.
   at Flurl.Http.FlurlClient.<HandleExceptionAsync>d__28.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.ResponseExtensions.<ReceiveJson>d__0`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at Coinbase.Tests.Endpoints.AddressTests.<can_list_addresses>d__0.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AddressTests.cs:line 17
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)
--JsonException
   at System.Text.Json.ThrowHelper.ThrowJsonException(String message)
   at System.Text.Json.Serialization.Converters.EnumConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](JsonReaderState& readerState, Boolean isFinalBlock, ReadOnlySpan`1 buffer, JsonSerializerOptions options, ReadStack& state, JsonConverter converterBase)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.ReadAll[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at Flurl.Http.Configuration.DefaultJsonSerializer.Deserialize[T](Stream stream)
   at Flurl.Http.FlurlResponse.<GetJsonAsync>d__18`1.MoveNext()

Anticipated server response:
---

{
    "pagination": 
   {
    "ending_before": null,
    "starting_after": null,
    "limit": 25,
    "order": "desc",
    "previous_uri": null,
    "next_uri": "/v2/accounts?&limit=25&starting_after=5d5aed5f-b7c0-5585-a3dd-a7ed9ef0e414"
  },
    "data": [{
      "id": "dd3183eb-af1d-5f5d-a90d-cbff946435ff",
      "address": "mswUGcPHp1YnkLCgF1TtoryqSc5E9Q8xFa",
      "name": null,
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-03-31T17:25:29-07:00",
      "network": "bitcoin",
      "resource": "address",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/dd3183eb-af1d-5f5d-a90d-cbff946435ff"
    },{
      "id": "ac5c5f15-0b1d-54f5-8912-fecbf66c2a64",
      "address": "mgSvu1z1amUFAPkB4cUg8ujaDxKAfZBt5Q",
      "name": null,
      "created_at": "2015-03-31T17:23:52-07:00",
      "updated_at": "2015-01-31T20:49:02Z",
      "network": "bitcoin",
      "resource": "address",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/ac5c5f15-0b1d-54f5-8912-fecbf66c2a64"
    }]
}

---

Listing 1. Result of the failing AddressTests.can_list_addresses test.

I am going to access the implementation of the IAddressesEndpoint.ListAddressesAsync method, which is currently:

/// <inheritdoc />
Task<PagedResponse<AddressEntity>> IAddressesEndpoint.ListAddressesAsync(string accountId, PaginationOptions pagination, CancellationToken cancellationToken)
{
   return Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId, "addresses")
            .WithPagination(pagination))
         .GetJsonAsync<PagedResponse<AddressEntity>>(cancellationToken: cancellationToken);
}

Listing 2. Current implementation of the IAddressesEndpoint.ListAddressesAsync method.

I have a hunch that I need to change it in a manner very similar to what I did with the Accounts endpoint methods. The new implementation of the method is as follows:

/// <inheritdoc />   
async Task<PagedResponse<AddressEntity>> IAddressesEndpoint.ListAddressesAsync(string accountId, PaginationOptions pagination, CancellationToken cancellationToken)
{
   var responseBody = await Request(
      AccountsEndpoint.AppendPathSegmentsRequire(accountId, "addresses")
                      .WithPagination(pagination)
   ).GetStringAsync(cancellationToken: cancellationToken);
   if( string.IsNullOrWhiteSpace(responseBody) ) return new PagedResponse<AddressEntity>();

   return JsonConvert.DeserializeObject<PagedResponse<AddressEntity>>(responseBody);
}

Listing 3. New implementation of the IAddressesEndpoint.ListAddressesAsync method.

Indeed, the unit test now passes.


Unit Test: AddressTests.get_an_address


The failing message for the AddressTests.get_an_address unit test is the following:


Expected property truth.Data.CreatedAt to be <0001-01-01 00:00:00.000>, but found <2015-01-31 20:49:02 +0h>.
Expected property truth.Data.UpdatedAt to be <0001-01-01 00:00:00.000>, but found <2015-03-31 17:25:29 -7h>.
Expected property truth.Data.ResourcePath to be <null>, but found "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/dd3183eb-af1d-5f5d-a90d-cbff946435ff".

With configuration:
- Use declared types and members
- Compare enums by value
- Compare tuples by their properties
- Compare anonymous types by their properties
- Compare records by their members
- Include non-browsable members
- Match member by name (or throw)
- Be strict about the order of items in byte arrays
- Without automatic conversion.

   at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)
   at FluentAssertions.Execution.CollectingAssertionStrategy.ThrowIfAny(IDictionary`2 context)
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(Comparands comparands, EquivalencyValidationContext context)
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, Func`2 config, String because, Object[] becauseArgs)
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, String because, Object[] becauseArgs)
   at Coinbase.Tests.Endpoints.AddressTests.<get_an_address>d__1.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AddressTests.cs:line 43
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

Anticipated server response:
---

{
    "data": {
      "id": "dd3183eb-af1d-5f5d-a90d-cbff946435ff",
      "address": "mswUGcPHp1YnkLCgF1TtoryqSc5E9Q8xFa",
      "name": null,
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-03-31T17:25:29-07:00",
      "network": "bitcoin",
      "resource": "address",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/dd3183eb-af1d-5f5d-a90d-cbff946435ff"
    }
}

---

Listing 4. Failure message for the AddressTests.get_an_address unit test.

The current implementation of the IAddressesEndpoint.GetAddressAsync method is:

Task<Response<AddressEntity>> IAddressesEndpoint.GetAddressAsync(string accountId, string addressId, CancellationToken cancellationToken)
{
   return Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId, "addresses", addressId))
      .GetJsonAsync<Response<AddressEntity>>(cancellationToken: cancellationToken);
}

Listing 5. The current implementation of the method IAddressesEndpoint.GetAddressAsync.

I propose to change it thus:

/// <inheritdoc />
async Task<Response<AddressEntity>> IAddressesEndpoint.GetAddressAsync(string accountId, string addressId, CancellationToken cancellationToken)
{
   var responseBody = await Request(
      AccountsEndpoint.AppendPathSegmentsRequire(accountId, "addresses", addressId)
      ).GetStringAsync(cancellationToken: cancellationToken);
   if( string.IsNullOrWhiteSpace(responseBody) ) return new Response<AddressEntity>();

   return JsonConvert.DeserializeObject<Response<AddressEntity>>(responseBody);
}

Listing 6. The proposed new implementation of the IAddressesEndpoint.GetAddressAsync method.

The unit test passes with the new implementation.


Unit Test: AddressTests.can_list_address_transactions


Currently, this unit test fails. The message I receive when it fails is the following:


Flurl.Http.FlurlParsingException : Response could not be deserialized to JSON: GET https://api.coinbase.com/v2/accounts/fff/addresses/uuu/transactions
  ----> System.Text.Json.JsonException : The JSON value could not be converted to Coinbase.Models.SortOrder. Path: $.pagination.order | LineNumber: 6 | BytePositionInLine: 19.
   at Flurl.Http.FlurlClient.HandleExceptionAsync(FlurlCall call, Exception ex, CancellationToken token)
   at Flurl.Http.FlurlResponse.GetJsonAsync[T]()
   at Flurl.Http.ResponseExtensions.ReceiveJson[T](Task`1 response)
   at Coinbase.Tests.Endpoints.AddressTests.can_list_address_transactions() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AddressTests.cs:line 54
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)
--JsonException
   at System.Text.Json.ThrowHelper.ThrowJsonException(String message)
   at System.Text.Json.Serialization.Converters.EnumConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.ReadFromStream[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at Flurl.Http.Configuration.DefaultJsonSerializer.Deserialize[T](Stream stream)
   at Flurl.Http.FlurlResponse.GetJsonAsync[T]()

Anticipated server response:
---

{
    "pagination": 
   {
    "ending_before": null,
    "starting_after": null,
    "limit": 25,
    "order": "desc",
    "previous_uri": null,
    "next_uri": "/v2/accounts?&limit=25&starting_after=5d5aed5f-b7c0-5585-a3dd-a7ed9ef0e414"
  },
    "data": [{
      "id": "57ffb4ae-0c59-5430-bcd3-3f98f797a66c",
      "type": "send",
      "status": "completed",
      "amount": {
        "amount": "0.00100000",
        "currency": "BTC"
      },
      "native_amount": {
        "amount": "0.01",
        "currency": "USD"
      },
      "description": null,
      "created_at": "2015-03-11T13:13:35-07:00",
      "updated_at": "2015-03-26T15:55:43-07:00",
      "resource": "transaction",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/transactions/57ffb4ae-0c59-5430-bcd3-3f98f797a66c",
      "network": {
        "status": "off_blockchain",
        "name": "bitcoin"
      },
      "from": {
        "id": "a6b4c2df-a62c-5d68-822a-dd4e2102e703",
        "resource": "user"
      },
      "instant_exchange": false
    }]
}

---

Listing 7. Failure message for the AddressTests.can_list_address_transactions unit test.

I suspect that I simply have to adapt the implementation of the IAddressesEndpoint.ListAddressTransactionsAsync method in a similar fashion as I have been doing above. Let me try that.


NOTE: I have been having a discussion on this Flurl issue with the maintainers/contributors of that library. Apparently, Flurl likes to use System.Text.Json whereas we're using Newtonsoft. I propose we retain Newtonsoft as it is a well-known library and also has worked in the past, and simply proceed to adapt all our code to this new approach of receiving the response string first, and then deserializing with Newtonsoft instead of the built-in Flurl deserialization algorithm. That, or overhaul this library completely to use System.Text.Json instead, which I am not really willing to do (sounds like more work) at this juncture.


The current implementation of the IAddressesEndpoint.ListAddressTransactionsAsync method is as follows:

/// <inheritdoc />
Task<PagedResponse<Transaction>> IAddressesEndpoint.ListAddressTransactionsAsync(string accountId, string addressId, PaginationOptions pagination, CancellationToken cancellationToken)
{
   return Request(AccountsEndpoint
                  .AppendPathSegmentsRequire(accountId, "addresses", addressId, "transactions")
                  .WithPagination(pagination)
   ).GetJsonAsync<PagedResponse<Transaction>>(cancellationToken: cancellationToken);
}

Listing 8. The current implementation of the IAddressesEndpoint.ListAddressTransactionsAsync method.

I am now going to change it to the following:

/// <inheritdoc />
async Task<PagedResponse<Transaction>> IAddressesEndpoint.ListAddressTransactionsAsync(string accountId, string addressId, PaginationOptions pagination, CancellationToken cancellationToken)
{
   var responseBody = await Request(
      AccountsEndpoint
         .AppendPathSegmentsRequire(accountId, "addresses", addressId, "transactions")
         .WithPagination(pagination)
   ).GetStringAsync(cancellationToken: cancellationToken);
   if( string.IsNullOrWhiteSpace(responseBody) ) return new PagedResponse<Transaction>();

   return JsonConvert.DeserializeObject<PagedResponse<Transaction>>(responseBody);
}

Listing 9. The updated implementation of the IAddressesEndpoint.ListAddressTransactionsAsync method.

Upon running the unit test, AddressTests.can_list_address_transactions, with the new changes, the unit test passes.


Unit Test: AddressTests.can_create_address


Currently, as of 13:03 hours MST on 01/05/2024, the unit test is failing. The failure message is:


Expected property truth.Data.CreatedAt to be <0001-01-01 00:00:00.000>, but found <2015-01-31 20:49:02 +0h>.
Expected property truth.Data.UpdatedAt to be <0001-01-01 00:00:00.000>, but found <2015-03-31 17:25:29 -7h>.
Expected property truth.Data.ResourcePath to be <null>, but found "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/dd3183eb-af1d-5f5d-a90d-cbff946435ff".

With configuration:
- Use declared types and members
- Compare enums by value
- Compare tuples by their properties
- Compare anonymous types by their properties
- Compare records by their members
- Include non-browsable members
- Match member by name (or throw)
- Be strict about the order of items in byte arrays
- Without automatic conversion.

   at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)
   at FluentAssertions.Execution.CollectingAssertionStrategy.ThrowIfAny(IDictionary`2 context)
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(Comparands comparands, EquivalencyValidationContext context)
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, Func`2 config, String because, Object[] becauseArgs)
   at FluentAssertions.Primitives.ObjectAssertions`2.BeEquivalentTo[TExpectation](TExpectation expectation, String because, Object[] becauseArgs)
   at Coinbase.Tests.Endpoints.AddressTests.<can_create_address>d__3.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\AddressTests.cs:line 83
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

Anticipated server response:
---

{
    "data": {
      "id": "dd3183eb-af1d-5f5d-a90d-cbff946435ff",
      "address": "mswUGcPHp1YnkLCgF1TtoryqSc5E9Q8xFa",
      "name": "ddd",
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-03-31T17:25:29-07:00",
      "network": "bitcoin",
      "resource": "address",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/addresses/dd3183eb-af1d-5f5d-a90d-cbff946435ff"
    }
}

---

Listing 10. Failure message for the AddressTests.can_create_address unit test.

The current implementation of the IAddressesEndpoint.CreateAddressAsync method is as follows:

/// <inheritdoc />
Task<Response<AddressEntity>> IAddressesEndpoint.CreateAddressAsync(string accountId, CreateAddress createAddress, CancellationToken cancellationToken)
{
   return Request(AccountsEndpoint
                  .AppendPathSegmentsRequire(accountId, "addresses")
                  ).PostJsonAsync(
                     createAddress, cancellationToken: cancellationToken
                  ).ReceiveJson<Response<AddressEntity>>();
}

Listing 11. Current implementation of the IAddressesEndpoint.CreateAddressAsync method.

The proposed new implementation of the IAddressesEndpoint.CreateAddressAsync method is as follows:

/// <inheritdoc />
async Task<Response<AddressEntity>> IAddressesEndpoint.CreateAddressAsync(string accountId, CreateAddress createAddress, CancellationToken cancellationToken)
{
   using var response = Request(AccountsEndpoint.AppendPathSegmentsRequire(accountId, "addresses"))
      .PostJsonAsync(createAddress, cancellationToken: cancellationToken);
   var responseBody = await response.ReceiveString();
   if( string.IsNullOrWhiteSpace(responseBody) )
      return new Response<AddressEntity>();

   return JsonConvert.DeserializeObject<Response<AddressEntity>>(responseBody);
}

Listing 12. The proposed new implementation of the IAddressesEndpoint.CreateAddressAsync method.

With this change, the AddressTests.can_create_address unit test now passes.

astrohart commented 8 months ago

image Figure 1. All the AddressTests now pass.

As of 13:16 hours MST on 01/05/2024, the AddressTests unit tests all pass, as is shown in Figure 1.

Now, I will move on to get the DepositTests to work, which are all currently failing.

To save time and comment space, I will refrain from describing my efforts in detail from this point, as I have a pretty big hunch that I simply need to make method changes such as previously.

I will, however, log unit-test fixing approaches that appear to have a different root cause besides System.Text.Json vs. Newtonsoft.

astrohart commented 8 months ago

Commit: 5360afd790a03ab1d1964b5c7cdf86d985f611e0

At 13:31 hours MST on 01/05/2024, I was trying really hard to get the DepositTests.can_depositfunds unit test to work. I did my usual changes, adapting the method IDepositsEndpoint.DepositFundsAsync as previous, and I kept getting the failure message:


Flurl.Http.Testing.HttpTestException : Expected any calls to be made with URL pattern https://api.coinbase.com/v2/accounts/fff/deposits and body {"amount":10.0,"currency":"USD","payment_method":"B28EB04F-BD70-4308-90A1-96065283A001","commit":false}, but no matching calls were made.
   at Flurl.Http.Testing.HttpCallAssertion.Assert(Nullable`1 count)
   at Flurl.Http.Testing.HttpCallAssertion.With(Func`2 match, String descrip)
   at Coinbase.Tests.Endpoints.DepositTests.<can_depositfunds>d__1.MoveNext() in C:\Users\Brian Hart\source\repos\bchavez\Coinbase\Source\Coinbase.Tests\Endpoints\DepositTests.cs:line 45
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

Anticipated server response:
---

{
    "data": {
      "id": "67e0eaec-07d7-54c4-a72c-2e92826897df",
      "status": "completed",
      "payment_method": {
        "id": "83562370-3e5c-51db-87da-752af5ab9559",
        "resource": "payment_method",
        "resource_path": "/v2/payment-methods/83562370-3e5c-51db-87da-752af5ab9559"
      },
      "transaction": {
        "id": "441b9494-b3f0-5b98-b9b0-4d82c21c252a",
        "resource": "transaction",
        "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/transactions/441b9494-b3f0-5b98-b9b0-4d82c21c252a"
      },
      "amount": {
        "amount": "10.00",
        "currency": "USD"
      },
      "subtotal": {
        "amount": "10.00",
        "currency": "USD"
      },
      "created_at": "2015-01-31T20:49:02Z",
      "updated_at": "2015-02-11T16:54:02-08:00",
      "resource": "deposit",
      "resource_path": "/v2/accounts/2bbf394c-193b-5b2a-9155-3b4732659ede/deposits/67e0eaec-07d7-54c4-a72c-2e92826897df",
      "committed": true,
      "fee": {
        "amount": "0.00",
        "currency": "USD"
      },
      "payout_at": "2015-02-18T16:54:00-08:00"
    }
}

---

Listing 1. Failure message for the DepositTests.can_depositfunds method.

Upon changing the line in Listing 2 to that of Listing 3 (with the introduction of the using statement shown at the top of Listing 3, the test passes. Here's the listings of what was there currently vs. what was changed:

[Test]
public async Task can_depositfunds()
{
    /* ... */

         .WithRequestBody(@"{""amount"":10.0,""currency"":""USD"",""payment_method"":""B28EB04F-BD70-4308-90A1-96065283A001"",""commit"":false}")

    /* ... */         
}

Listing 2. Line to be changed.

using JsonSerializer = System.Text.Json.JsonSerializer;

[Test]
public async Task can_depositfunds()
{
    /* ... */

         .WithRequestBody(JsonSerializer.Serialize(create));

    /* ... */         
}

Listing 3. Using System.Text.Json to serialize the create to specify what request body is to be passed.

As you can see in Listing 3, I used the System.Text.Json.JsonSerializer.Serialize to serialize the create object as opposed to passing it as a hard-coded JSON string. The unit test now passes, both with the modification to the implementation of the IDepositsEndpoint.DepositFundsAsync method and the new way of specifying the proper request body.

This is due to the fact that Flurl now utilizes System.Text.Json to serialize / deserialize, whereas we utilize Newtonsoft.

astrohart commented 8 months ago

At 13:40 hours MST on 01/05/2024, I am moving onward to finish the DepositTests. Again, since I figure the changes to make are the same as previous, I will just document cases that deviate from the usual fix.

astrohart commented 8 months ago

Commit: 0d8ca14dce572f5d31af71ae3d4034793cb938c3

image Figure 1. All DepositTests unit tests are passing.

Success. All DepositTests unit tests are now passing.

astrohart commented 8 months ago

As of 13:46 hours MST on 01/05/2024, I am taking a lunch break.

astrohart commented 8 months ago

At 14:35 hours MST on 01/05/2024, I am resuming my work on the unit tests. Next up is the Coinbase.Tests.Endpoints.NotificationTests fixture. I am going to apply the usual technique(s) to get these unit tests to pass, as I suspect they are victims of the same things that befell the unit tests before them.

If there are any deviations from the suspected root causes of failure of these tests, I shall make a note here.

astrohart commented 8 months ago

Commit: ed273ed5868aefb8aca9233423cd1d9d04abc9ad

See the commit with the SHA referenced above. This commit consists of those changes that make the NotificationTests all pass. These changes were identical to the changes in previous commits regarding working around the JSON serialization issue.

astrohart commented 8 months ago

As of 14:47 hours MST on 01/05/2024, I am moving on to fix the failing PaymentMethodTests. Again, as previously, I suspect the fix is the same as for Commit ed273ed5868aefb8aca9233423cd1d9d04abc9ad.

Of course, as always, should the fix involve root causes other than the System.Text.Json/Newtonsoft issue, I will of course document it here.

astrohart commented 8 months ago

Commit: a1c877535356071f999de519dc3e323d13a1d12d

image Figure 1. Passing NotificationTests and PaymentMethodTests.

Please refer to the Commit having the SHA that is listed above. This commit consists of those changes that were necessary to make the PaymentMethodTests pass. As I suspected, the fix was to first obtain the response as a string, and then use JsonConvert.DeserializeObject to convert the string content into the proper response object.

astrohart commented 8 months ago

As of 14:53 hours MST on 01/05/2024, I am now moving on to making the TransactionTests pass. Again, as previously, I suspect the fix is the same as for Commit ed273ed5868aefb8aca9233423cd1d9d04abc9ad.

Of course, as always, should the fix involve root causes other than the System.Text.Json/Newtonsoft issue, I will of course document it here.

astrohart commented 8 months ago

As of 15:01 hours MST on 01/05/2024, I did make a small change to the TransactionTests.can_resend test. Specifically, the method that it calls returns Task<IFlurlResponse> which implements IDisposable. Therefore, we need to precede the var r with a using statement.

The unit test's code used to be:

[Test]
public async Task can_resend()
{
   var r = await client.Transactions.ResendRequestMoneyAsync("fff", "uuu");

   server.ShouldHaveCalled("https://api.coinbase.com/v2/accounts/fff/transactions/uuu/resend")
         .WithVerb(HttpMethod.Post);
}

Listing 1. The former code of the TransactionTests.can_resend unit test.

The new code of the unit test adds the using keyword before the text var r:

[Test]
public async Task can_resend()
{
   using var r = await client.Transactions.ResendRequestMoneyAsync("fff", "uuu");

   server.ShouldHaveCalled("https://api.coinbase.com/v2/accounts/fff/transactions/uuu/resend")
         .WithVerb(HttpMethod.Post);
}

Listing 2. The new code of the TransactionTests.can_resend unit test.

I propose that this small update to the body of the TransactionTests.can_resend be adopted. It should help decrease the risk of memory leaks.

astrohart commented 8 months ago

Commit: 87f3d3ed0eacc9f1ac4b5c9450a96a861a89adbf

image Figure 1. The `TransactionTests are all passing.

As shown in Figure 1 above, the TransactionTests are passing. Commit 87f3d3ed0eacc9f1ac4b5c9450a96a861a89adbf consists of the changes to make this so.

astrohart commented 8 months ago

At 15:33 hours MST on 01/05/2024, I am moving on to fix the WithdrawTests. Usual notes on technique to fix.

astrohart commented 8 months ago

Commit: 8e6fa2f8116b4176a248a66edb5ef38fc914225b

image Figure 1. The WithdrawalTests are passing.

As of 15:48 hours MST on 01/05/2024, I have made all the unit tests in WithdrawalTests pass, as shown in Figure 1.

astrohart commented 8 months ago

Commit: 98b744363ebe9bc73c5abd92bee5a1844c9881ae

image Figure 1. Current API version.

I noticed, when logging into my coinbase.com account, that the API Version is 2021-06-05. I suggest that the value of the Coinbase.CoinbaseClient.ApiVersionDate constant be modified to reflect this value. See commit 98b744363ebe9bc73c5abd92bee5a1844c9881ae.

astrohart commented 8 months ago

image Figure 1. Results of running the GitHubIssues suite of unit tests.

See *Figure 1. As of 15:56 hours MST on 01/05/2024, the GitHubIssues unit tests appear to pass with no further code changes.

astrohart commented 8 months ago

As of 15:59 hours MST on 01/05/2024, I am now moving on to get all the DataTests to pass. I am working off of the usual assumption that the changes that need to be made are in a similar vein to those above. Any different root causes, I will document here.

astrohart commented 8 months ago

As of 16:09 hours MST on 01/05/2024, I found that the unit test DataTests.can_get_currencies is failing partially because the line usd.Name.Should().StartWith("US Dollar"); is wrong. The Name property is now filled with United States Dollar. I propose that the testing line be changed to say usd.Name.Should().StartWith("United States Dollar");.

astrohart commented 8 months ago

Commit: 078b3d2c76a32ae2f1fbb3ed4952cba4cb675fa1

image Figure 1. All the DataTests are now passing.

As shown in Figure 1, all the DataTests are now passing. I cannot do the OAuthTests as they require a secrets.txt file to which I do not have access.

astrohart commented 8 months ago

As of 16:17 hours MST on 01/05/2024, I deleted the entire UserTests fixture since Coinbase has sunsetted the Users endpoint. See commit c280012fab8f2de9b26e08e5c42bfe8737fad228.

astrohart commented 8 months ago

As of 16:21 hours MST on 01/05/2024, all the tests are passing except the OAuthTests since those are dependent on secrets you possess and I do not.

I am pushing commit f9d111e0401e08587fbbef1a8a5f417c4dcde409 which deletes the [TestFixture] attribute from the CoinbaseApiKeyTests class. The [TestFixture] attribute is not needed. This class does not contain any unit tests.

astrohart commented 8 months ago

@bchavez I am going to close this PR and recommend you merge it. I have gotten AppVeyor to stop yelling at me! :-)

astrohart commented 8 months ago

@bchavez Whoops, I think that closing the PR is not advisable since it will be difficult for you to merge it.

astrohart commented 8 months ago

@bchavez Well, I am not sure what exactly the error

Could not detect any platforms from 'net6.0' in 'C:\Users\appveyor\.nuget\packages\fake.dotnet.nuget\6.0.0\lib\net6.0\Fake.DotNet.NuGet.dll', please tell the package authors

and those like it in AppVeyor mean, but could have something to do with this PR targeting Net 7.0 and not 6.0. Anyhow, can you please look into this? I would appreciate you considering this PR for merge. Thank you.

astrohart commented 8 months ago

log.txt File 1. AppVeyor logs from the latest build.

I am unsure of what to do at this juncture. It builds just fine on my PC, and all the unit tests (except the ones I cannot run due to not having the secrets.txt) pass. I am betting it's a configuration on the AppVeyor instance.

astrohart commented 8 months ago

@bchavez I had an inspiration. Upon looking at the AppVeyor logs I found where it was locking down the versions of the SharpCompress, FSharp.Data, and secure-file NuGet packages to 0.22.0, 2.4.6, and 1.0.31, respectively.

Consulting the nuget.org website for each of those packages, I found that, while secure-file has not had a version upgrade recently, both SharpCompress and FSharp.Data have, to versions 0.35.0 and 6.3.0, respectively. I figured it would not hurt to upgrade the package versions in build.fsx under the Builder project.

I pushed commit 58681d9d48e5094238735cf37ad7d5f33a2c7ac7, which includes these changes.

astrohart commented 8 months ago

@bchavez In the AppVeyor log, I see:

Script is not valid:
    C:\projects\coinbase\Source\Builder\Utils.fsx (3,10)-(3,17): Error FS0039: The namespace 'Runtime' is not defined.
    C:\projects\coinbase\Source\Builder\Utils.fsx (16,10)-(16,17): Error FS0039: The namespace 'Runtime' is not defined.

Other than that, I give up. I ask for your assistance. I simply do not know F# and I think I need to prevail upon yourself to fix the config for the new .NET targets.

Thank you!

astrohart commented 8 months ago

As of 16:54 hours MST on 01/05/2024, I decided to re-target the .NET COre multi-targeting of the Coinbase project to .NET 6 to see if that will make it play nicer with AppVeyor. I think .NET Core 7 has not been released on AppVeyor yet.

bchavez commented 8 months ago

Thanks for the investigation and PR.

Yes, the problem is the F# Builder tooling. F# FAKE build https://fake.build has been a bit of a pain because of the exact problems you're running into. I've recently migrated my open-source projects to NUKE build: https://nuke.build

A more recent example of Builder using NUKE build can be found here: https://github.com/BitArmory/Turnstile/tree/master/Builder

Basically, migrating to NUKE build is the first step in getting this repo running/building again. The NUKE build migration will have to be done in a separate PR when I can get to it. As you can see, there's a bit of maintenance work to keep things going.

bchavez commented 8 months ago

Just merged PR #91 to get the repo building again.

astrohart commented 8 months ago

Please review my PR and merge what you deem appropriate! I tried to be super thorough in my comments and I tried to be prodigious about writing down which Commit SHA each comment goes with.