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

Default to using entityType for incomplete serializationInfo. #2279

Merged
merged 4 commits into from
Jan 6, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
36 changes: 22 additions & 14 deletions src/Microsoft.OData.Core/Evaluation/ODataResourceMetadataContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,31 +166,43 @@ internal static ODataResourceMetadataContext Create(
/// <param name="resource">The resource instance.</param>
/// <param name="serializationInfo">The serialization info of the resource for writing without model.</param>
/// <param name="actualEntityType">The edm entity type of the resource</param>
/// <param name="requiresId">Whether key properties are required to be returned</param>
/// <returns>Key value pair array</returns>
internal static KeyValuePair<string, object>[] GetKeyProperties(
ODataResourceBase resource,
ODataResourceSerializationInfo serializationInfo,
IEdmEntityType actualEntityType)
IEdmEntityType actualEntityType,
bool requiresId)
{
KeyValuePair<string, object>[] keyProperties = null;
string actualEntityTypeName = null;
string actualEntityTypeName = resource.TypeName ?? actualEntityType.FullName();

// if we have serializationInfo, try that first
if (serializationInfo != null)
{
if (String.IsNullOrEmpty(resource.TypeName))
if (String.IsNullOrEmpty(actualEntityTypeName))
{
throw new ODataException(Strings.ODataResourceTypeContext_ODataResourceTypeNameMissing);
}

actualEntityTypeName = resource.TypeName;
keyProperties = ODataResourceMetadataContextWithoutModel.GetPropertiesBySerializationInfoPropertyKind(resource, ODataPropertyKind.Key, actualEntityTypeName);
}
else

// if we didn't get any keys from serializationInfo, try using entity type
if ((keyProperties == null || keyProperties.Length == 0) && actualEntityType != null)
{
keyProperties = GetPropertyValues(actualEntityType.Key(), resource, actualEntityType, /*isKeyProperty*/ true, requiresId).ToArray();
}

if (requiresId)
{
keyProperties = GetPropertyValues(actualEntityType.Key(), resource, actualEntityType, /*isKeyProperty*/ true, /*isRequired*/ true).ToArray();
ValidateEntityTypeHasKeyProperties(keyProperties, actualEntityTypeName);
}
else if (keyProperties == null)
{
keyProperties = Enumerable.Empty<KeyValuePair<string, object>>().ToArray();
}

ValidateEntityTypeHasKeyProperties(keyProperties, actualEntityTypeName);
return keyProperties;
}

