Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- fixes an issue where inline response types would collide #2953

Merged
merged 12 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Added support for tracing in Python. [#1872](https://github.com/microsoft/kiota/issues/1872)
- Added auto-generated comment for TypeScript generation. [#3244](https://github.com/microsoft/kiota/issues/3244)
- Added a new switch to exclude all assets generated only for backward compatibility. [#2952](https://github.com/microsoft/kiota/issues/2952)

### Changed

- Updated constructor for request builders in Python to set passed path parameters. [#3352](https://github.com/microsoft/kiota/issues/3352)
- Fixed naming convention for inline response types. [#2952](https://github.com/microsoft/kiota/issues/2952)
- Localhost based descriptions are not cached anymore to facilitate development workflows. [#3316](https://github.com/microsoft/kiota/issues/3316)
- Fixed a bug where the hints would miss quotes for paths and always use the API manifest. [#3342](https://github.com/microsoft/kiota/issues/3342)
- Fixed a bug where inline composed types for components schemas would have the wrong name. [#3067](https://github.com/microsoft/kiota/issues/3067)
Expand Down
4 changes: 2 additions & 2 deletions it/compare-generation.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ else {
$tmpFolder1 = New-TemporaryDirectory
$tmpFolder2 = New-TemporaryDirectory

Start-Process "$kiotaExec" -ArgumentList "generate --clean-output --language ${language} --openapi ${targetOpenapiPath} --dvr all --output $tmpFolder1" -Wait -NoNewWindow
Start-Process "$kiotaExec" -ArgumentList "generate --clean-output --language ${language} --openapi ${targetOpenapiPath} --dvr all --output $tmpFolder2" -Wait -NoNewWindow
Start-Process "$kiotaExec" -ArgumentList "generate --exclude-backward-compatible --clean-output --language ${language} --openapi ${targetOpenapiPath} --dvr all --output $tmpFolder1" -Wait -NoNewWindow
Start-Process "$kiotaExec" -ArgumentList "generate --exclude-backward-compatible --clean-output --language ${language} --openapi ${targetOpenapiPath} --dvr all --output $tmpFolder2" -Wait -NoNewWindow

# Remove variable output files
Remove-Item (Join-Path -Path $tmpFolder1 -ChildPath "kiota-lock.json")
Expand Down
6 changes: 1 addition & 5 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,6 @@
}
],
"IdempotencySuppressions": [
{
"Language": "csharp",
"Rationale": "https://github.com/microsoft/kiota/issues/2952"
},
{
"Language": "go",
"Rationale": "https://github.com/microsoft/kiota/issues/2834"
Expand Down Expand Up @@ -330,4 +326,4 @@
}
]
}
}
}
2 changes: 1 addition & 1 deletion it/generate-code.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ else {
Start-Process "$kiotaExec" -ArgumentList "download ${descriptionUrl} --clean-output --output $targetOpenapiPath" -Wait -NoNewWindow
}

Start-Process "$kiotaExec" -ArgumentList "generate --clean-output --language ${language} --openapi ${targetOpenapiPath}${additionalArguments}" -Wait -NoNewWindow
Start-Process "$kiotaExec" -ArgumentList "generate --exclude-backward-compatible --clean-output --language ${language} --openapi ${targetOpenapiPath}${additionalArguments}" -Wait -NoNewWindow
9 changes: 9 additions & 0 deletions src/Kiota.Builder/Configuration/GenerationConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ public bool UsesBackingStore
{
get; set;
}
public bool ExcludeBackwardCompatible
{
get; set;
}
public bool IncludeBackwardCompatible
{
get => !ExcludeBackwardCompatible;
}
public bool IncludeAdditionalData { get; set; } = true;
public HashSet<string> Serializers
{
Expand Down Expand Up @@ -118,6 +126,7 @@ public object Clone()
{
return new GenerationConfiguration
{
ExcludeBackwardCompatible = ExcludeBackwardCompatible,
OpenAPIFilePath = OpenAPIFilePath,
OutputPath = OutputPath,
ClientClassName = ClientClassName,
Expand Down
66 changes: 61 additions & 5 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,9 +1113,9 @@
IndexParameter = parameter,
}};

if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase))
if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase) && config.IncludeBackwardCompatible)
{ // adding a second indexer for the string version of the parameter so we keep backward compatibility
//TODO remove for v2

Check warning on line 1118 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var backCompatibleValue = (CodeIndexer)result[0].Clone();
backCompatibleValue.Name += "-string";
backCompatibleValue.IndexParameter.Type = DefaultIndexerParameterType;
Expand Down Expand Up @@ -1265,11 +1265,55 @@
executorMethod.AddErrorMapping(errorCode, errorType);
}
}
private CodeTypeBase? GetExecutorMethodReturnType(OpenApiUrlTreeNode currentNode, OpenApiSchema? schema, OpenApiOperation operation, CodeClass parentClass)
private (CodeTypeBase?, CodeTypeBase?) GetExecutorMethodReturnType(OpenApiUrlTreeNode currentNode, OpenApiSchema? schema, OpenApiOperation operation, CodeClass parentClass, OperationType operationType)
{
if (schema != null)
{
return CreateModelDeclarations(currentNode, schema, operation, parentClass, "Response");
var suffix = $"{operationType}Response";
var modelType = CreateModelDeclarations(currentNode, schema, operation, parentClass, suffix);
if (modelType is not null && config.IncludeBackwardCompatible && config.Language is GenerationLanguage.CSharp or GenerationLanguage.Go && modelType.Name.EndsWith(suffix, StringComparison.Ordinal))
{ //TODO remove for v2

Check warning on line 1275 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var obsoleteTypeName = modelType.Name[..^suffix.Length] + "Response";
if (modelType is CodeType codeType &&
codeType.TypeDefinition is CodeClass codeClass)
{
var obsoleteClassDefinition = new CodeClass
{
Kind = CodeClassKind.Model,
Name = obsoleteTypeName,
Deprecation = new($"This class is obsolete. Use {modelType.Name} instead.", IsDeprecated: true),
Documentation = (CodeDocumentation)codeClass.Documentation.Clone()
};
var originalFactoryMethod = codeClass.Methods.First(static x => x.Kind is CodeMethodKind.Factory);
var obsoleteFactoryMethod = (CodeMethod)originalFactoryMethod.Clone();
obsoleteFactoryMethod.ReturnType = new CodeType { Name = obsoleteTypeName, TypeDefinition = obsoleteClassDefinition };
obsoleteClassDefinition.AddMethod(obsoleteFactoryMethod);
obsoleteClassDefinition.StartBlock.Inherits = (CodeType)codeType.Clone();
var obsoleteClass = codeClass.Parent switch
{
CodeClass modelParentClass => modelParentClass.AddInnerClass(obsoleteClassDefinition).First(),
CodeNamespace modelParentNamespace => modelParentNamespace.AddClass(obsoleteClassDefinition).First(),
_ => throw new InvalidOperationException("Could not find a valid parent for the obsolete class")
};
return (modelType, new CodeType
{
TypeDefinition = obsoleteClass,
});
}
else if (modelType is CodeComposedTypeBase codeComposedTypeBase)
{
var obsoleteComposedType = codeComposedTypeBase switch
{
CodeUnionType u => (CodeComposedTypeBase)u.Clone(),
CodeIntersectionType i => (CodeComposedTypeBase)i.Clone(),
_ => throw new InvalidOperationException("Could not create an obsolete composed type"),
};
obsoleteComposedType.Name = obsoleteTypeName;
obsoleteComposedType.Deprecation = new($"This class is obsolete. Use {modelType.Name} instead.", IsDeprecated: true);
return (modelType, obsoleteComposedType);
}
}
return (modelType, null);
}
else
{
Expand All @@ -1280,7 +1324,7 @@
returnType = "string";
else
returnType = "binary";
return new CodeType { Name = returnType, IsExternal = true, };
return (new CodeType { Name = returnType, IsExternal = true, }, null);
}
}
private void CreateOperationMethods(OpenApiUrlTreeNode currentNode, OperationType operationType, OpenApiOperation operation, CodeClass parentClass)
Expand All @@ -1301,6 +1345,7 @@
var schema = operation.GetResponseSchema(config.StructuredMimeTypes);
var method = (HttpMethod)Enum.Parse(typeof(HttpMethod), operationType.ToString());
var deprecationInformation = operation.GetDeprecationInformation();
var returnTypes = GetExecutorMethodReturnType(currentNode, schema, operation, parentClass, operationType);
var executorMethod = new CodeMethod
{
Name = operationType.ToString(),
Expand All @@ -1316,7 +1361,7 @@
operation.Summary)
.CleanupDescription(),
},
ReturnType = GetExecutorMethodReturnType(currentNode, schema, operation, parentClass) ?? throw new InvalidSchemaException(),
ReturnType = returnTypes.Item1 ?? throw new InvalidSchemaException(),
Deprecation = deprecationInformation,
};

