Skip to content

Commit

Permalink
[csharp][generichost] Add options for model parameter sorting (#18886)
Browse files Browse the repository at this point in the history
* fixed parameter ordering

* placed changes behind a switch

* bug fix

* minor revert

* use lombok.Setter

* addressed comment

* lint

* minor refactor

* massively improve csharp templates code

improved apiclient.mustache to keep it dry, sharing a single exec with Action<> delegate.
improved api.mustache to keep it dry, used chain constructors, kept RequestOptions in a single method, set configuration with alias directive
moved fqn types to using directive for cleaner code
removed 'this' from variables/props that are redundant
fix CSharpClientDeepObjectTest code is now DRY so count must only be one

* updated test samples csharp

* removed async from ExecAsync (not needed here anymore)

* updated samples csharp

* nullable property not working on models due to not being defined in yaml schema

updated samples

* added options

* rebuild tests

* revert unintended commits

* rebuild samples

* rebuild samples

* rererebuild samples

* fix manual tests

---------

Co-authored-by: filipe <[email protected]>
  • Loading branch information
devhl-labs and filipe-silva authored Jun 12, 2024
1 parent fc0105e commit 3abe251
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 14 deletions.
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net4.7-formModels.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ additionalProperties:
equatable: true
targetFramework: net47
skipFormModel: false
modelPropertySorting: alphabetical
inlineSchemaOptions:
RESOLVE_INLINE_ENUMS: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ additionalProperties:
packageGuid: '{2E60EF87-DB0B-4D01-A36E-F5E90F7EC757}'
useDateTimeForDate: true
targetFramework: net47
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net4.7.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ additionalProperties:
nullableReferenceTypes: false
equatable: true
targetFramework: net47
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net4.8-formModels.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ additionalProperties:
equatable: true
targetFramework: net48
skipFormModel: false
modelPropertySorting: alphabetical
inlineSchemaOptions:
RESOLVE_INLINE_ENUMS: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ additionalProperties:
packageGuid: '{2E60EF87-DB0B-4D01-A36E-F5E90F7EC757}'
useDateTimeForDate: true
targetFramework: net48
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net4.8.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ additionalProperties:
nullableReferenceTypes: false
equatable: true
targetFramework: net48
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net8-formModels.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ additionalProperties:
equatable: true
targetFramework: net8.0
skipFormModel: false
modelPropertySorting: alphabetical
inlineSchemaOptions:
RESOLVE_INLINE_ENUMS: true
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ additionalProperties:
useSourceGeneration: true
equatable: true
targetFramework: net8.0
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net8-nrt.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ additionalProperties:
nullableReferenceTypes: true
equatable: true
targetFramework: net8.0
modelPropertySorting: alphabetical
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ additionalProperties:
packageGuid: '{2E60EF87-DB0B-4D01-A36E-F5E90F7EC757}'
useDateTimeForDate: true
targetFramework: net8.0
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-net8.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ additionalProperties:
nullableReferenceTypes: false
equatable: true
targetFramework: net8.0
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions bin/configs/csharp-generichost-netstandard2.0.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ additionalProperties:
disallowAdditionalPropertiesIfNotPresent: false
targetFramework: netstandard2.0
equatable: true
modelPropertySorting: alphabetical
1 change: 1 addition & 0 deletions docs/generators/csharp.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl
|library|HTTP library template (sub-template) to use|<dl><dt>**generichost**</dt><dd>HttpClient with Generic Host dependency injection (https://docs.microsoft.com/en-us/dotnet/core/extensions/generic-host) (Experimental. Subject to breaking changes without notice.)</dd><dt>**httpclient**</dt><dd>HttpClient (https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient) (Experimental. Subject to breaking changes without notice.)</dd><dt>**unityWebRequest**</dt><dd>UnityWebRequest (...) (Experimental. Subject to breaking changes without notice.)</dd><dt>**restsharp**</dt><dd>RestSharp (https://github.com/restsharp/RestSharp)</dd></dl>|restsharp|
|licenseId|The identifier of the license| |null|
|modelPropertyNaming|Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name| |PascalCase|
|modelPropertySorting|One of legacy, alphabetical, default (only `generichost` library supports this option).| |legacy|
|netCoreProjectFile|Use the new format (.NET Core) for .NET project files (.csproj).| |false|
|nonPublicApi|Generates code with reduced access modifiers; allows embedding elsewhere without exposing non-public API calls to consumers.| |false|
|nullableReferenceTypes|Use nullable annotations in the project. Only supported on C# 8 / ASP.NET Core 3.1 or newer. Starting in .NET 6.0 the default is true.| |false|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,14 @@ public class CSharpClientCodegen extends AbstractCSharpCodegen {
protected boolean nonPublicApi = Boolean.FALSE;

private static final String OPERATION_PARAMETER_SORTING_KEY = "operationParameterSorting";
private static final String MODEL_PROPERTY_SORTING_KEY = "modelPropertySorting";
enum SortingMethod {
DEFAULT,
ALPHABETICAL,
LEGACY
}
private SortingMethod operationParameterSorting = SortingMethod.LEGACY;
private SortingMethod modelPropertySorting = SortingMethod.LEGACY;

protected boolean caseInsensitiveResponseHeaders = Boolean.FALSE;
protected String releaseNote = "Minor update";
Expand Down Expand Up @@ -230,6 +232,10 @@ public CSharpClientCodegen() {
"One of legacy, alphabetical, default (only `generichost` library supports this option).",
this.operationParameterSorting.toString().toLowerCase(Locale.ROOT));

addOption(CSharpClientCodegen.MODEL_PROPERTY_SORTING_KEY,
"One of legacy, alphabetical, default (only `generichost` library supports this option).",
this.modelPropertySorting.toString().toLowerCase(Locale.ROOT));

CliOption framework = new CliOption(
CodegenConstants.DOTNET_FRAMEWORK,
CodegenConstants.DOTNET_FRAMEWORK_DESC
Expand Down Expand Up @@ -465,21 +471,42 @@ public CodegenModel fromModel(String name, Schema model) {
// avoid breaking changes
if (GENERICHOST.equals(getLibrary()) && codegenModel != null) {

Collections.sort(codegenModel.vars, propertyComparatorByName);
Collections.sort(codegenModel.allVars, propertyComparatorByName);
Collections.sort(codegenModel.requiredVars, propertyComparatorByName);
Collections.sort(codegenModel.optionalVars, propertyComparatorByName);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByName);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByName);
Collections.sort(codegenModel.parentVars, propertyComparatorByName);
if (this.modelPropertySorting == SortingMethod.LEGACY) {
Collections.sort(codegenModel.vars, propertyComparatorByName);
Collections.sort(codegenModel.allVars, propertyComparatorByName);
Collections.sort(codegenModel.requiredVars, propertyComparatorByName);
Collections.sort(codegenModel.optionalVars, propertyComparatorByName);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByName);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByName);
Collections.sort(codegenModel.parentVars, propertyComparatorByName);

Collections.sort(codegenModel.vars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.allVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.requiredVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.optionalVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
Collections.sort(codegenModel.parentVars, propertyComparatorByNotNullableRequiredNoDefaultLegacy);
}
else {
if (this.modelPropertySorting == SortingMethod.ALPHABETICAL) {
Collections.sort(codegenModel.vars, propertyComparatorByName);
Collections.sort(codegenModel.allVars, propertyComparatorByName);
Collections.sort(codegenModel.requiredVars, propertyComparatorByName);
Collections.sort(codegenModel.optionalVars, propertyComparatorByName);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByName);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByName);
Collections.sort(codegenModel.parentVars, propertyComparatorByName);
}

Collections.sort(codegenModel.vars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.allVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.requiredVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.optionalVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.parentVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.vars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.allVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.requiredVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.optionalVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.readOnlyVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.readWriteVars, propertyComparatorByNotNullableRequiredNoDefault);
Collections.sort(codegenModel.parentVars, propertyComparatorByNotNullableRequiredNoDefault);
}
}

