From 94de6d3f75fdb47589f4879c24281aa60bf11681 Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Tue, 23 Mar 2021 10:41:52 -0700 Subject: [PATCH 1/8] add parameter to control if Nullable gets written always --- .../EdmModelCsdlSchemaJsonWriter.cs | 12 +++++-- .../Serialization/EdmModelCsdlSchemaWriter.cs | 2 +- .../EdmModelCsdlSchemaXmlWriter.cs | 11 ++++-- .../EdmModelCsdlSerializationVisitor.cs | 4 +-- .../EdmModelCsdlSerializationVisitorTests.cs | 34 +++++++++++++++++++ 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs index d437d42e23..c2c046c4dc 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs @@ -384,10 +384,18 @@ internal override void WriteEnumMemberElementHeader(IEdmEnumMember member) /// 7.2.1 Nullable, A Boolean value specifying whether a value is required for the property. /// /// The Edm type reference. - internal override void WriteNullableAttribute(IEdmTypeReference reference) + /// Specifies if the attribute is always written or not if the value is the default value. + internal override void WriteNullableAttribute(IEdmTypeReference reference, bool alwaysWrite) { // The value of $Nullable is one of the Boolean literals true or false. Absence of the member means false. - this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); + if(alwaysWrite) + { + this.jsonWriter.WriteRequiredProperty("$Nullable", reference.IsNullable); + } + else + { + this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); + } } internal override void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs index 0c3f1f771d..7b85ddbbf4 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs @@ -105,7 +105,7 @@ internal virtual void WriteNavigationPropertyBindingsEnd(IEnumerable v.VisitSchemaType(structuredType), @" + + + + +" +); + + // Act & Assert for JSON + VisitAndVerifyJson(v => v.VisitSchemaType(structuredType), @"{ + ""Customer"": { + ""$Kind"": ""ComplexType"", + ""test01"": { ""$Type"": ""Edm.Decimal"", ""$Collection"": true, ""$Nullable"": true }, + ""test02"": { ""$Type"": ""Edm.Decimal"", ""$Collection"": true, ""$Nullable"": false }, + ""test03"": { ""$Type"": ""Edm.Decimal"", ""$Nullable"": true }, + ""test04"": { ""$Type"": ""Edm.Decimal"", ""$Nullable"": false }, + } +}"); + } + #endregion + internal void VisitAndVerifyXml(Action testAction, string expected, bool indent = true) { XmlWriter xmlWriter; From 0fda56153802280985735158a35bcad93ced01e9 Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Wed, 24 Mar 2021 08:23:58 -0700 Subject: [PATCH 2/8] added Nullable=""true"" to existing test's data. --- .../Csdl/CsdlWriterTests.cs | 10 +++++----- .../EdmModelCsdlSerializationVisitorTests.cs | 10 +++++----- ...sFunctionsRelationshipChangesAcceptanceTests.cs | 10 +++++----- .../Vocabularies/AlternateKeysVocabularyTests.cs | 4 ++-- .../Vocabularies/CapabilitiesVocabularyTests.cs | 14 +++++++------- .../Vocabularies/ValidationVocabularyTests.cs | 4 ++-- 6 files changed, 26 insertions(+), 26 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs index 8e2d314e66..757356ea35 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs @@ -1421,7 +1421,7 @@ public void ShouldWriteEdmPathTypeProperty() "" + "" + "" + - "" + + "" + "" + "" + "" + @@ -1604,8 +1604,8 @@ public void CanWriteEdmEntityTypeWithCollectionAbstractTypeButValidationFailed() "" + "" + "" + - "" + - "" + + "" + + "" + "" + "" + "" + @@ -1730,7 +1730,7 @@ public void CanWriteEdmFunctioneWithCollectionAbstractTypeButValidationFailed() "" + "" + "" + - "" + + "" + "" + "" + "" + @@ -2377,7 +2377,7 @@ public void CanWriteEdmModelWithUntypedProperty() "" + "" + "" + - "" + + "" + "" + "" + "" + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs index 4838b1f56f..bcb368bf19 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs @@ -93,7 +93,7 @@ public void VerifyComplexTypeWithNavigationPropertiesWrittenCorrectly() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitSchemaType(complexType), @" - + @@ -367,7 +367,7 @@ public void VerifyPrimitiveCollectionReturnTypeDefinedInChildReturnTypeElement() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitSchemaElement(action), - @"", false); + @"", false); // Act & Assert for JSON VisitAndVerifyJson(v => v.VisitSchemaElement(action), @"{ @@ -624,7 +624,7 @@ public void FunctionShouldWriteOutCorrectly() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitSchemaElement(function), @" - + "); // Act & Assert for JSON @@ -649,7 +649,7 @@ public void VerifyFunctionWrittenCorrectly() VisitAndVerifyXml(v => v.VisitSchemaElement(function), @" - + "); @@ -1891,7 +1891,7 @@ public void VerifyAnnotationWithIsOfThenElseExpressionWrittenCorrectly() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitVocabularyAnnotation(annotation), @" - + Customer "); diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs index 3d6cfe894a..bbff554ced 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs @@ -42,29 +42,29 @@ public class DefaultTestModel - + - + - + - - + + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs index b28c58e501..c6c4c8189c 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs @@ -27,7 +27,7 @@ public void TestAlternateKeysVocabularyModel() const string expectedText = @" - + @@ -39,7 +39,7 @@ public void TestAlternateKeysVocabularyModel() - + "; diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs index 3766a956b4..d620e331f3 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs @@ -304,7 +304,7 @@ public void TestCapabilitiesVocabularyModel() - + @@ -372,7 +372,7 @@ public void TestCapabilitiesVocabularyModel() - + @@ -417,7 +417,7 @@ public void TestCapabilitiesVocabularyModel() - + @@ -476,7 +476,7 @@ public void TestCapabilitiesVocabularyModel() - + @@ -510,7 +510,7 @@ public void TestCapabilitiesVocabularyModel() - + @@ -716,13 +716,13 @@ public void TestCapabilitiesVocabularyModel() - + - + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs index 487764dd4c..acf6fd9493 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs @@ -93,12 +93,12 @@ public void TestValidationVocabularyModel() - + - + From 74b4a94657e83217de200decab4861835a267c46 Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Wed, 24 Mar 2021 09:07:34 -0700 Subject: [PATCH 3/8] ensure only colelction property require teh nullable attribute --- .../EdmModelCsdlSerializationVisitor.cs | 25 +++++++++++-------- .../Csdl/CsdlWriterTests.cs | 2 +- .../EdmModelCsdlSerializationVisitorTests.cs | 8 +++--- ...tionsRelationshipChangesAcceptanceTests.cs | 10 ++++---- .../AlternateKeysVocabularyTests.cs | 2 +- .../CapabilitiesVocabularyTests.cs | 4 +-- .../Vocabularies/ValidationVocabularyTests.cs | 4 +-- 7 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs index ded4ab578f..cabbf2d0f7 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs @@ -156,7 +156,10 @@ protected override void ProcessEntityType(IEdmEntityType element) protected override void ProcessStructuralProperty(IEdmStructuralProperty element) { bool inlineType = IsInlineType(element.Type); - this.BeginElement(element, (IEdmStructuralProperty t) => { this.schemaWriter.WriteStructuralPropertyElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType); }); + this.BeginElement(element, + (IEdmStructuralProperty t) => { this.schemaWriter.WriteStructuralPropertyElementHeader(t, inlineType); }, + e => { this.ProcessFacets(e.Type, inlineType, true); } + ); if (!inlineType) { VisitTypeReference(element.Type); @@ -243,7 +246,7 @@ protected override void ProcessTypeDefinition(IEdmTypeDefinition element) protected override void ProcessTerm(IEdmTerm term) { bool inlineType = term.Type != null && IsInlineType(term.Type); - this.BeginElement(term, (IEdmTerm t) => { this.schemaWriter.WriteTermElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType); }); + this.BeginElement(term, (IEdmTerm t) => { this.schemaWriter.WriteTermElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType, false); }); if (!inlineType) { if (term.Type != null) @@ -271,7 +274,7 @@ protected override void ProcessOperationParameter(IEdmOperationParameter element this.BeginElement( element, (IEdmOperationParameter t) => { this.schemaWriter.WriteOperationParameterElementHeader(t, inlineType); }, - e => { this.ProcessFacets(e.Type, inlineType); }); + e => { this.ProcessFacets(e.Type, inlineType, false); }); if (!inlineType) { VisitTypeReference(element.Type); @@ -303,7 +306,7 @@ protected override void ProcessOperationReturn(IEdmOperationReturn operationRetu if (inlineType) { this.schemaWriter.WriteTypeAttribute(type); - this.ProcessFacets(type, true /*inlineType*/); + this.ProcessFacets(type, true /*inlineType*/, false); } else { @@ -319,7 +322,7 @@ protected override void ProcessCollectionType(IEdmCollectionType element) this.BeginElement( element, (IEdmCollectionType t) => this.schemaWriter.WriteCollectionTypeElementHeader(t, inlineType), - e => this.ProcessFacets(e.ElementType, inlineType)); + e => this.ProcessFacets(e.ElementType, inlineType, false)); if (!inlineType) { VisitTypeReference(element.ElementType); @@ -439,7 +442,7 @@ protected override void ProcessIsTypeExpression(IEdmIsTypeExpression expression) if (this.isXml) { - this.BeginElement(expression, (IEdmIsTypeExpression t) => { this.schemaWriter.WriteIsTypeExpressionElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType); }); + this.BeginElement(expression, (IEdmIsTypeExpression t) => { this.schemaWriter.WriteIsTypeExpressionElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType, false); }); if (!inlineType) { VisitTypeReference(expression.Type); @@ -453,7 +456,7 @@ protected override void ProcessIsTypeExpression(IEdmIsTypeExpression expression) this.BeginElement(expression, (IEdmIsTypeExpression t) => { this.schemaWriter.WriteIsTypeExpressionElementHeader(t, inlineType); }); this.VisitExpression(expression.Operand); this.schemaWriter.WriteIsOfExpressionType(expression, inlineType); - this.ProcessFacets(expression.Type, inlineType); + this.ProcessFacets(expression.Type, inlineType, false); this.EndElement(expression); } } @@ -533,7 +536,7 @@ protected override void ProcessCastExpression(IEdmCastExpression expression) if (this.isXml) { - this.BeginElement(expression, (IEdmCastExpression t) => { this.schemaWriter.WriteCastExpressionElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType); }); + this.BeginElement(expression, (IEdmCastExpression t) => { this.schemaWriter.WriteCastExpressionElementHeader(t, inlineType); }, e => { this.ProcessFacets(e.Type, inlineType, false); }); if (!inlineType) { VisitTypeReference(expression.Type); @@ -549,7 +552,7 @@ protected override void ProcessCastExpression(IEdmCastExpression expression) this.VisitExpression(expression.Operand); this.schemaWriter.WriteCastExpressionType(expression, inlineType); - this.ProcessFacets(expression.Type, inlineType); + this.ProcessFacets(expression.Type, inlineType, false); this.EndElement(expression, t => this.schemaWriter.WriteCastExpressionElementEnd(t, inlineType)); } @@ -653,7 +656,7 @@ private void ProcessReferentialConstraint(IEdmReferentialConstraint element) } } - private void ProcessFacets(IEdmTypeReference element, bool inlineType) + private void ProcessFacets(IEdmTypeReference element, bool inlineType, bool nullableAttributeRequiredForCollection) { if (element != null) { @@ -668,7 +671,7 @@ private void ProcessFacets(IEdmTypeReference element, bool inlineType) if (element.TypeKind() == EdmTypeKind.Collection) { IEdmCollectionTypeReference collectionElement = element.AsCollection(); - this.schemaWriter.WriteNullableAttribute(collectionElement.CollectionDefinition().ElementType, true); + this.schemaWriter.WriteNullableAttribute(collectionElement.CollectionDefinition().ElementType, nullableAttributeRequiredForCollection); VisitTypeReference(collectionElement.CollectionDefinition().ElementType); } else diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs index 757356ea35..3d488facd5 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs @@ -1730,7 +1730,7 @@ public void CanWriteEdmFunctioneWithCollectionAbstractTypeButValidationFailed() "" + "" + "" + - "" + + "" + "" + "" + "" + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs index bcb368bf19..2c6ed783d2 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs @@ -367,7 +367,7 @@ public void VerifyPrimitiveCollectionReturnTypeDefinedInChildReturnTypeElement() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitSchemaElement(action), - @"", false); + @"", false); // Act & Assert for JSON VisitAndVerifyJson(v => v.VisitSchemaElement(action), @"{ @@ -624,7 +624,7 @@ public void FunctionShouldWriteOutCorrectly() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitSchemaElement(function), @" - + "); // Act & Assert for JSON @@ -649,7 +649,7 @@ public void VerifyFunctionWrittenCorrectly() VisitAndVerifyXml(v => v.VisitSchemaElement(function), @" - + "); @@ -1891,7 +1891,7 @@ public void VerifyAnnotationWithIsOfThenElseExpressionWrittenCorrectly() // Act & Assert for XML VisitAndVerifyXml(v => v.VisitVocabularyAnnotation(annotation), @" - + Customer "); diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs index bbff554ced..3d6cfe894a 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/ScenarioTests/OasisActionsFunctionsRelationshipChangesAcceptanceTests.cs @@ -42,29 +42,29 @@ public class DefaultTestModel - + - + - + - - + + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs index c6c4c8189c..0b678cb01c 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs @@ -39,7 +39,7 @@ public void TestAlternateKeysVocabularyModel() - + "; diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs index d620e331f3..e30718f853 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs @@ -716,13 +716,13 @@ public void TestCapabilitiesVocabularyModel() - + - + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs index acf6fd9493..487764dd4c 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/ValidationVocabularyTests.cs @@ -93,12 +93,12 @@ public void TestValidationVocabularyModel() - + - + From 8be4c66803d1639421f49e0d75a4f415757a82ec Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Mon, 29 Mar 2021 16:18:57 -0700 Subject: [PATCH 4/8] fix expected JSON CSDL values --- .../Csdl/CsdlWriterTests.cs | 9 ++++--- .../EdmModelCsdlSerializationVisitorTests.cs | 24 +++++++++++++++---- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs index 3d488facd5..55c554ac74 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs @@ -311,7 +311,8 @@ public void WriteNavigationPropertyInComplexType() }, ""Addresses"": { ""$Collection"": true, - ""$Type"": ""DefaultNs.Address"" + ""$Type"": ""DefaultNs.Address"", + ""$Nullable"": false } }, ""City"": { @@ -745,7 +746,8 @@ public void SetNavigationPropertyPartnerTest() }, ""ComplexProp"": { ""$Collection"": true, - ""$Type"": ""NS.ComplexType1"" + ""$Type"": ""NS.ComplexType1"", + ""$Nullable"": false }, ""OuterNavA"": { ""$Kind"": ""NavigationProperty"", @@ -1442,7 +1444,8 @@ public void ShouldWriteEdmPathTypeProperty() }, ""DefaultHidden"": { ""$Collection"": true, - ""$Type"": ""Edm.NavigationPropertyPath"" + ""$Type"": ""Edm.NavigationPropertyPath"", + ""$Nullable"": false } }, ""MyTerm"": { diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs index 2c6ed783d2..afb1e2abf3 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/Serialization/EdmModelCsdlSerializationVisitorTests.cs @@ -216,7 +216,8 @@ public void VerifyEntityTypeWithNavigationPropertiesWrittenCorrectly() ""$Kind"": ""EntityType"", ""Locations"": { ""$Collection"": true, - ""$Type"": ""NS.Address"" + ""$Type"": ""NS.Address"", + ""$Nullable"": false }, ""Orders"": { ""$Kind"": ""NavigationProperty"", @@ -2040,10 +2041,23 @@ public void VerifyCollectionTypeReferencesWrittenCorrectly() VisitAndVerifyJson(v => v.VisitSchemaType(structuredType), @"{ ""Customer"": { ""$Kind"": ""ComplexType"", - ""test01"": { ""$Type"": ""Edm.Decimal"", ""$Collection"": true, ""$Nullable"": true }, - ""test02"": { ""$Type"": ""Edm.Decimal"", ""$Collection"": true, ""$Nullable"": false }, - ""test03"": { ""$Type"": ""Edm.Decimal"", ""$Nullable"": true }, - ""test04"": { ""$Type"": ""Edm.Decimal"", ""$Nullable"": false }, + ""test01"": { + ""$Collection"": true, + ""$Type"": ""Edm.Decimal"", + ""$Nullable"": true + }, + ""test02"": { + ""$Collection"": true, + ""$Type"": ""Edm.Decimal"", + ""$Nullable"": false + }, + ""test03"": { + ""$Type"": ""Edm.Decimal"", + ""$Nullable"": true + }, + ""test04"": { + ""$Type"": ""Edm.Decimal"" + } } }"); } From 23de41978c7b1fc25b3023c41c49cd927535559d Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Tue, 30 Mar 2021 09:06:36 -0700 Subject: [PATCH 5/8] fixed EdmLibTests.FunctionalTests.CsdlSerializingTests --- .../FunctionalTests/CsdlSerializingTests.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/FunctionalTests/Tests/DataEdmLib/FunctionalTests/CsdlSerializingTests.cs b/test/FunctionalTests/Tests/DataEdmLib/FunctionalTests/CsdlSerializingTests.cs index c98816164f..b8ef503853 100644 --- a/test/FunctionalTests/Tests/DataEdmLib/FunctionalTests/CsdlSerializingTests.cs +++ b/test/FunctionalTests/Tests/DataEdmLib/FunctionalTests/CsdlSerializingTests.cs @@ -153,11 +153,11 @@ public void SerializeComplexCollectionProperty() - - - - - + + + + + @@ -372,7 +372,7 @@ public void SerializeMultipleFilesDottedNamespace() "; - VerifyRoundTrip(new string[] {inputText1, inputText2}); + VerifyRoundTrip(new string[] { inputText1, inputText2 }); } [TestMethod] @@ -543,7 +543,7 @@ public void FailSerializingTwoSchemasToOneFile() using (XmlWriter xw = XmlWriter.Create(sw, settings)) { - IEnumerable expectedSerializationErrors = new EdmLibTestErrors() + IEnumerable expectedSerializationErrors = new EdmLibTestErrors() { {0, 0, EdmErrorCode.SingleFileExpected}, }; @@ -566,7 +566,7 @@ public void SerializeCollectionReturnTypesRegressionTest() - + "; VerifyRoundTrip(inputText); @@ -699,7 +699,7 @@ public void SerializeTerm() VerifyRoundTrip(inputText); } - + [TestMethod] public void SerializeWithCoreVocabularyTerm() { @@ -1396,10 +1396,10 @@ public void SerializeUsingAlias() ")}; - var actualCsdls = this.GetSerializerResult(this.GetParserResult(expectedCsdls)).Select(n=>XElement.Parse(n)); + var actualCsdls = this.GetSerializerResult(this.GetParserResult(expectedCsdls)).Select(n => XElement.Parse(n)); Assert.IsTrue ( - XElement.DeepEquals( GetCsdl(expectedCsdls, "DefaultNamespace"), GetCsdl(actualCsdls, "DefaultNamespace")) && XElement.DeepEquals( GetCsdl(expectedCsdls, "Org.OData.Display"), GetCsdl(actualCsdls, "Org.OData.Display")), + XElement.DeepEquals(GetCsdl(expectedCsdls, "DefaultNamespace"), GetCsdl(actualCsdls, "DefaultNamespace")) && XElement.DeepEquals(GetCsdl(expectedCsdls, "Org.OData.Display"), GetCsdl(actualCsdls, "Org.OData.Display")), "The CSDLs that the serializer generates is different from the original CSDLs" ); } @@ -1433,7 +1433,7 @@ public void RoundtripModelWithTypeDefinitions() VerifyRoundTrip(csdl); } - [ TestMethod] + [TestMethod] public void RoundtripModelWithTypeDefinitionFacets() { const string csdl = @" From 3d027694193f65e748c90697d940c22883021bfe Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Wed, 11 Aug 2021 16:27:31 -0700 Subject: [PATCH 6/8] move login intto writer. --- .../EdmModelCsdlSchemaJsonWriter.cs | 16 +++++----------- .../Serialization/EdmModelCsdlSchemaWriter.cs | 8 +++++++- .../Serialization/EdmModelCsdlSchemaXmlWriter.cs | 7 +++++-- .../EdmModelCsdlSerializationVisitor.cs | 11 ++++++----- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs index c2c046c4dc..d0d2df6195 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs @@ -384,18 +384,12 @@ internal override void WriteEnumMemberElementHeader(IEdmEnumMember member) /// 7.2.1 Nullable, A Boolean value specifying whether a value is required for the property. /// /// The Edm type reference. - /// Specifies if the attribute is always written or not if the value is the default value. - internal override void WriteNullableAttribute(IEdmTypeReference reference, bool alwaysWrite) + /// The parent object's (the property) type kind. + internal override void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind) { - // The value of $Nullable is one of the Boolean literals true or false. Absence of the member means false. - if(alwaysWrite) - { - this.jsonWriter.WriteRequiredProperty("$Nullable", reference.IsNullable); - } - else - { - this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); - } + // The value of $Nullable is one of the Boolean literals true or false. Absence of the JSON property means false. + // parentTypeKind gets intentionally ignored since JSON CSDL never writes $Nullable=false + this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); } internal override void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs index 7b85ddbbf4..224ed1a39c 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs @@ -105,7 +105,13 @@ internal virtual void WriteNavigationPropertyBindingsEnd(IEnumerable + /// 7.2.1 Nullable, A Boolean value specifying whether a value is required for the property. + /// + /// The Edm type reference. + /// The parent object's (the property's) type kind. + internal abstract void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind); + internal abstract void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference); diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs index 57b70e3c7e..43c5b92fee 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs @@ -223,9 +223,12 @@ internal override void WriteEnumMemberElementEnd(IEdmEnumMember member) this.xmlWriter.WriteEndElement(); } - internal override void WriteNullableAttribute(IEdmTypeReference reference, bool alwaysWrite) + /// + internal override void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind) { - if (alwaysWrite) + // if the parent type is a collection ensure the XML attribute is always written, even if false. + // see section 7.2.1 Nullable of OData 4.0.1 + if (parentTypeKind == EdmTypeKind.Collection) { this.WriteRequiredAttribute(CsdlConstants.Attribute_Nullable, reference.IsNullable, EdmValueWriter.BooleanAsXml); } diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs index cabbf2d0f7..9c9a4dfacc 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs @@ -668,15 +668,16 @@ private void ProcessFacets(IEdmTypeReference element, bool inlineType, bool null if (inlineType) { - if (element.TypeKind() == EdmTypeKind.Collection) + var typeKind = element.TypeKind(); + if (typeKind == EdmTypeKind.Collection) { - IEdmCollectionTypeReference collectionElement = element.AsCollection(); - this.schemaWriter.WriteNullableAttribute(collectionElement.CollectionDefinition().ElementType, nullableAttributeRequiredForCollection); - VisitTypeReference(collectionElement.CollectionDefinition().ElementType); + var elementType = element.AsCollection().CollectionDefinition().ElementType; + this.schemaWriter.WriteNullableAttribute(elementType, typeKind); + VisitTypeReference(elementType); } else { - this.schemaWriter.WriteNullableAttribute(element, false); + this.schemaWriter.WriteNullableAttribute(element, typeKind); VisitTypeReference(element); } } From 2ae59f6bf0ff8c588acc8e7877a06aa708c3a5c8 Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Wed, 11 Aug 2021 16:27:31 -0700 Subject: [PATCH 7/8] move logic to writer --- .../EdmModelCsdlSchemaJsonWriter.cs | 16 +++++----------- .../Serialization/EdmModelCsdlSchemaWriter.cs | 8 +++++++- .../Serialization/EdmModelCsdlSchemaXmlWriter.cs | 7 +++++-- .../EdmModelCsdlSerializationVisitor.cs | 11 ++++++----- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs index c2c046c4dc..d0d2df6195 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs @@ -384,18 +384,12 @@ internal override void WriteEnumMemberElementHeader(IEdmEnumMember member) /// 7.2.1 Nullable, A Boolean value specifying whether a value is required for the property. /// /// The Edm type reference. - /// Specifies if the attribute is always written or not if the value is the default value. - internal override void WriteNullableAttribute(IEdmTypeReference reference, bool alwaysWrite) + /// The parent object's (the property) type kind. + internal override void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind) { - // The value of $Nullable is one of the Boolean literals true or false. Absence of the member means false. - if(alwaysWrite) - { - this.jsonWriter.WriteRequiredProperty("$Nullable", reference.IsNullable); - } - else - { - this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); - } + // The value of $Nullable is one of the Boolean literals true or false. Absence of the JSON property means false. + // parentTypeKind gets intentionally ignored since JSON CSDL never writes $Nullable=false + this.jsonWriter.WriteOptionalProperty("$Nullable", reference.IsNullable, defaultValue: false); } internal override void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs index 7b85ddbbf4..224ed1a39c 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaWriter.cs @@ -105,7 +105,13 @@ internal virtual void WriteNavigationPropertyBindingsEnd(IEnumerable + /// 7.2.1 Nullable, A Boolean value specifying whether a value is required for the property. + /// + /// The Edm type reference. + /// The parent object's (the property's) type kind. + internal abstract void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind); + internal abstract void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference); diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs index 57b70e3c7e..43c5b92fee 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs @@ -223,9 +223,12 @@ internal override void WriteEnumMemberElementEnd(IEdmEnumMember member) this.xmlWriter.WriteEndElement(); } - internal override void WriteNullableAttribute(IEdmTypeReference reference, bool alwaysWrite) + /// + internal override void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind) { - if (alwaysWrite) + // if the parent type is a collection ensure the XML attribute is always written, even if false. + // see section 7.2.1 Nullable of OData 4.0.1 + if (parentTypeKind == EdmTypeKind.Collection) { this.WriteRequiredAttribute(CsdlConstants.Attribute_Nullable, reference.IsNullable, EdmValueWriter.BooleanAsXml); } diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs index cabbf2d0f7..9c9a4dfacc 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs @@ -668,15 +668,16 @@ private void ProcessFacets(IEdmTypeReference element, bool inlineType, bool null if (inlineType) { - if (element.TypeKind() == EdmTypeKind.Collection) + var typeKind = element.TypeKind(); + if (typeKind == EdmTypeKind.Collection) { - IEdmCollectionTypeReference collectionElement = element.AsCollection(); - this.schemaWriter.WriteNullableAttribute(collectionElement.CollectionDefinition().ElementType, nullableAttributeRequiredForCollection); - VisitTypeReference(collectionElement.CollectionDefinition().ElementType); + var elementType = element.AsCollection().CollectionDefinition().ElementType; + this.schemaWriter.WriteNullableAttribute(elementType, typeKind); + VisitTypeReference(elementType); } else { - this.schemaWriter.WriteNullableAttribute(element, false); + this.schemaWriter.WriteNullableAttribute(element, typeKind); VisitTypeReference(element); } } From 93d30f33ca2c83dbfb9d3d60336e4e103d4751f1 Mon Sep 17 00:00:00 2001 From: Christof Sprenger Date: Wed, 11 Aug 2021 16:34:14 -0700 Subject: [PATCH 8/8] fix test case for JSON with nullable collection --- .../Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs index 55c554ac74..3be5048ae8 100644 --- a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs @@ -747,7 +747,6 @@ public void SetNavigationPropertyPartnerTest() ""ComplexProp"": { ""$Collection"": true, ""$Type"": ""NS.ComplexType1"", - ""$Nullable"": false }, ""OuterNavA"": { ""$Kind"": ""NavigationProperty"",