Expand Down Expand Up @@ -269,7 +281,6 @@ private static object GetPrimitiveOrEnumPropertyValue(string entityTypeName, ODa
/// <param name="actualEntityTypeName">The entity type name of the resource.</param>
private static void ValidateEntityTypeHasKeyProperties(KeyValuePair<string, object>[] keyProperties, string actualEntityTypeName)
{
Debug.Assert(keyProperties != null, "keyProperties != null");
if (keyProperties == null || keyProperties.Length == 0)
{
throw new ODataException(Strings.ODataResourceMetadataContext_EntityTypeWithNoKeyProperties(actualEntityTypeName));
Expand All @@ -288,7 +299,7 @@ private static KeyValuePair<string, object>[] GetPropertiesBySerializationInfoPr
Debug.Assert(resource != null, "resource != null");
Debug.Assert(propertyKind == ODataPropertyKind.Key || propertyKind == ODataPropertyKind.ETag, "propertyKind == ODataPropertyKind.Key || propertyKind == ODataPropertyKind.ETag");

List<KeyValuePair<string, object>> properties = new List<KeyValuePair<string, object>>(); ;
List<KeyValuePair<string, object>> properties = new List<KeyValuePair<string, object>>();
if (resource.NonComputedProperties != null)
{
foreach(ODataProperty property in resource.NonComputedProperties)
Expand Down Expand Up @@ -357,11 +368,8 @@ public override ICollection<KeyValuePair<string, object>> KeyProperties
{
if (this.keyProperties == null)
{
this.keyProperties = GetPropertiesBySerializationInfoPropertyKind(this.resource, ODataPropertyKind.Key, this.ActualResourceTypeName);
if (this.requiresId)
{
ValidateEntityTypeHasKeyProperties(this.keyProperties, this.ActualResourceTypeName);
}
IEdmEntityType entityType = this.ActualResourceType as IEdmEntityType;
this.keyProperties = GetKeyProperties(this.resource, this.serializationInfo, entityType, this.requiresId);
}

return this.keyProperties;
Expand Down
20 changes: 9 additions & 11 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2426,20 +2426,18 @@ private void StartNestedStreamInfo(ODataJsonLightReaderStreamInfo readerStreamIn
/// <returns>True if successfully append key segment.</returns>
private bool TryAppendEntitySetKeySegment(ref ODataPath odataPath)
{
try
if (EdmExtensionMethods.HasKey(this.CurrentScope.NavigationSource, this.CurrentScope.ResourceType as IEdmStructuredType))
{
if (EdmExtensionMethods.HasKey(this.CurrentScope.NavigationSource, this.CurrentScope.ResourceType as IEdmStructuredType))
IEdmEntityType currentEntityType = this.CurrentScope.ResourceType as IEdmEntityType;
ODataResourceBase resource = this.CurrentScope.Item as ODataResourceBase;
KeyValuePair<string, object>[] keys = ODataResourceMetadataContext.GetKeyProperties(resource, null, currentEntityType, false);
Copy link
Contributor

@corranrogue9 corranrogue9 Jan 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetKeyProperties seems to make the assumption that resource isn't null. I know that this isn't really related to your change, but not validating that currentEntityType is actually a IEdmEntityType means we could get a NullReferenceException here. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is our code and should never be called with a null resource. I added an assert to GetKeyProperties.

if (keys.Length == 0)
{
IEdmEntityType currentEntityType = this.CurrentScope.ResourceType as IEdmEntityType;
ODataResourceBase resource = this.CurrentScope.Item as ODataResourceBase;
KeyValuePair<string, object>[] keys = ODataResourceMetadataContext.GetKeyProperties(resource, null, currentEntityType);
odataPath = odataPath.AddKeySegment(keys, currentEntityType, this.CurrentScope.NavigationSource);
odataPath = null;
return false;
}
}
catch (ODataException)
{
odataPath = null;
return false;

odataPath = odataPath.AddKeySegment(keys, currentEntityType, this.CurrentScope.NavigationSource);
}

return true;
Expand Down
3 changes: 1 addition & 2 deletions src/Microsoft.OData.Core/ODataWriterCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1935,7 +1935,6 @@ private void WriteEndImplementation()
this.InterceptException(
(thisParam) =>
{
bool wasenableDelta;
Scope currentScope = thisParam.CurrentScope;

switch (currentScope.State)
Expand Down Expand Up @@ -2868,7 +2867,7 @@ private ODataPath AppendEntitySetKeySegment(ODataPath odataPath, bool throwIfFai
else
{
KeyValuePair<string, object>[] keys = ODataResourceMetadataContext.GetKeyProperties(resource,
this.GetResourceSerializationInfo(resource), currentEntityType);
serializationInfo, currentEntityType, true);

path = path.AddKeySegment(keys, currentEntityType, this.CurrentScope.NavigationSource);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ public void ShouldGetKeyPropertiesBasedOnMetadata()
{
this.entry.Properties = new[]
{
new ODataProperty { Name = "ID1", Value = 1, SerializationInfo = new ODataPropertySerializationInfo { PropertyKind = ODataPropertyKind.Key } },
new ODataProperty { Name = "ID2", Value = 2, SerializationInfo = new ODataPropertySerializationInfo { PropertyKind = ODataPropertyKind.Key } },
new ODataProperty { Name = "ID1", Value = 1 },
new ODataProperty { Name = "ID2", Value = 2 },
new ODataProperty { Name = "ID3", Value = 3 },
};

Expand All @@ -206,6 +206,7 @@ public void ShouldGetKeyPropertiesBasedOnMetadata()
Assert.Contains(keys, p => p.Key == "ID2");
Assert.Contains(keys, p => p.Key == "ID3");
}

#endregion KeyProperties

#region ETagProperties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,146 @@ static AutoComputePayloadMetadataInJsonIntegrationTests()
Model.AddElement(function4);
}

public enum SerializationType
{
NoSerializationInfo,
ResourceSerializationInfo,
ResourceAndPropertySerializationInfo
}

[Theory]
[InlineData(SerializationType.NoSerializationInfo, /*fullMetadata*/ false)]
[InlineData(SerializationType.ResourceAndPropertySerializationInfo, /*fullMetadata*/ false)]
[InlineData(SerializationType.ResourceSerializationInfo, /*fullMetadata*/ false)]
[InlineData(SerializationType.NoSerializationInfo, /*fullMetadata*/ true)]
[InlineData(SerializationType.ResourceAndPropertySerializationInfo, /*fullMetadata*/ true)]
[InlineData(SerializationType.ResourceSerializationInfo, /*fullMetadata*/ true)]
public void WritingNestedResourceInfoWithVariousSerializationInfoSucceeds(SerializationType serializationType, bool fullMetadata)
{
// setup model
var model = new EdmModel();
var entityType = new EdmEntityType("NS", "entityType");
entityType.AddKeys(
entityType.AddStructuralProperty("keyProperty", EdmPrimitiveTypeKind.Int64));

var nestedEntityType = new EdmEntityType("NS", "nestedEntityType");
nestedEntityType.AddKeys(
nestedEntityType.AddStructuralProperty("keyProperty", EdmPrimitiveTypeKind.Int64));

entityType.AddUnidirectionalNavigation(new EdmNavigationPropertyInfo {
Name = "nestedEntities",
Target = nestedEntityType,
TargetMultiplicity = EdmMultiplicity.Many,
ContainsTarget = true
});

var container = new EdmEntityContainer("NS", "Container");
var entitySet = container.AddEntitySet("EntitySet", entityType);

model.AddElements(new IEdmSchemaElement[] { entityType, nestedEntityType, container });

// setup writer
var stream = new MemoryStream();
Copy link
Contributor

@corranrogue9 corranrogue9 Jan 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MemoryStream implements IDisposable. Please wrap it in a using block #Resolved

var message = new InMemoryMessage { Stream = stream };
message.SetHeader("Content-Type", fullMetadata ? "application/json;odata.metadata=full" : "application/json");
var settings = new ODataMessageWriterSettings
{
ODataUri = new ODataUri
{
ServiceRoot = new Uri("http://svc/"),
Path = new ODataUriParser(model, new Uri("EntitySet",UriKind.Relative)).ParsePath()
},
};
var writer = new ODataMessageWriter((IODataResponseMessage)message, settings, model);

// write payload
var entitySetWriter = writer.CreateODataResourceSetWriter(entitySet, entitySet.EntityType());
entitySetWriter.WriteStart(new ODataResourceSet());
var resource = new ODataResource
{
Properties = new[]
{
new ODataProperty {
Name = "keyProperty",
Value = 1L,
SerializationInfo = serializationType == SerializationType.ResourceAndPropertySerializationInfo ?
new ODataPropertySerializationInfo {
PropertyKind = ODataPropertyKind.Key
} : null
}
},
TypeName = serializationType == SerializationType.NoSerializationInfo ? null : entityType.FullName
};

if (serializationType != SerializationType.NoSerializationInfo)
{
resource.SerializationInfo = new ODataResourceSerializationInfo
{
NavigationSourceName = "EntitySet",
NavigationSourceKind = EdmNavigationSourceKind.EntitySet,
NavigationSourceEntityTypeName = entityType.FullName,
ExpectedTypeName = entityType.FullName,
IsFromCollection = true
};
}

entitySetWriter.WriteStart( resource );
entitySetWriter.WriteStart(
new ODataNestedResourceInfo
{
Name = "nestedEntities",
}
);
entitySetWriter.WriteStart(new ODataResourceSet());
var entityValue = new ODataResource
{
TypeName = "NS.nestedEntityType",
Properties = new[]
{
new ODataProperty { Name = "keyProperty", Value = 1L }
}
};
entitySetWriter.WriteStart(entityValue);
entitySetWriter.WriteEnd(); // nestedEntity
entitySetWriter.WriteEnd(); // nested resourceSet
entitySetWriter.WriteEnd(); // nestedInfo
entitySetWriter.WriteEnd(); // entity
entitySetWriter.WriteEnd(); // resourceSet
var str = Encoding.UTF8.GetString(stream.ToArray());

var typeAnnotation =
serializationType == SerializationType.NoSerializationInfo ? "" :
"\"@odata.type\":\"#NS.entityType\",";

var expected = fullMetadata ?
"{\"@odata.context\":\"http://svc/$metadata#EntitySet\"," +
"\"value\":[{" +
typeAnnotation +
"\"@odata.id\":\"EntitySet(1)\"," +
"\"@odata.editLink\":\"EntitySet(1)\"," +
"\"[email protected]\":\"#Int64\"," +
"\"keyProperty\":1," +
"\"[email protected]\":\"http://svc/EntitySet(1)/nestedEntities/$ref\"," +
"\"[email protected]\":\"http://svc/EntitySet(1)/nestedEntities\"," +
"\"nestedEntities\":[{" +
"\"@odata.type\":\"#NS.nestedEntityType\"," +
"\"@odata.id\":\"EntitySet(1)/nestedEntities(1)\"," +
"\"@odata.editLink\":\"EntitySet(1)/nestedEntities(1)\"," +
"\"[email protected]\":\"#Int64\"," +
"\"keyProperty\":1" +
"}]" +
"}]}" :
"{\"@odata.context\":\"http://svc/$metadata#EntitySet\"," +
"\"value\":[{" +
"\"keyProperty\":1," +
"\"nestedEntities\":[{" +
"\"keyProperty\":1" +
"}]" +
"}]}";

Assert.Equal(expected, str);
}

[Fact]
public void WritingDynamicComplexPropertyWithModelSpecifiedInFullMetadataMode()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ public void ReadingingSingleComplexProppertyWithNotAllowedTypeWorksWithoutDerive
// Negative test case --Structural property has the derived type constraint.
SetDerivedTypeAnnotation(this.edmModel, locationProperty, "NS.CnAddress");

Action test = () => ReadEntityPayload(payload, this.edmModel, this.edmMe, this.edmCustomerType); ;
Action test = () => ReadEntityPayload(payload, this.edmModel, this.edmMe, this.edmCustomerType);
var exception = Assert.Throws<ODataException>(test);
Assert.Equal(Strings.ReaderValidationUtils_ValueTypeNotAllowedInDerivedTypeConstraint("NS.UsAddress", "nested resource", "Location"), exception.Message);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2026,7 +2026,7 @@ internal void VisitAndVerifyXml(Action<EdmModelCsdlSerializationVisitor> testAct
{
Indent = indent,
ConformanceLevel = ConformanceLevel.Auto
}); ;
});
var schemaWriter = new EdmModelCsdlSchemaXmlWriter(model, xmlWriter, edmxVersion);
var visitor = new EdmModelCsdlSerializationVisitor(model, schemaWriter);

Expand Down