diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs index d437d42e23..d0d2df6195 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaJsonWriter.cs @@ -384,10 +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. - internal override void WriteNullableAttribute(IEdmTypeReference reference) + /// 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. - 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 0c3f1f771d..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 09ccae1f10..43c5b92fee 100644 --- a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs +++ b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSchemaXmlWriter.cs @@ -223,9 +223,19 @@ internal override void WriteEnumMemberElementEnd(IEdmEnumMember member) this.xmlWriter.WriteEndElement(); } - internal override void WriteNullableAttribute(IEdmTypeReference reference) + /// + internal override void WriteNullableAttribute(IEdmTypeReference reference, EdmTypeKind parentTypeKind) { - this.WriteOptionalAttribute(CsdlConstants.Attribute_Nullable, reference.IsNullable, CsdlConstants.Default_Nullable, EdmValueWriter.BooleanAsXml); + // 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); + } + else + { + this.WriteOptionalAttribute(CsdlConstants.Attribute_Nullable, reference.IsNullable, CsdlConstants.Default_Nullable, EdmValueWriter.BooleanAsXml); + } } internal override void WriteTypeDefinitionAttributes(IEdmTypeDefinitionReference reference) diff --git a/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs b/src/Microsoft.OData.Edm/Csdl/Serialization/EdmModelCsdlSerializationVisitor.cs index 45b618822f..9c9a4dfacc 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) { @@ -665,15 +668,16 @@ private void ProcessFacets(IEdmTypeReference element, bool inlineType) 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); - VisitTypeReference(collectionElement.CollectionDefinition().ElementType); + var elementType = element.AsCollection().CollectionDefinition().ElementType; + this.schemaWriter.WriteNullableAttribute(elementType, typeKind); + VisitTypeReference(elementType); } else { - this.schemaWriter.WriteNullableAttribute(element); + this.schemaWriter.WriteNullableAttribute(element, typeKind); VisitTypeReference(element); } } diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlWriterTests.cs index 8e2d314e66..3be5048ae8 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,7 @@ public void SetNavigationPropertyPartnerTest() }, ""ComplexProp"": { ""$Collection"": true, - ""$Type"": ""NS.ComplexType1"" + ""$Type"": ""NS.ComplexType1"", }, ""OuterNavA"": { ""$Kind"": ""NavigationProperty"", @@ -1421,7 +1422,7 @@ public void ShouldWriteEdmPathTypeProperty() "" + "" + "" + - "" + + "" + "" + "" + "" + @@ -1442,7 +1443,8 @@ public void ShouldWriteEdmPathTypeProperty() }, ""DefaultHidden"": { ""$Collection"": true, - ""$Type"": ""Edm.NavigationPropertyPath"" + ""$Type"": ""Edm.NavigationPropertyPath"", + ""$Nullable"": false } }, ""MyTerm"": { @@ -1604,8 +1606,8 @@ public void CanWriteEdmEntityTypeWithCollectionAbstractTypeButValidationFailed() "" + "" + "" + - "" + - "" + + "" + + "" + "" + "" + "" + @@ -2377,7 +2379,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 2946095263..afb1e2abf3 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), @" - + @@ -216,7 +216,8 @@ public void VerifyEntityTypeWithNavigationPropertiesWrittenCorrectly() ""$Kind"": ""EntityType"", ""Locations"": { ""$Collection"": true, - ""$Type"": ""NS.Address"" + ""$Type"": ""NS.Address"", + ""$Nullable"": false }, ""Orders"": { ""$Kind"": ""NavigationProperty"", @@ -2015,6 +2016,53 @@ public void VerifyAnnotationWithRecordExpressionWrittenCorrectly() } #endregion + #region Collection Type References + [Fact] + public void VerifyCollectionTypeReferencesWrittenCorrectly() + { + // see https://github.com/OData/odata.net/issues/2028 + // Arrange + EdmComplexType structuredType = new EdmComplexType("NS", "Customer"); + structuredType.AddStructuralProperty("test01", EdmCoreModel.GetCollection(EdmCoreModel.Instance.GetDecimal(true))); + structuredType.AddStructuralProperty("test02", EdmCoreModel.GetCollection(EdmCoreModel.Instance.GetDecimal(false))); + structuredType.AddStructuralProperty("test03", EdmCoreModel.Instance.GetDecimal(true)); + structuredType.AddStructuralProperty("test04", EdmCoreModel.Instance.GetDecimal(false)); + + // Act & Assert for XML + VisitAndVerifyXml(v => v.VisitSchemaType(structuredType), @" + + + + +" +); + + // Act & Assert for JSON + VisitAndVerifyJson(v => v.VisitSchemaType(structuredType), @"{ + ""Customer"": { + ""$Kind"": ""ComplexType"", + ""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"" + } + } +}"); + } + #endregion + internal void VisitAndVerifyXml(Action testAction, string expected, bool indent = true) { XmlWriter xmlWriter; diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/AlternateKeysVocabularyTests.cs index b28c58e501..0b678cb01c 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 = @" - + diff --git a/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs b/test/FunctionalTests/Microsoft.OData.Edm.Tests/Vocabularies/CapabilitiesVocabularyTests.cs index 3766a956b4..e30718f853 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() - + 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 = @"