diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index af6046f4b4..c3ae9d6b9b 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -49,6 +49,7 @@ jobs: - "./tests/Kiota.Builder.IntegrationTests/NoUnderscoresInModel.yaml" - "./tests/Kiota.Builder.IntegrationTests/ToDoApi.yaml" - "./tests/Kiota.Builder.IntegrationTests/GeneratesUritemplateHints.yaml" + - "./tests/Kiota.Builder.IntegrationTests/DiscriminatorSample.yaml" - "oas::petstore" - "apisguru::twitter.com:current" - "apisguru::notion.com" diff --git a/CHANGELOG.md b/CHANGELOG.md index d29ee7d31b..6024f73168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Changed Csharp code generation to put braces on new lines (where it makes sense). [#4347](https://github.com/microsoft/kiota/issues/4347) - Fixed a bug where some no-content status codes would be considered structured (301, 302, 303, 307) when described. [#4190](https://github.com/microsoft/kiota/issues/4190) - TypeScript is now a preview language! +- Fixed a bug where the code generated for Python in presence of discriminator values was not compiling/working. [#4433](https://github.com/microsoft/kiota/issues/4433) ## [1.12.0] - 2024-03-06 diff --git a/it/config.json b/it/config.json index bef7691eac..2fec4e43a3 100644 --- a/it/config.json +++ b/it/config.json @@ -53,6 +53,19 @@ } ] }, + "./tests/Kiota.Builder.IntegrationTests/DiscriminatorSample.yaml": { + "MockServerITFolder": "discriminator", + "Suppressions": [ + { + "Language": "ruby", + "Rationale": "https://github.com/microsoft/kiota/issues/2484" + }, + { + "Language": "typescript", + "Rationale": "https://github.com/microsoft/kiota/issues/4434" + } + ] + }, "./tests/Kiota.Builder.IntegrationTests/InheritingErrors.yaml": { "MockServerITFolder": "basic", "Suppressions": [ @@ -341,9 +354,7 @@ ] }, "apisguru::apis.guru": { - "Suppressions": [ - ], - "IdempotencySuppressions": [ - ] + "Suppressions": [], + "IdempotencySuppressions": [] } -} +} \ No newline at end of file diff --git a/it/python/discriminator/test_serdeser.py b/it/python/discriminator/test_serdeser.py new file mode 100644 index 0000000000..aeaad07d76 --- /dev/null +++ b/it/python/discriminator/test_serdeser.py @@ -0,0 +1,56 @@ +import pytest + +from kiota_serialization_json.json_parse_node_factory import JsonParseNodeFactory +from kiota_serialization_json.json_serialization_writer_factory import JsonSerializationWriterFactory + +from client.models.component import Component +from client.models.component1 import Component1 +from client.models.component2 import Component2 + +@pytest.mark.asyncio +async def test_component1_deser(): + factory = JsonParseNodeFactory() + root = factory.get_root_parse_node('application/json', '{"objectType": "obj1", "one": "foo"}'.encode('utf-8')) + result = root.get_object_value(Component) + assert hasattr(result, "component1") + assert not hasattr(result, "component2") + assert isinstance(result.component1, Component1) + assert result.component1.object_type == "obj1" + assert result.component1.one == "foo" + +@pytest.mark.asyncio +async def test_component2_deser(): + factory = JsonParseNodeFactory() + root = factory.get_root_parse_node('application/json', '{"objectType": "obj2", "two": "bar"}'.encode('utf-8')) + result = root.get_object_value(Component) + assert hasattr(result, "component2") + assert not hasattr(result, "component1") + assert isinstance(result.component2, Component2) + assert result.component2.object_type == "obj2" + assert result.component2.two == "bar" + +@pytest.mark.asyncio +async def test_component1_ser(): + obj = Component() + obj1 = Component1() + obj1.object_type = "obj1" + obj1.one = "foo" + obj.component1 = obj1 + factory = JsonSerializationWriterFactory() + writer = factory.get_serialization_writer('application/json') + obj.serialize(writer) + content = writer.get_serialized_content().decode('utf-8') + assert content == '{"objectType": "obj1", "one": "foo"}' + +@pytest.mark.asyncio +async def test_component2_ser(): + obj = Component() + obj2 = Component2() + obj2.object_type = "obj2" + obj2.two = "bar" + obj.component2 = obj2 + factory = JsonSerializationWriterFactory() + writer = factory.get_serialization_writer('application/json') + obj.serialize(writer) + content = writer.get_serialized_content().decode('utf-8') + assert content == '{"objectType": "obj2", "two": "bar"}' diff --git a/it/python/requirements-dev.txt b/it/python/requirements-dev.txt index a54a2cbd60..2f992a5a1b 100644 --- a/it/python/requirements-dev.txt +++ b/it/python/requirements-dev.txt @@ -134,7 +134,5 @@ six==1.16.0 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2' sniffio==1.3.1 ; python_version >= '3.7' -uritemplate==4.1.1 ; python_version >= '3.6' - yarl==1.9.4 ; python_version >= '3.7' diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs index 41a41061c2..89fec99333 100644 --- a/src/Kiota.Builder/CodeDOM/CodeClass.cs +++ b/src/Kiota.Builder/CodeDOM/CodeClass.cs @@ -77,8 +77,15 @@ public override IEnumerable AddProperty(params CodeProperty[] prop if (properties.Length == 0) throw new ArgumentOutOfRangeException(nameof(properties)); - return properties.Select(property => + return properties.GroupBy(static x => x.Name, StringComparer.Ordinal).Select(static x => x.First()).Select(property => { + // Prevent duplicates + var otherProp = FindPropertyByNameInTypeHierarchy(property.Name); + if (otherProp != null && otherProp.WireName.Equals(property.WireName, StringComparison.OrdinalIgnoreCase)) + { + return null; // this property is going to collide, skipping + } + if (property.IsOfKind(CodePropertyKind.Custom, CodePropertyKind.QueryParameter)) { if (GetOriginalPropertyDefinedFromBaseType(property.WireName) is CodeProperty original) @@ -98,7 +105,7 @@ public override IEnumerable AddProperty(params CodeProperty[] prop } var result = base.AddProperty(property).First(); return PropertiesByWireName.GetOrAdd(result.WireName, result); - }).ToArray(); + }).OfType().ToArray(); } public override void RenameChildElement(string oldName, string newName) { diff --git a/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs index 67eee514f2..fead21630b 100644 --- a/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs +++ b/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs @@ -137,7 +137,14 @@ private void WriteFactoryMethodBodyForInheritedModel(CodeClass parentClass, Lang private const string ResultVarName = "result"; private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer) { - writer.WriteLine($"{ResultVarName} = {parentClass.Name}()"); + var className = parentClass.Name; + if (parentClass.Parent is CodeClass pc && + !string.IsNullOrEmpty(pc.Name) && + pc.InnerClasses.Any(x => x == parentClass)) + { + className = $"{pc.Name}.{parentClass.Name}"; + } + writer.WriteLine($"{ResultVarName} = {className}()"); var includeElse = false; foreach (var property in parentClass.GetPropertiesOfKind(CodePropertyKind.Custom) .OrderBy(static x => x, CodePropertyTypeForwardComparer) @@ -508,7 +515,7 @@ private void WriteDeserializerBodyForUnionModel(CodeMethod method, CodeClass par .ThenBy(static x => x.Name) .Select(static x => x.Name)) { - writer.StartBlock($"if self.{otherPropName}:"); + writer.StartBlock($"if hasattr(self, \"{otherPropName}\"):"); writer.WriteLine($"return self.{otherPropName}.{method.Name}()"); writer.DecreaseIndent(); } @@ -659,7 +666,7 @@ private void WriteSerializerBodyForUnionModel(CodeClass parentClass, LanguageWri .OrderBy(static x => x, CodePropertyTypeForwardComparer) .ThenBy(static x => x.Name)) { - writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if self.{otherProp.Name}:"); + writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if hasattr(self, \"{otherProp.Name}\"):"); writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}(None, self.{otherProp.Name})"); writer.DecreaseIndent(); if (!includeElse) @@ -676,7 +683,7 @@ private void WriteSerializerBodyForIntersectionModel(CodeClass parentClass, Lang .OrderBy(static x => x, CodePropertyTypeBackwardComparer) .ThenBy(static x => x.Name)) { - writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if self.{otherProp.Name}:"); + writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if hasattr(self, \"{otherProp.Name}\"):"); writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}(None, self.{otherProp.Name})"); writer.DecreaseIndent(); if (!includeElse) diff --git a/tests/Kiota.Builder.IntegrationTests/DiscriminatorSample.yaml b/tests/Kiota.Builder.IntegrationTests/DiscriminatorSample.yaml new file mode 100644 index 0000000000..1169b5707a --- /dev/null +++ b/tests/Kiota.Builder.IntegrationTests/DiscriminatorSample.yaml @@ -0,0 +1,51 @@ +openapi: 3.0.0 +info: + title: "Discriminator API" + version: "1.0.0" +servers: + - url: https://mytodos.doesnotexist/ +paths: + /discriminateme: + post: + description: Return something + responses: + "200": + description: OK + content: + application/json: + schema: + $ref: "#/components/schemas/Component" + requestBody: + content: + application/json: + schema: + $ref: "#/components/schemas/Component" +components: + schemas: + Component: + oneOf: + - $ref: "#/components/schemas/Component1" + - $ref: "#/components/schemas/Component2" + discriminator: + propertyName: objectType + mapping: + obj1: "#/components/schemas/Component1" + obj2: "#/components/schemas/Component2" + Component1: + type: object + required: + - objectType + properties: + objectType: + type: string + one: + type: string + Component2: + type: object + required: + - objectType + properties: + objectType: + type: string + two: + type: string diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs index a1d78fd859..d1d7a41037 100644 --- a/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs @@ -154,10 +154,8 @@ public void DoesntWritePropertiesExistingInParentType() Name = "string" }, Kind = CodePropertyKind.Custom, - }).First(); - writer.Write(propertyToWrite); - var result = tw.ToString(); - Assert.Empty(result); + }); + Assert.Empty(propertyToWrite); } [Fact] diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs index fb18a5651e..5816005456 100644 --- a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs @@ -811,7 +811,7 @@ public void WritesUnionDeSerializerBody() Assert.DoesNotContain("super_fields = super()", result); Assert.DoesNotContain("return fields", result); Assert.DoesNotContain("elif", result); - Assert.Contains("if self.complex_type1_value:", result); + Assert.Contains("if hasattr(self, \"complex_type1_value\"):", result); Assert.Contains("return self.complex_type1_value.get_field_deserializers()", result); Assert.Contains("return {}", result); } @@ -907,11 +907,11 @@ public void WritesUnionSerializerBody() writer.Write(serializationMethod); var result = tw.ToString(); Assert.DoesNotContain("super().serialize", result); - Assert.Contains("if self.complex_type1_value:", result); + Assert.Contains("if hasattr(self, \"complex_type1_value\"):", result); Assert.Contains("writer.write_object_value(None, self.complex_type1_value)", result); - Assert.Contains("if self.string_value:", result); + Assert.Contains("if hasattr(self, \"string_value\"):", result); Assert.Contains("writer.write_str_value(None, self.string_value)", result); - Assert.Contains("if self.complex_type2_value:", result); + Assert.Contains("if hasattr(self, \"complex_type2_value\"):", result); Assert.Contains("writer.write_collection_of_object_values(None, self.complex_type2_value)", result); } [Fact] @@ -941,11 +941,11 @@ public void WritesIntersectionSerializerBody() writer.Write(serializationMethod); var result = tw.ToString(); Assert.DoesNotContain("super().serialize", result); - Assert.DoesNotContain("if self.complex_type1_value:", result); + Assert.DoesNotContain("if hasattr(self, \"complex_type1_value\"):", result); Assert.Contains("writer.write_object_value(None, self.complex_type1_value, self.complex_type3_value)", result); - Assert.Contains("if self.string_value:", result); + Assert.Contains("if hasattr(self, \"string_value\"):", result); Assert.Contains("writer.write_str_value(None, self.string_value)", result); - Assert.Contains("if self.complex_type2_value:", result); + Assert.Contains("if hasattr(self, \"complex_type2_value\"):", result); Assert.Contains("writer.write_collection_of_object_values(None, self.complex_type2_value)", result); } [Fact]