feat: add dedicated endpoint for route custom data#109
feat: add dedicated endpoint for route custom data#109yurii-bart wants to merge 4 commits intoroute4me:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds dedicated endpoints for managing route custom data in the Route4Me API V5, replacing the previous approach of managing custom data through the general Routes API. The PR introduces a new manager class (RouteCustomDataManagerV5), new data types for handling bulk operations, and deprecates old methods while maintaining backward compatibility.
Changes:
- Introduces
RouteCustomDataManagerV5with dedicated CRUD operations for route custom data via/route-custom-data/endpoints - Adds support for bulk retrieval of custom data for multiple routes via the
/route-custom-data/bulkendpoint - Deprecates existing
UpdateRouteCustomDataandGetRouteCustomDatamethods that use the Routes API in favor of new dedicated endpoints
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Route4MeSDKLibrary/Managers/RouteCustomDataManagerV5.cs | New manager class providing GET, PUT, and bulk POST operations for route custom data |
| Route4MeSDKLibrary/DataTypes/V5/Routes/RouteCustomDataCollection.cs | Response type for bulk route custom data operations |
| Route4MeSDKLibrary/DataTypes/V5/Routes/RouteCustomDataDictionaryConverter.cs | JSON converter handling both array and object response formats from the API |
| Route4MeSDKLibrary/Route4MeManagerV5.cs | Added new methods delegating to RouteCustomDataManagerV5 and marked old methods as obsolete |
| Route4MeSDKLibrary/Managers/RouteManagerV5.cs | Marked existing custom data methods as obsolete |
| Route4MeSDKLibrary/Consts.cs | Added endpoint constants for route custom data endpoints |
| Route4MeSDKTest/Examples/API5/Routes/*.cs | Four new example files demonstrating usage of the new endpoints |
| Route4MeSdkV5UnitTest/V5/Routes/RouteCustomDataTests.cs | Comprehensive test suite for new route custom data functionality |
| Route4MeSdkV5UnitTest/V5/AddOnRoutesApi/AddOnRoutesApiTests.cs | Updated existing tests to use new API, marked old tests as obsolete |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteCustomDataManagerV5.cs
Show resolved
Hide resolved
| /// <param name="routeId">The route ID (32-character hex string)</param> | ||
| /// <param name="resultResponse">Failure response</param> | ||
| /// <returns>An array of dictionaries representing route custom data, or null if not set</returns> | ||
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomDataDedicated")] |
There was a problem hiding this comment.
The Obsolete attribute message references "RouteCustomDataManagerV5.GetRouteCustomDataDedicated" but the actual method name is "GetRouteCustomData" not "GetRouteCustomDataDedicated". The message should be updated to reference the correct method: "Use RouteCustomDataManagerV5.GetRouteCustomData" for consistency.
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomDataDedicated")] | |
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomData")] |
| /// </summary> | ||
| /// <param name="routeId">The route ID (32-character hex string)</param> | ||
| /// <returns>A Tuple containing route custom data or/and failure response</returns> | ||
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomDataDedicatedAsync")] |
There was a problem hiding this comment.
The Obsolete attribute message references "RouteCustomDataManagerV5.GetRouteCustomDataDedicatedAsync" but the actual method name is "GetRouteCustomDataAsync" not "GetRouteCustomDataDedicatedAsync". The message should be updated to reference the correct method: "Use RouteCustomDataManagerV5.GetRouteCustomDataAsync" for consistency.
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomDataDedicatedAsync")] | |
| [Obsolete("Use RouteCustomDataManagerV5.GetRouteCustomDataAsync")] |
...e4me-csharp-sdk/Route4MeSDKLibrary/DataTypes/V5/Routes/RouteCustomDataDictionaryConverter.cs
Show resolved
Hide resolved
route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteCustomDataManagerV5.cs
Show resolved
Hide resolved
route4me-csharp-sdk/Route4MeSDKLibrary/Managers/RouteCustomDataManagerV5.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Updates the custom data for the specified route using the dedicated | ||
| /// PUT /route-custom-data/{route_id} endpoint. | ||
| /// Replaces all existing custom data; keys not present in <paramref name="customData"/> are removed. | ||
| /// To clear all custom data, submit a single key with a null value. |
There was a problem hiding this comment.
Documentation is inconsistent with the implementation in RouteCustomDataManagerV5. The documentation states "To clear all custom data, submit a single key with a null value" but RouteCustomDataManagerV5.UpdateRouteCustomData rejects null customData. This documentation should be corrected to match the actual behavior.
| /// To clear all custom data, submit a single key with a null value. | |
| /// To clear all custom data, submit an empty dictionary for <paramref name="customData"/>. |
| /// Updates the custom data for the specified route asynchronously using the dedicated | ||
| /// PUT /route-custom-data/{route_id} endpoint. | ||
| /// Replaces all existing custom data; keys not present in <paramref name="customData"/> are removed. | ||
| /// To clear all custom data, submit a single key with a null value. |
There was a problem hiding this comment.
Documentation is inconsistent with the implementation in RouteCustomDataManagerV5. The documentation states "To clear all custom data, submit a single key with a null value" but RouteCustomDataManagerV5.UpdateRouteCustomDataAsync rejects null customData. This documentation should be corrected to match the actual behavior.
| resultResponse = new ResultResponse | ||
| { | ||
| Status = false, | ||
| Messages = new Dictionary<string, string[]> | ||
| { | ||
| { "Error", new[] { "The customData parameter is null" } } | ||
| } | ||
| }; | ||
|
|
||
| return null; |
There was a problem hiding this comment.
Documentation and implementation are inconsistent. The documentation states "To clear all custom data, submit a single key with a null value" but the code rejects null customData and returns an error. Either the validation should allow null (treating it as a clear operation), or the documentation should be corrected to explain how to actually clear custom data (e.g., by sending an empty dictionary).
| resultResponse = new ResultResponse | |
| { | |
| Status = false, | |
| Messages = new Dictionary<string, string[]> | |
| { | |
| { "Error", new[] { "The customData parameter is null" } } | |
| } | |
| }; | |
| return null; | |
| // Treat null customData as a request to clear all custom data | |
| // by sending an empty dictionary to the API. | |
| customData = new Dictionary<string, string>(); |
| /// <summary> | ||
| /// Filter routes by custom data key-value pairs. | ||
| /// Each item specifies a <c>key</c> (max 50 chars) and a <c>value</c> | ||
| /// that is either a single string or an array of strings. |
There was a problem hiding this comment.
Documentation states that the value can be "either a single string or an array of strings" but the property type is Dictionary<string, string>, which only supports single string values. Either the documentation should be corrected to remove mention of array values, or the type should be changed to Dictionary<string, object> or Dictionary<string, string[]> if array values are actually supported by the API.
| /// that is either a single string or an array of strings. | |
| /// that is a single string. |
| [Obsolete] | ||
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
There was a problem hiding this comment.
These test attributes are misleading. The test is now calling the NEW non-obsolete UpdateRouteCustomData method (which accepts Dictionary<string, string> and returns Dictionary<string, string>), not the deprecated one. The test should either be removed, updated to test the deprecated method explicitly by passing Dictionary<string, string>[], or have these misleading attributes removed since it's actually testing the new API.
| [Obsolete] | |
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] | |
| [Ignore("This test is currently ignored pending review of the UpdateRouteCustomData API usage.")] |
| [Obsolete] | ||
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
There was a problem hiding this comment.
These test attributes are misleading. The test is now calling the NEW non-obsolete UpdateRouteCustomDataAsync method (which accepts Dictionary<string, string>), not the deprecated one. The test should either be removed, updated to test the deprecated method explicitly, or have these misleading attributes removed since it's actually testing the new API.
| [Obsolete] | ||
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
There was a problem hiding this comment.
These test attributes are misleading. The test is now calling the NEW non-obsolete GetRouteCustomData method, not the deprecated one. The comment says "First, set custom data on the route using the dedicated endpoint" but the Obsolete and Ignore attributes suggest this uses deprecated methods. The test should either be removed, updated to test the deprecated method explicitly, or have these misleading attributes removed.
| [Obsolete] | |
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
| [Obsolete] | ||
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
There was a problem hiding this comment.
These test attributes are misleading. The test is now calling the NEW non-obsolete GetRouteCustomDataAsync method, not the deprecated one. The test should either be removed, updated to test the deprecated method explicitly, or have these misleading attributes removed.
| [Obsolete] | |
| [Ignore("This test uses deprecated methods and will be removed in a future version.")] |
| if (customData == null) | ||
| { | ||
| return new Tuple<Dictionary<string, string>, ResultResponse>(null, new ResultResponse | ||
| { | ||
| Status = false, | ||
| Messages = new Dictionary<string, string[]> | ||
| { | ||
| { "Error", new[] { "The customData parameter is null" } } | ||
| } | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Documentation and implementation are inconsistent. The documentation states "To clear all custom data, submit a single key with a null value" but the code rejects null customData and returns an error. Either the validation should allow null (treating it as a clear operation), or the documentation should be corrected to explain how to actually clear custom data.
| if (customData == null) | |
| { | |
| return new Tuple<Dictionary<string, string>, ResultResponse>(null, new ResultResponse | |
| { | |
| Status = false, | |
| Messages = new Dictionary<string, string[]> | |
| { | |
| { "Error", new[] { "The customData parameter is null" } } | |
| } | |
| }); | |
| } |
No description provided.