return codegenModel;
Expand All @@ -492,6 +519,18 @@ public int compare(CodegenProperty one, CodegenProperty another) {
}
};

public static Comparator<CodegenProperty> propertyComparatorByNotNullableRequiredNoDefaultLegacy = new Comparator<CodegenProperty>() {
@Override
public int compare(CodegenProperty one, CodegenProperty another) {
if (one.isNullable == another.isNullable && one.required == another.required && (one.defaultValue == null) == (another.defaultValue == null))
return 0;
else if (!one.isNullable && one.required && one.defaultValue == null)
return -1;
else
return 1;
}
};

public static Comparator<CodegenProperty> propertyComparatorByNotNullableRequiredNoDefault =
Comparator.comparing(p -> p.isNullable || !p.required || p.defaultValue != null);

Expand Down Expand Up @@ -686,6 +725,10 @@ public void processOpts() {
setOperationParameterSorting((String) additionalProperties.get(CSharpClientCodegen.OPERATION_PARAMETER_SORTING_KEY));
}

if (additionalProperties.containsKey(CSharpClientCodegen.MODEL_PROPERTY_SORTING_KEY)) {
setModelPropertySorting((String) additionalProperties.get(CSharpClientCodegen.MODEL_PROPERTY_SORTING_KEY));
}

if (additionalProperties.containsKey(CodegenConstants.API_NAME)) {
setApiName((String) additionalProperties.get(CodegenConstants.API_NAME));
} else {
Expand Down Expand Up @@ -1123,6 +1166,14 @@ public void setOperationParameterSorting(String operationParameterSorting) {
this.operationParameterSorting = SortingMethod.valueOf(operationParameterSorting.toUpperCase(Locale.ROOT));
}

public void setModelPropertySorting(String modelPropertySorting) {
if (modelPropertySorting == null) {
modelPropertySorting = "DEFAULT";
}

this.modelPropertySorting = SortingMethod.valueOf(modelPropertySorting.toUpperCase(Locale.ROOT));
}

public void setSupportsAsync(Boolean supportsAsync) {
this.supportsAsync = supportsAsync;
}
Expand Down

0 comments on commit 3abe251

Please sign in to comment.