Expand Down Expand Up @@ -1347,6 +1392,17 @@
Type = new CodeType { Name = "CancellationToken", IsExternal = true },
};
executorMethod.AddParameter(cancellationParam);// Add cancellation token parameter

if (returnTypes.Item2 is not null && config.IncludeBackwardCompatible)
{ //TODO remove for v2

Check warning on line 1397 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var additionalExecutorMethod = (CodeMethod)executorMethod.Clone();
additionalExecutorMethod.ReturnType = returnTypes.Item2;
additionalExecutorMethod.OriginalMethod = executorMethod;
var newName = $"{executorMethod.Name}As{executorMethod.ReturnType.Name.ToFirstCharacterUpperCase()}";
additionalExecutorMethod.Deprecation = new($"This method is obsolete. Use {newName} instead.", IsDeprecated: true);
parentClass.RenameChildElement(executorMethod.Name, newName);
parentClass.AddMethod(additionalExecutorMethod);
}
logger.LogTrace("Creating method {Name} of {Type}", executorMethod.Name, executorMethod.ReturnType);

var generatorMethod = new CodeMethod
Expand Down
9 changes: 9 additions & 0 deletions src/Kiota.Builder/Lock/KiotaLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public bool UsesBackingStore
get; set;
}
/// <summary>
/// Whether backward compatible code was excluded for this client.
/// </summary>
public bool ExcludeBackwardCompatible
{
get; set;
}
/// <summary>
/// Whether additional data was used for this client.
/// </summary>
public bool IncludeAdditionalData
Expand Down Expand Up @@ -89,6 +96,7 @@ public void UpdateGenerationConfigurationFromLock(GenerationConfiguration config
if (Enum.TryParse<GenerationLanguage>(Language, out var parsedLanguage))
config.Language = parsedLanguage;
config.UsesBackingStore = UsesBackingStore;
config.ExcludeBackwardCompatible = ExcludeBackwardCompatible;
config.IncludeAdditionalData = IncludeAdditionalData;
config.Serializers = Serializers;
config.Deserializers = Deserializers;
Expand All @@ -115,6 +123,7 @@ public KiotaLock(GenerationConfiguration config)
ClientClassName = config.ClientClassName;
ClientNamespaceName = config.ClientNamespaceName;
UsesBackingStore = config.UsesBackingStore;
ExcludeBackwardCompatible = config.ExcludeBackwardCompatible;
IncludeAdditionalData = config.IncludeAdditionalData;
Serializers = config.Serializers;
Deserializers = config.Deserializers;
Expand Down
17 changes: 9 additions & 8 deletions src/Kiota.Builder/Lock/KiotaLockComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ public int GetHashCode([DisallowNull] KiotaLock obj)
{
if (obj == null) return 0;
return
_stringIEnumerableDeepComparer.GetHashCode(obj.DisabledValidationRules?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty<string>()) * 47 +
obj.KiotaVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 43 +
obj.LockFileVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 41 +
(string.IsNullOrEmpty(obj.DescriptionLocation) ? 0 : obj.DescriptionLocation.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 37 +
(string.IsNullOrEmpty(obj.DescriptionHash) ? 0 : obj.DescriptionHash.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 31 +
(string.IsNullOrEmpty(obj.ClientClassName) ? 0 : obj.ClientClassName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 29 +
(string.IsNullOrEmpty(obj.ClientNamespaceName) ? 0 : obj.ClientNamespaceName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 23 +
(string.IsNullOrEmpty(obj.Language) ? 0 : obj.Language.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 19 +
_stringIEnumerableDeepComparer.GetHashCode(obj.DisabledValidationRules?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty<string>()) * 53 +
obj.KiotaVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 47 +
obj.LockFileVersion.GetHashCode(StringComparison.OrdinalIgnoreCase) * 43 +
(string.IsNullOrEmpty(obj.DescriptionLocation) ? 0 : obj.DescriptionLocation.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 41 +
(string.IsNullOrEmpty(obj.DescriptionHash) ? 0 : obj.DescriptionHash.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 37 +
(string.IsNullOrEmpty(obj.ClientClassName) ? 0 : obj.ClientClassName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 31 +
(string.IsNullOrEmpty(obj.ClientNamespaceName) ? 0 : obj.ClientNamespaceName.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 29 +
(string.IsNullOrEmpty(obj.Language) ? 0 : obj.Language.GetHashCode(StringComparison.OrdinalIgnoreCase)) * 23 +
obj.ExcludeBackwardCompatible.GetHashCode() * 19 +
obj.UsesBackingStore.GetHashCode() * 17 +
obj.IncludeAdditionalData.GetHashCode() * 13 +
_stringIEnumerableDeepComparer.GetHashCode(obj.Serializers?.Order(StringComparer.OrdinalIgnoreCase) ?? Enumerable.Empty<string>()) * 11 +
Expand Down
33 changes: 17 additions & 16 deletions src/Kiota.Builder/Refiners/CSharpRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,23 @@
IsExternal = true
}
});
//TODO uncomment on the next major version
// RemoveRequestConfigurationClasses(generatedCode,
// new CodeUsing
// {
// Name = "RequestConfiguration",
// Declaration = new CodeType
// {
// Name = AbstractionsNamespaceName,
// IsExternal = true
// }
// },
// new CodeType
// {
// Name = "DefaultQueryParameters",
// IsExternal = true,
// });
//TODO remove the condition for v2

Check warning on line 34 in src/Kiota.Builder/Refiners/CSharpRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
if (_configuration.ExcludeBackwardCompatible)
RemoveRequestConfigurationClasses(generatedCode,
new CodeUsing
{
Name = "RequestConfiguration",
Declaration = new CodeType
{
Name = AbstractionsNamespaceName,
IsExternal = true
}
},
new CodeType
{
Name = "DefaultQueryParameters",
IsExternal = true,
});
AddDefaultImports(generatedCode, defaultUsingEvaluators);
MoveClassesWithNamespaceNamesUnderNamespace(generatedCode);
ConvertUnionTypesToWrapper(generatedCode,
Expand Down
2 changes: 1 addition & 1 deletion src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public override void WriteCodeElement(CodeMethod codeElement, LanguageWriter wri
case CodeMethodKind.RequestGenerator when !codeElement.IsOverload:
WriteRequestGeneratorBody(codeElement, requestParams, writer, parentClass);
break;
case CodeMethodKind.RequestExecutor when !codeElement.IsOverload:
case CodeMethodKind.RequestExecutor when !codeElement.IsOverload || (codeElement.Deprecation?.IsDeprecated ?? false): //TODO remove deprecation condition for v2
WriteRequestExecutorBody(codeElement, requestParams, returnType, parentClass, writer);
break;
case CodeMethodKind.Getter:
Expand Down
7 changes: 7 additions & 0 deletions src/kiota/Handlers/KiotaGenerationCommandHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
string openapi = context.ParseResult.GetValueForOption(DescriptionOption) ?? string.Empty;
string manifest = context.ParseResult.GetValueForOption(ManifestOption) ?? string.Empty;
bool backingStore = context.ParseResult.GetValueForOption(BackingStoreOption);
bool excludeBackwardCompatible = context.ParseResult.GetValueForOption(ExcludeBackwardCompatibleOption);
bool clearCache = context.ParseResult.GetValueForOption(ClearCacheOption);
bool includeAdditionalData = context.ParseResult.GetValueForOption(AdditionalDataOption);
string className = context.ParseResult.GetValueForOption(ClassOption) ?? string.Empty;
Expand All @@ -89,6 +90,7 @@ public override async Task<int> InvokeAsync(InvocationContext context)
AssignIfNotNullOrEmpty(className, (c, s) => c.ClientClassName = s);
AssignIfNotNullOrEmpty(namespaceName, (c, s) => c.ClientNamespaceName = s);
Configuration.Generation.UsesBackingStore = backingStore;
Configuration.Generation.ExcludeBackwardCompatible = excludeBackwardCompatible;
Configuration.Generation.IncludeAdditionalData = includeAdditionalData;
Configuration.Generation.Language = language;
if (serializer.Any())
Expand Down Expand Up @@ -167,4 +169,9 @@ public required Option<string> ManifestOption
{
get; init;
}
public required Option<bool> ExcludeBackwardCompatibleOption
{
get;
set;
}
}
Loading
Loading