Skip to content

Commit

Permalink
different null validation messages for complex and primitive collections
Browse files Browse the repository at this point in the history
  • Loading branch information
ElizabethOkerio committed Jun 25, 2020
1 parent 3d97baa commit 7e98502
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ protected ODataJsonLightReaderNestedResourceInfo InnerReadUndeclaredProperty(IOD

// Complex property or collection of complex property.
bool isCollection = payloadTypeReference.IsCollection();
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName);
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName, payloadTypeReference);
if (isCollection)
{
readerNestedResourceInfo = this.ReadingResponse
Expand Down Expand Up @@ -473,7 +473,8 @@ protected ODataJsonLightReaderNestedResourceInfo InnerReadUndeclaredProperty(IOD
/// <param name="jsonReader">The IJsonReader.</param>
/// <param name="isCollection">true if the property is entity set reference property; false for a resource reference property, null if unknown.</param>
/// <param name="propertyName">Name for the navigation property, used in error message only.</param>
protected static void ValidateExpandedNestedResourceInfoPropertyValue(IJsonReader jsonReader, bool? isCollection, string propertyName)
/// <param name="typeReference">Type of navigation property</param>
protected static void ValidateExpandedNestedResourceInfoPropertyValue(IJsonReader jsonReader, bool? isCollection, string propertyName, IEdmTypeReference typeReference)
{
// an expanded link with resource requires a StartObject node here;
// an expanded link with resource set requires a StartArray node here;
Expand All @@ -491,7 +492,21 @@ protected static void ValidateExpandedNestedResourceInfoPropertyValue(IJsonReade
// Expanded resource (null or non-null)
if (isCollection == true)
{
throw new ODataException(ODataErrorStrings.ODataJsonLightResourceDeserializer_CannotReadCollectionNestedResource(nodeType, propertyName));
if (typeReference.IsODataPrimitiveTypeKind() ||
typeReference.IsODataTypeDefinitionTypeKind() ||
typeReference.IsODataEnumTypeKind() ||
typeReference.IsODataComplexTypeKind() ||
typeReference.IsUntyped() ||
typeReference.IsNonEntityCollectionType())
{
ReaderValidationUtils.ValidateNullValue(typeReference, true,
true, propertyName, false);
}
else
{
throw new ODataException(ODataErrorStrings.ODataJsonLightResourceDeserializer_CannotReadCollectionNestedResource(nodeType, propertyName));
}

}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1232,7 +1232,7 @@ private ODataJsonLightReaderNestedInfo ReadPropertyWithValue(IODataJsonLightRead
ODataJsonLightReaderNestedResourceInfo readerNestedResourceInfo = null;

// Complex property or collection of complex property.
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName);
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName, edmProperty.Type);

if (isCollection)
{
Expand All @@ -1251,7 +1251,7 @@ private ODataJsonLightReaderNestedInfo ReadPropertyWithValue(IODataJsonLightRead
ODataJsonLightReaderNestedResourceInfo readerNestedResourceInfo = null;

// Expanded link
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName);
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName, edmProperty.Type);
if (isCollection)
{
readerNestedResourceInfo = this.ReadingResponse || isDeltaResourceSet
Expand Down Expand Up @@ -1548,7 +1548,7 @@ private ODataJsonLightReaderNestedInfo InnerReadUndeclaredProperty(IODataJsonLig
if (payloadTypeOrItemType != null)
{
// Complex property or collection of complex property.
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName);
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, isCollection, propertyName, payloadTypeReference);
if (isCollection)
{
return ReadNonExpandedResourceSetNestedResourceInfo(resourceState, null, payloadTypeOrItemType, propertyName);
Expand Down Expand Up @@ -1667,8 +1667,8 @@ private ODataJsonLightReaderNestedInfo ReadUndeclaredProperty(IODataJsonLightRea

// If the property is expanded, ignore the content if we're asked to do so.
if (propertyWithValue)
{
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, null, propertyName);
{
ValidateExpandedNestedResourceInfoPropertyValue(this.JsonReader, null, propertyName, resourceState.ResourceType.ToTypeReference());

// Since we marked the nested resource info as deferred the reader will not try to read its content
// instead it will behave as if it was a real deferred link (without a property value).
Expand Down
4 changes: 3 additions & 1 deletion src/Microsoft.OData.Core/Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace Microsoft.OData
/// <summary>
/// AutoGenerated resource class. Usage:
///
/// string s = TextRes.GetString(TextRes.MyIdentifier);
/// string s = TextRes.GetString(TextRes.MyIdenfitier);
/// </summary>
internal sealed class TextRes {
internal const string ExceptionUtils_ArgumentStringEmpty = "ExceptionUtils_ArgumentStringEmpty";
Expand Down Expand Up @@ -251,7 +251,9 @@ internal sealed class TextRes {
internal const string ODataUtils_ModelDoesNotHaveContainer = "ODataUtils_ModelDoesNotHaveContainer";
internal const string ReaderUtils_EnumerableModified = "ReaderUtils_EnumerableModified";
internal const string ReaderValidationUtils_NullValueForNonNullableType = "ReaderValidationUtils_NullValueForNonNullableType";
internal const string ReaderValidationUtils_NullValueForNullableType = "ReaderValidationUtils_NullValueForNullableType";
internal const string ReaderValidationUtils_NullNamedValueForNonNullableType = "ReaderValidationUtils_NullNamedValueForNonNullableType";
internal const string ReaderValidationUtils_NullNamedValueForNullableType = "ReaderValidationUtils_NullNamedValueForNullableType";
internal const string ReaderValidationUtils_EntityReferenceLinkMissingUri = "ReaderValidationUtils_EntityReferenceLinkMissingUri";
internal const string ReaderValidationUtils_ValueWithoutType = "ReaderValidationUtils_ValueWithoutType";
internal const string ReaderValidationUtils_ResourceWithoutType = "ReaderValidationUtils_ResourceWithoutType";
Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ ODataUtils_ModelDoesNotHaveContainer=The provided model does not contain an enti
ReaderUtils_EnumerableModified=The value returned by the '{0}' property cannot be modified until the end of the owning resource is reported by the reader.

ReaderValidationUtils_NullValueForNonNullableType=A null value was found with the expected type '{0}[Nullable=False]'. The expected type '{0}[Nullable=False]' does not allow null values.
ReaderValidationUtils_NullValueForNullableType=A null value was found with the expected type '{0}[Nullable=True]'. The expected type '{0}[Nullable=True]' cannot be null but it can have null values.
ReaderValidationUtils_NullNamedValueForNonNullableType=A null value was found for the property named '{0}', which has the expected type '{1}[Nullable=False]'. The expected type '{1}[Nullable=False]' does not allow null values.
ReaderValidationUtils_NullNamedValueForNullableType=A null value was found for the property named '{0}', which has the expected type '{1}[Nullable=True]'. The expected type '{1}[Nullable=True]' cannot be null but it can have null values.
ReaderValidationUtils_EntityReferenceLinkMissingUri=No URI value was found for an entity reference link. A single URI value was expected.
ReaderValidationUtils_ValueWithoutType=A value without a type name was found and no expected type is available. When the model is specified, each value in the payload must have a type which can be either specified in the payload, explicitly by the caller or implicitly inferred from the parent value.
ReaderValidationUtils_ResourceWithoutType=A resource without a type name was found, but no expected type was specified. To allow entries without type information, the expected type must also be specified when the model is specified.
Expand Down
14 changes: 14 additions & 0 deletions src/Microsoft.OData.Core/Parameterized.Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2147,6 +2147,13 @@ internal static string ReaderValidationUtils_NullValueForNonNullableType(object
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ReaderValidationUtils_NullValueForNonNullableType, p0);
}

/// <summary>
/// A string like "A null value was found with the expected type '{0}[Nullable=True]'. The expected type '{0}[Nullable=True]' does not allow null values."
/// </summary>
internal static string ReaderValidationUtils_NullValueForNullableType(object p0) {
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ReaderValidationUtils_NullValueForNullableType, p0);
}

/// <summary>
/// A string like "A null value was found for the property named '{0}', which has the expected type '{1}[Nullable=False]'. The expected type '{1}[Nullable=False]' does not allow null values."
/// </summary>
Expand All @@ -2155,6 +2162,13 @@ internal static string ReaderValidationUtils_NullNamedValueForNonNullableType(ob
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ReaderValidationUtils_NullNamedValueForNonNullableType, p0, p1);
}

/// <summary>
/// A string like "A null value was found for the property named '{0}', which has the expected type '{1}[Nullable=True]'. The expected type '{1}[Nullable=True]' cannot be null but it can have null values."
/// </summary>
internal static string ReaderValidationUtils_NullNamedValueForNullableType(object p0, object p1) {
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.ReaderValidationUtils_NullNamedValueForNullableType, p0, p1);
}

/// <summary>
/// A string like "No URI value was found for an entity reference link. A single URI value was expected."
/// </summary>
Expand Down
21 changes: 20 additions & 1 deletion src/Microsoft.OData.Core/ReaderValidationUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,11 @@ private static void ValidateNullValueAllowed(IEdmTypeReference expectedValueType
{
if (isDynamicProperty != true)
{
ThrowNullValueForNonNullableTypeException(expectedValueTypeReference, propertyName);
if (!expectedValueTypeReference.IsNullable)
{
ThrowNullValueForNonNullableTypeException(expectedValueTypeReference, propertyName);
}
ThrowNullValueForNullableTypeException(expectedValueTypeReference, propertyName);
}
}
else if (expectedValueTypeReference.IsODataComplexTypeKind())
Expand Down Expand Up @@ -1151,6 +1155,21 @@ private static void ThrowNullValueForNonNullableTypeException(IEdmTypeReference
throw new ODataException(Strings.ReaderValidationUtils_NullNamedValueForNonNullableType(propertyName, expectedValueTypeReference.FullName()));
}

/// <summary>
/// Create and throw exception that a null value was found when the expected type is non-nullable.
/// </summary>
/// <param name="expectedValueTypeReference">The expected type for this value.</param>
/// <param name="propertyName">The name of the property whose value is being read, if applicable.</param>
private static void ThrowNullValueForNullableTypeException(IEdmTypeReference expectedValueTypeReference, string propertyName)
{
if (string.IsNullOrEmpty(propertyName))
{
throw new ODataException(Strings.ReaderValidationUtils_NullValueForNullableType(expectedValueTypeReference.FullName()));
}

throw new ODataException(Strings.ReaderValidationUtils_NullNamedValueForNullableType(propertyName, expectedValueTypeReference.FullName()));
}


/// <summary>
/// The validate resource set context uri.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1510,6 +1510,7 @@ private static IEdmModel GetModel()

person.AddStructuralProperty("Address", new EdmComplexTypeReference(complex, false));
person.AddStructuralProperty("Addresses", new EdmCollectionTypeReference(new EdmCollectionType(new EdmComplexTypeReference(complex, false))));
person.AddStructuralProperty("NewAddresses", new EdmCollectionTypeReference(new EdmCollectionType(new EdmComplexTypeReference(complex, true))));

model.AddElement(person);
model.AddElement(employee);
Expand Down Expand Up @@ -1652,5 +1653,53 @@ private static IEdmModel ContainmentComplexModel()
}

#endregion
[Fact]
public void NullValidationErrorMessageForCollectionsofComplexTypes()
{
string expectedErrorMessage = "A null value was found for the property named 'Addresses', which has the expected type 'Collection(DefaultNs.Address)[Nullable=False]'. The expected type 'Collection(DefaultNs.Address)[Nullable=False]' does not allow null values.";
string actualErrorMessage = "";
const string payload =
"{" +
"\"@odata.context\":\"http://host/$metadata#People/$entity\"," +
"\"UserName\":\"abc\"," +
"\"Addresses\":null" +
"}";

try
{
ReadPayload(payload, Model, EntitySet, EntityType).OfType<ODataResource>().ToList();
}
catch (Exception e)
{
actualErrorMessage = e.Message;
}
Assert.Equal(expectedErrorMessage, actualErrorMessage);
}

/// <summary>
/// A nullable collection cannot be null but it can have null values.
/// </summary>
[Fact]
public void NullValidationErrorMessageForANullableCollectionsofComplexTypes()
{
string expectedErrorMessage = "A null value was found for the property named 'NewAddresses', which has the expected type 'Collection(DefaultNs.Address)[Nullable=True]'. The expected type 'Collection(DefaultNs.Address)[Nullable=True]' cannot be null but it can have null values.";
string actualErrorMessage = "";
const string payload =
"{" +
"\"@odata.context\":\"http://host/$metadata#People/$entity\"," +
"\"UserName\":\"abc\"," +
"\"NewAddresses\":null" +
"}";

try
{
ReadPayload(payload, Model, EntitySet, EntityType).OfType<ODataResource>().ToList();
}
catch (Exception e)
{
actualErrorMessage = e.Message;
}
Assert.Equal(expectedErrorMessage, actualErrorMessage);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ public void CollectionValueTest()
PayloadElement = PayloadBuilder.Property("PrimitiveCollection",
PayloadBuilder.PrimitiveMultiValue("Collection(Edm.Int32)"))
.JsonRepresentation(
"{" +
"{" +
"\"" + JsonLightConstants.ODataPropertyAnnotationSeparator + JsonLightConstants.ODataContextAnnotationName + "\":\"http://odata.org/test/$metadata#Collection(Edm.Int32)\"," +
"\"value\":null" +
"}")
.ExpectedProperty(owningType, "PrimitiveCollection"),
SkipTestConfiguration = tc => tc.IsRequest,
ExpectedException = ODataExpectedExceptions.ODataException("ReaderValidationUtils_NullNamedValueForNonNullableType", "value", "Collection(Edm.Int32)")
ExpectedException = ODataExpectedExceptions.ODataException("ReaderValidationUtils_NullNamedValueForNullableType", "value", "Collection(Edm.Int32)")
},
new PayloadReaderTestDescriptor(this.Settings)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,6 @@ public void TopLevelPropertyTest()
ExpectedException = ODataExpectedExceptions.ODataException("ValidationUtils_IncorrectTypeKind", "Unknown", "Primitive", "Complex")
},
new
{
Description = "Invalid type information for top-level null - we don't allow null collections - should fail.",
Json = "{ " +
"\"" + JsonLightConstants.ODataPropertyAnnotationSeparator + JsonLightConstants.ODataContextAnnotationName + "\":\"http://odata.org/test/$metadata#Collection(Edm.Int32)\", " +
"\"" + JsonLightConstants.ODataPropertyAnnotationSeparator + JsonLightConstants.ODataTypeAnnotationName + "\": \"Collection(Edm.Int32)\"," +
"\"" + JsonLightConstants.ODataValuePropertyName + "\": null" +
"}",
ReaderMetadata = collectionReaderMetadata,
ExpectedException = ODataExpectedExceptions.ODataException("ReaderValidationUtils_NullNamedValueForNonNullableType", "value", "Collection(Edm.Int32)")
},
new
{
Description = "Type information for top-level spatial - should work.",
Json = "{ " +
Expand Down

0 comments on commit 7e98502

Please sign in to comment.