Skip to content

fix(routes-v5): deserialize route API response from data envelope#102

Open
r4m-semeyon wants to merge 4 commits intomasterfrom
fix/routes-object-issues-260202
Open

fix(routes-v5): deserialize route API response from data envelope#102
r4m-semeyon wants to merge 4 commits intomasterfrom
fix/routes-object-issues-260202

Conversation

@r4m-semeyon
Copy link
Copy Markdown
Contributor

  • Use RoutesResponse wrapper for GetRoutes/GetRoutesAsync (V5 returns { data: [...] })
  • Add RouteResponse wrapper and use for UpdateRoute/UpdateRouteAsync (single route in data)
  • Fix MemberConfigStorage null reference in DataObjectRoute
  • Fix XML doc typos and param description in RouteManagerV5

- Use RoutesResponse wrapper for GetRoutes/GetRoutesAsync (V5 returns { data: [...] })
- Add RouteResponse wrapper and use for UpdateRoute/UpdateRouteAsync (single route in data)
- Fix MemberConfigStorage null reference in DataObjectRoute
- Fix XML doc typos and param description in RouteManagerV5
@r4m-semeyon r4m-semeyon requested review from h2rd and r4m-juan February 2, 2026 06:10
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 2, 2026

Gemini AI Code Risk & Compatibility Analysis

Status: [OK] Analysis completed successfully

Files analyzed:

  • route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/Route.cs
  • route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/RouteResponse.cs
  • route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteManagerV5.cs

Analysis Results

Hook registry initialized with 0 hook entries
(node:2314) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. MaxListeners is 10. Use events.setMaxListeners() to increase limit
(Use node --trace-warnings ... to show where the warning was created)
Here is the code analysis report.

High Severity

  • Potential Backward Compatibility Risks (Data Contract Changes)
    • Location: route4me-csharp-sdk/RouteMeSDKLibrary/Managers/RouteManagerV5.cs
    • Risk: The methods GetRoutes, GetRoutesAsync, UpdateRoute, and UpdateRouteAsync have been changed to expect a new JSON response structure from the API. The route data is no longer expected as the top-level object or array but is now nested within a data property. This is handled by introducing new wrapper classes (RouteResponse, RoutesResponse). This is a breaking data contract change.
    • Details:
      • In GetRoutes and GetRoutesAsync, the deserialization target was changed from DataObjectRoute[] to RoutesResponse, and the code now returns result.Data.
      • In UpdateRoute and UpdateRouteAsync, the deserialization target was changed from DataObjectRoute to RouteResponse, and the code now returns response.Data.
    • Suggestion: This change requires a major version bump of the SDK package. The release notes must clearly communicate that the SDK is now incompatible with older API versions and that the response format has changed, affecting any direct JSON manipulation clients might be performing.

Low Severity

  • Behavior changes affecting clients
    • Location: route4me-csharp-sdk/RouteMeSDKLibrary/DataTypes/V5/Routes/Route.cs
    • Risk: The MemberConfigStorage property getter has been modified to be more resilient to null values. Previously, it could throw a NullReferenceException if MemberConfigStorageInternal or a value within it was null. It now returns an empty dictionary or an empty string in these scenarios. While this makes the code more robust, it's a behavior change that could affect clients who (improperly) relied on the exception being thrown.
    • Suggestion: This is a positive change for stability and should be kept. Mentioning this improvement in the change log is good practice.

Improvements

  • Code Quality and Correctness
    • Location: route4me-csharp-sdk/RouteMeSDKLibrary/Managers/RouteManagerV5.cs
    • Change: The UpdateRouteAsync method signature was correctly updated to be async and now uses await with .ConfigureAwait(false).
    • Comment: This is a good improvement that aligns the method with best practices for asynchronous programming in C#, preventing potential deadlocks in certain contexts.

@route4me route4me deleted a comment from github-actions bot Feb 2, 2026
@route4me route4me deleted a comment from github-actions bot Feb 2, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 2, 2026

Gemini AI Code Risk & Compatibility Analysis

Status: [OK] Analysis completed successfully

Files analyzed:

  • route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/Route.cs
  • route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/RouteResponse.cs
  • route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteManagerV5.cs
  • route4me-csharp-sdk/Route4MeSDKLibrary/QueryTypes/V5/RouteFilterParameters.cs

