Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,24 @@ private static string GetCorrectedPageSizeName(string originalName, ClientProvid
return originalName;
}

private static string GetCorrectedMaxCountName(ClientProvider client)
{
// Check if "top" parameter exists in LastContractView for backward compatibility
var existingTopParam = client.LastContractView?.Methods
Comment thread
JoshLove-msft marked this conversation as resolved.
Outdated
?.SelectMany(method => method.Signature.Parameters)
.FirstOrDefault(parameter => string.Equals(parameter.Name, TopParameterName, StringComparison.OrdinalIgnoreCase))
?.Name;

if (existingTopParam != null)
{
// Preserve the exact name (including casing) from the previous contract for backward compatibility
return existingTopParam;
}

// Use the new standard "maxCount" parameter name
return MaxCountParameterName;
}

private static bool ShouldUpdateReinjectedParameter(InputParameter inputParameter, InputPagingServiceMethod? pagingServiceMethod)
{
// Check if this is an API version parameter
Expand Down Expand Up @@ -1013,11 +1031,15 @@ internal static List<ParameterProvider> GetMethodParameters(
continue;
}

// For paging operations, rename "top" parameter to "maxCount"
// For paging operations, rename "top" parameter to "maxCount" (with backward compatibility)
if (serviceMethod is InputPagingServiceMethod &&
string.Equals(inputParam.Name, TopParameterName, StringComparison.OrdinalIgnoreCase))
{
inputParam.Update(name: MaxCountParameterName);
var correctedMaxCountName = GetCorrectedMaxCountName(client);
if (!string.Equals(inputParam.Name, correctedMaxCountName, StringComparison.Ordinal))
{
inputParam.Update(name: correctedMaxCountName);
}
}

// For paging operations, ensure page size parameter uses the correct casing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,38 @@ public void TopParameterRenamedToMaxCountInPagingOperation()
Assert.Contains("maxCount", parameterNames, "Should contain 'maxCount' parameter");
Assert.IsFalse(parameterNames.Contains("top"), "Should not contain 'top' parameter after renaming");
}

[Test]
public void TopParameterConvertedToMaxCountWhenNoBackwardCompatibility()
Comment thread
JoshLove-msft marked this conversation as resolved.
Outdated
{
// This test verifies that when a "top" parameter exists in the paging operation,
// it is converted to "maxCount" when no LastContractView is present (no backward compatibility)
var topParameter = InputFactory.QueryParameter("top", InputPrimitiveType.Int32);
var pagingMetadata = InputFactory.PagingMetadata(["items"], null, null);
var responseModel = InputFactory.Model("Response", properties: [
InputFactory.Property("items", InputFactory.Array(InputPrimitiveType.String))
]);
var response = InputFactory.OperationResponse([200], responseModel);
var operation = InputFactory.Operation("getItems", parameters: [topParameter], responses: [response]);
var pagingMethod = InputFactory.PagingServiceMethod("getItems", operation, pagingMetadata: pagingMetadata);
var client = InputFactory.Client("testClient", methods: [pagingMethod]);

MockHelpers.LoadMockGenerator(inputModels: () => [responseModel], clients: () => [client]);

var restClientProviders = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders
.OfType<RestClientProvider>().ToList();

Assert.IsTrue(restClientProviders.Count > 0, "RestClientProvider should be generated");

var parameterNames = restClientProviders
.SelectMany(p => p.Methods)
.SelectMany(m => m.Signature.Parameters)
.Select(param => param.Name)
.ToList();

Assert.Contains("maxCount", parameterNames, "Should contain 'maxCount' parameter after conversion");
Assert.IsFalse(parameterNames.Contains("top"), "Should not contain 'top' parameter after conversion to 'maxCount'");
}

