BundleExporter.CheckBundleExportStatus cannot deserialize status "in_progress" properly.

[PatrickMNL]

When the BundleExporter.CheckBundleExportStatus returns a status of "in_progress" we run into the following JSON deserialization issue;

System.ArgumentException: Error occurred during deserialization from JSON String
   at Crowdin.Api.Core.Converters.DescriptionEnumConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSerializer serializer) in C:\Projects\crowdin-api-client-dotnet\src\Crowdin.Api\Core\Converters\DescriptionEnumConverter.cs:line 85
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty property, JsonConverter propertyConverter, JsonContainerContract containerContract, JsonProperty containerProperty, JsonReader reader, Object target)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader reader, Type objectType)
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String value, Type type, JsonSerializerSettings settings)
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String value, JsonSerializerSettings settings)
   at Crowdin.Api.Core.JsonParser.ParseResponseObject[TData](JObject rootElement) in C:\Projects\crowdin-api-client-dotnet\src\Crowdin.Api\Core\JsonParser.cs:line 53
   at Crowdin.Api.Bundles.BundlesApiExecutor.CheckBundleExportStatus(Int32 projectId, Int32 bundleId, String exportId) in C:\Projects\crowdin-api-client-dotnet\src\Crowdin.Api\Bundles\BundlesApiExecutor.cs:line 133
   at OSM.Translations.Api.Services.CrowdinService.ExportBundle(Bundle bundleDefinition) in C:\Projects\OSM.Translations\OSM.Translations.Api\Services\CrowdinService.cs:line 55
   at OSM.Translations.Api.Services.CrowdinService.GetBundle(String bundleName) in C:\Projects\OSM.Translations\OSM.Translations.Api\Services\CrowdinService.cs:line 21
   at OSM.Translations.Api.V1.TranslationController.GetCrowdinTranslationsBundleAsZip(String bundleName) in C:\Projects\OSM.Translations\OSM.Translations.Api\V1\TranslationController.cs:line 23
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfIActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Logged|17_1(ResourceInvoker invoker)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

HEADERS
=======
Accept: application/json
Host: localhost:59954
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/114.0.0.0 Safari/537.36
:method: GET
Accept-Encoding: gzip, deflate, br
Accept-Language: en,nl;q=0.9,nl-NL;q=0.8
Referer: https://localhost:59954/swagger/index.html
sec-ch-ua: "Not.A/Brand";v="8", "Chromium";v="114", "Google Chrome";v="114"
sec-ch-ua-mobile: ?0
sec-ch-ua-platform: "Windows"
sec-fetch-site: same-origin
sec-fetch-mode: cors
sec-fetch-dest: empty

Expected: It should properly parse the status to BuildStatus.InProgress.

Reason: The Enum definition has a nice Description attribute above it which is read by a custom deserializer, but it is initialized with inProgress instead of in_progress (perhaps because it has been changed recently?)

[PatrickMNL]

@andrii-bodnar I don't mind picking this up myself, if I can get permissions to open pull requests.

[andrii-bodnar]

@PatrickMNL great, thank you! 🚀

Feel free to fork the repo and create a PR

[PatrickMNL]

After some investigation, I found that both these status representations are being returned by the v2 API;

• "status": "inProgress" (Translations -> Build Status)
• "status": "in_progress" (Bundles -> Build Status)

A proper fix should probably be done on the API server side instead of in this package.

I can still offer a fix, but I don't really like the options I have of fixing it, what I considered:

• Adding a new BuildStatus enum specifically for Crowdin.Api.Bundles (probably what I would go for).
• Adding a new attribute which is also considered by the DescriptionEnumConverter (feels hacky)
• Remove any underscores before doing a case insensitive equals check and using the current comparison (also feels hacky).
• More hacky stuff 😓.

[andrii-bodnar]

@PatrickMNL thanks a lot for the investigation!

This should definitely be fixed on the API side. Passing it on to the development team

[PatrickMNL]

Alright, glad we're on the same page, thanks @andrii-bodnar! Would u have a ballpark estimate about when we can expect the fix? Just checking if it's worth waiting or that we should build in a small work around for now.

[andrii-bodnar]

@PatrickMNL I will provide the exact estimate as soon as possible. It would be great if we could make a temporary workaround just to not block further integration.

[PatrickMNL]

@andrii-bodnar I have added 'temporary workaround' suggestion in this PR: #169

[PatrickMNL]

Thanks for considering and merging the PR @andrii-bodnar, when can we expect a new release of the NuGet package? I would like to start using it with the fixes included 😄.

[andrii-bodnar]

@PatrickMNL sure, I am going to release a new version in a few minutes

[andrii-bodnar]

@PatrickMNL just released a new version - 2.14.3

[PatrickMNL]

@andrii-bodnar Thanks for the release! I see that the NuGet feed did not yet receive the new version, does that take a while or was it perhaps missed during the update?

[PatrickMNL]

Ignore the nuget feed question, I see it just got updated! Thanks again 👍!

[innomaxx]

@PatrickMNL thanks for your contribution to this project!

@andrii-bodnar can we consider migration to permanent solution for this case? For example add extra [Description] attribute with "in_progress" value or create separate enum for Bundle API. As OP mentioned before.

Probably new value from API is already used by end users. We can avoid breaking backward compatibility just by fixing this on client side

[PatrickMNL]

@innomaxx thanks for the suggestions, I did also consider the second [Description] attribute, but unfortunately that .NET attribute doesn't allow for 2 occurences of the attribute on the same property. We could also move away from the attribute and instead use a custom one, that allows for multiple names on a property. Or we could indeed consider the seperate enum for the BundleAPI (if we really want to fix this clientsided), both would require small adjustments to the current serialization flow. I can see that backwards compatibility would indeed break for people with custom implementations, made during the time the Bundle API send back 'in_progress'.

[andrii-bodnar]

Hi @PatrickMNL,

let's create a new class for bundle build status.

Having the status response in the snake_case is more correct.

Due to some historical reasons, it was introduced in camelCase for the Build Project Translation and Build Project Directory Translation API methods and can't be changed now.

Currently, the following API endpoints have the snake_case status response:

• Export Bundle
• Export Glossary
• Import Glossary
• Apply Pre-Translation
• Export TM
• Import TM

as well as the correcponding Check ... status

[PatrickMNL]

@andrii-bodnar thanks for the reply.

Can do, definitely, are you aware that introducing the new enum (class?), would cause 'breaking changes' when updating the package, as in, people will need to move their own Bundle implementations to this newly created enum.

If we're okay with that, than I can introduce a PR for that.

Something small to consider; If we're introducing breaking changes, we are also able to fix the existing BuildStatus and move the older incorrectly named variants to something like a LegacyBuildStatus.

[andrii-bodnar]

@PatrickMNL Yes, we'll add a warning to the release notes about these breaking changes.

Something small to consider; If we're introducing breaking changes, we are also able to fix the existing BuildStatus and move the older incorrectly named variants to something like a LegacyBuildStatus.

I think this is a good idea!

[PatrickMNL]

@andrii-bodnar I have created the above PR as potential fix, I am able to test the Bundle API against our live implementation, we don't have a Translation/Directory API implementation internally, so I am unable to test those, is that something you can verify?

[andrii-bodnar]

@PatrickMNL thanks a lot!

We have some examples in this repo, maybe it would be possible to test the new code using these examples?