Analysis Results

Hook registry initialized with 0 hook entries
(node:2434) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. MaxListeners is 10. Use events.setMaxListeners() to increase limit
(Use node --trace-warnings ... to show where the warning was created)
Analysis of Code Changes (Unified Diff)


1. route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/Route.cs

  • Change: Added null checks to the MemberConfigStorage property accessor to prevent NullReferenceException and ensure a non-null dictionary is always returned.
  • Security and performance risks: Low. No new risks introduced; rather, it improves the robustness of the code by preventing potential runtime errors.
  • Deprecated API usage: None.
  • Coding standard violations: None. This change adheres to good coding practices for null safety.
  • Potential backward compatibility risks: Low. This change makes the code more resilient to null values. Existing clients might experience a change in behavior if they were relying on a NullReferenceException for a null MemberConfigStorageInternal or pair.Value, but this is unlikely and generally an improvement.

2. route4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/RouteResponse.cs

  • Change: A new file RouteResponse.cs was created, defining a RouteResponse class that acts as a wrapper for a single DataObjectRoute instance, encapsulating it within a Data property.
  • Security and performance risks: Low. No inherent security or performance risks with the introduction of this wrapper class.
  • Deprecated API usage: None.
  • Coding standard violations:
    • Severity: Medium. The Data property is missing XML documentation for its purpose.
    • Severity: Low. The file ends without a newline character, which is a minor formatting inconsistency.
  • Potential backward compatibility risks: High. The introduction of this new wrapper class suggests a change in the API's contract where single route responses are now enveloped. While the diffs in RouteManagerV5.cs attempt to maintain the existing public API signatures, the underlying structure of the API response has changed. This could impact clients that directly consume the raw API responses or have custom deserialization logic.

3. route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteManagerV5.cs

  • Change:
    • The GetRoutes and GetRoutesAsync methods now internally receive a RoutesResponse (an implicit wrapper for DataObjectRoute[], not fully shown in the diff but implied) from GetJsonObjectFromAPI / GetJsonObjectFromAPIAsync, and then extract the Data array. Null checks (?.Data ?? Array.Empty<DataObjectRoute>()) are added.
    • The UpdateRoute and UpdateRouteAsync methods now internally receive a RouteResponse from GetJsonObjectFromAPI / GetJsonObjectFromAPIAsync, and then extract the Data object. Null checks (?.Data) are added.
    • XML comments were improved for UpdateRoute and UpdateRouteAsync, and ConfigureAwait(false) was added to UpdateRouteAsync.
  • Security and performance risks: Low. These changes improve code robustness by explicitly handling null responses and asynchronous context management.
  • Deprecated API usage: None.
  • Coding standard violations: None. The addition of ConfigureAwait(false) and improved XML comments are positive changes.
  • Potential backward compatibility risks: High.
    • API Behavior Change: Although the public method signatures for GetRoutes, GetRoutesAsync, UpdateRoute, and UpdateRouteAsync appear unchanged, their internal implementation now processes a wrapped API response. More importantly, the null-handling behavior for the returned DataObjectRoute or DataObjectRoute[] has changed. Previously, a null API response might have resulted in a null return value or an exception. Now, GetRoutes will return an empty array (Array.Empty<DataObjectRoute>()) if the data is null, and UpdateRoute (and UpdateRouteAsync) will return null if the data is null. Clients that explicitly rely on distinguishing between a null array and an empty array, or expecting specific null object behavior, could be affected.
    • Internal Callers: If GetJsonObjectFromAPI or GetJsonObjectFromAPIAsync were used directly by other internal methods and expected DataObjectRoute[] or DataObjectRoute, those internal calls would now be broken.

4. route4me-csharp-sdk/Route4MeSDKLibrary/QueryTypes/V5/RouteFilterParameters.cs

  • Change: Removal of the Byte Order Mark (BOM) from the file.
  • Security and performance risks: Low. This is a technical formatting change with no security or performance implications.
  • Deprecated API usage: None.
  • Coding standard violations: Low. This is an improvement as removing BOMs from source files is generally a good practice for consistency and avoiding potential parsing issues in certain environments.
  • Potential backward compatibility risks: None.

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

Successfully merging this pull request may close these issues.

1 participant