[Test]
public void NoNextLinkOrContinuationTokenOfT()
Expand Down
115 changes: 115 additions & 0 deletions packages/http-client-csharp/generator/docs/backward-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- [Non-abstract Base Models](#non-abstract-base-models)
- [Model Constructors](#model-constructors)
- [Parameter Name Casing](#parameter-name-casing)
- [Page Size Parameter Casing Correction](#scenario-page-size-parameter-casing-correction)
- [Top Parameter Conversion to MaxCount](#scenario-top-parameter-conversion-to-maxcount)

## Overview

Expand Down Expand Up @@ -475,3 +477,116 @@ public virtual AsyncPageable<Item> GetItemsAsync(int? maxPageSize = null, Cancel
- **Case 2 (Normalization)**: If the parameter does NOT exist in LastContractView, badly-cased variants are normalized to proper camelCase
- The HTTP query parameter always uses the original serialized name from the spec (e.g., `maxpagesize`)
- Existing client code continues to compile without changes

#### Scenario: Top Parameter Conversion to MaxCount

**Description:** For paging operations, the generator converts the `top` parameter to `maxCount` to follow standard naming conventions. However, backward compatibility is maintained in two ways:

1. **If the `top` parameter exists in LastContractView**: The generator preserves the `top` parameter name (with its exact casing) to maintain backward compatibility
2. **If the `top` parameter does NOT exist in LastContractView**: The generator converts `top` to the standardized `maxCount` parameter name

This commonly occurs when:

- Migrating from an older API version or generator that used `top` for pagination limits
- TypeSpec defines paging operations with a `top` parameter
- The generator needs to standardize on `maxCount` while maintaining backward compatibility

**Example:**

**Case 1: Top parameter exists in LastContractView - preserved for backward compatibility**

Previous version had `top` parameter:

```csharp
public virtual AsyncPageable<Item> GetItemsAsync(int? top = null, CancellationToken cancellationToken = default)
{
HttpMessage CreateRequest()
{
var message = pipeline.CreateMessage();
var request = message.Request;
request.Method = RequestMethod.Get;
var uri = new RequestUriBuilder();
uri.Reset(endpoint);
uri.AppendPath("/items", false);
if (top != null)
{
uri.AppendQuery("top", top.Value, true); // Serialized name from spec
}
// ...
}
// ...
}
```

Current TypeSpec still defines `top` parameter:

```typespec
op getItems(@query top?: int32): Page<Item>;
```

**Generated Compatibility Result:**

The generator detects the `top` parameter in LastContractView and preserves it exactly to maintain backward compatibility:

```csharp
public virtual AsyncPageable<Item> GetItemsAsync(int? top = null, CancellationToken cancellationToken = default)
{
HttpMessage CreateRequest()
{
var message = pipeline.CreateMessage();
var request = message.Request;
request.Method = RequestMethod.Get;
var uri = new RequestUriBuilder();
uri.Reset(endpoint);
uri.AppendPath("/items", false);
if (top != null)
{
uri.AppendQuery("top", top.Value, true); // Still uses "top" for backward compatibility
}
// ...
}
// ...
}
```

**Case 2: Top parameter does NOT exist in LastContractView - converted to maxCount**

New paging operation with `top` parameter (no previous version):

```typespec
op getItems(@query top?: int32): Page<Item>;
```

**Generated Result:**

The generator converts the parameter name to standardized `maxCount` since there's no previous version to maintain compatibility with:

```csharp
public virtual AsyncPageable<Item> GetItemsAsync(int? maxCount = null, CancellationToken cancellationToken = default)
{
HttpMessage CreateRequest()
{
var message = pipeline.CreateMessage();
var request = message.Request;
request.Method = RequestMethod.Get;
var uri = new RequestUriBuilder();
uri.Reset(endpoint);
uri.AppendPath("/items", false);
if (maxCount != null)
{
uri.AppendQuery("top", maxCount.Value, true); // Serialized name still uses "top" from spec
}
// ...
}
// ...
}
```

**Key Points:**

- **Case 1 (Backward compatibility)**: If `top` parameter exists in LastContractView, its exact name and casing are preserved
- **Case 2 (Standardization)**: If `top` parameter does NOT exist in LastContractView, it is converted to `maxCount` for consistency
- The HTTP query parameter always uses the original serialized name from the spec (e.g., `top`)
- This conversion is specific to paging operations only
- Existing client code with `top` continues to compile without changes
- New code benefits from the standardized `maxCount` naming convention
Loading