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

allow configuring alternate key vocabulary and address performance co… #2491

Open
wants to merge 2 commits into
base: release-7.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,7 @@ internal sealed class TextRes {
internal const string UriParser_ExpandDepthExceeded = "UriParser_ExpandDepthExceeded";
internal const string UriParser_TypeInvalidForSelectExpand = "UriParser_TypeInvalidForSelectExpand";
internal const string UriParser_ContextHandlerCanNotBeNull = "UriParser_ContextHandlerCanNotBeNull";
internal const string UriParser_NullAlternateKeyTerm = "UriParser_NullAlternateKeyTerm";
internal const string UriParserMetadata_MultipleMatchingPropertiesFound = "UriParserMetadata_MultipleMatchingPropertiesFound";
internal const string UriParserMetadata_MultipleMatchingNavigationSourcesFound = "UriParserMetadata_MultipleMatchingNavigationSourcesFound";
internal const string UriParserMetadata_MultipleMatchingTypesFound = "UriParserMetadata_MultipleMatchingTypesFound";
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.OData.Core/Microsoft.OData.Core.txt
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,7 @@ UriParser_ExpandCountExceeded=The result of parsing $expand contained at least {
UriParser_ExpandDepthExceeded=The result of parsing $expand was at least {0} items deep, but the maximum allowed is {1}.
UriParser_TypeInvalidForSelectExpand=The type '{0}' is not valid for $select or $expand, only structured types are allowed.
UriParser_ContextHandlerCanNotBeNull=The handler property for context '{0}' should not return null.
UriParser_NullAlternateKeyTerm=A null term was provided in the '{0}' parameter
UriParserMetadata_MultipleMatchingPropertiesFound=More than one properties match the name '{0}' were found in type '{1}'.
UriParserMetadata_MultipleMatchingNavigationSourcesFound=More than one navigation sources match the name '{0}' were found in model.
UriParserMetadata_MultipleMatchingTypesFound=More than one types match the name '{0}' were found in model.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6372,6 +6372,14 @@ internal static string UriParser_ContextHandlerCanNotBeNull(object p0)
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriParser_ContextHandlerCanNotBeNull, p0);
}

/// <summary>
/// A string like "A null term was provided in the '{0}' parameter"
/// </summary>
internal static string UriParser_NullAlternateKeyTerm(object p0)
{
return Microsoft.OData.TextRes.GetString(Microsoft.OData.TextRes.UriParser_NullAlternateKeyTerm, p0);
}

/// <summary>
/// A string like "More than one properties match the name '{0}' were found in type '{1}'."
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ namespace Microsoft.OData.UriParser
using System.Collections.Generic;
using System.Linq;
using Microsoft.OData.Edm;
using Microsoft.OData.Edm.Vocabularies;
using Microsoft.OData.Edm.Vocabularies.Community.V1;
using Microsoft.OData.Edm.Vocabularies.V1;

/// <summary>
/// Implementation for resolving the alternate keys.
Expand All @@ -21,12 +24,41 @@ public sealed class AlternateKeysODataUriResolver : ODataUriResolver
/// </summary>
private readonly IEdmModel model;

/// <summary>
/// The <see cref="IEdmTerm"/>s that are used within <see cref="model"/> to represent alternate key annotations
/// </summary>
private readonly IEnumerable<IEdmTerm> alternateKeyTerms;

/// <summary>
/// Constructs a AlternateKeysODataUriResolver with the given edmModel to be used for resolving alternate keys
/// </summary>
/// <param name="model">The model to be used.</param>
public AlternateKeysODataUriResolver(IEdmModel model)
: this(model, new[] { AlternateKeysVocabularyModel.AlternateKeysTerm, CoreVocabularyModel.AlternateKeysTerm })
{
}

/// <summary>
/// Constructs a AlternateKeysODataUriResolver with the given edmModel to be used for resolving alternate keys
/// </summary>
/// <param name="model">The model to be used.</param>
/// <param name="alternateKeyTerms">The <see cref="IEdmTerm"/>s that are used within <paramref name="model"/> to represent alternate key annotations</param>
/// <exception cref="ArgumentNullException">Thrown if <paramref name="alternateKeyTerms"/> is <see langword="null"/></exception>
/// <exception cref="ArgumentException">Thrown if <paramref name="alternateKeyTerms"/> contains any <see langword="null"/> values</exception>
public AlternateKeysODataUriResolver(IEdmModel model, IEnumerable<IEdmTerm> alternateKeyTerms)
{
if (alternateKeyTerms == null)
{
throw new ArgumentNullException(nameof(alternateKeyTerms));
}

this.alternateKeyTerms = alternateKeyTerms.ToList();

if (this.alternateKeyTerms.Where(term => term == null).Any())
{
throw new ArgumentException(Strings.UriParser_NullAlternateKeyTerm(nameof(alternateKeyTerms)));
}

this.model = model;
}

Expand Down Expand Up @@ -62,7 +94,7 @@ public override IEnumerable<KeyValuePair<string, object>> ResolveKeys(IEdmEntity
/// <returns>True if resolve succeeded.</returns>
private bool TryResolveAlternateKeys(IEdmEntityType type, IDictionary<string, string> namedValues, Func<IEdmTypeReference, string, object> convertFunc, out IEnumerable<KeyValuePair<string, object>> convertedPairs)
{
IEnumerable<IDictionary<string, IEdmProperty>> alternateKeys = model.GetAlternateKeysAnnotation(type);
IEnumerable<IDictionary<string, IEdmProperty>> alternateKeys = model.GetAlternateKeysAnnotation(type, this.alternateKeyTerms);
foreach (IDictionary<string, IEdmProperty> keys in alternateKeys)
{
if (TryResolveKeys(type, namedValues, keys, convertFunc, out convertedPairs))
Expand Down
66 changes: 47 additions & 19 deletions src/Microsoft.OData.Edm/ExtensionMethods/ExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2155,14 +2155,37 @@ public static bool IsKey(this IEdmProperty property)
/// <param name="type">Reference to the calling object.</param>
/// <returns>Alternate Keys of this type.</returns>
public static IEnumerable<IDictionary<string, IEdmProperty>> GetAlternateKeysAnnotation(this IEdmModel model, IEdmEntityType type)
{
return GetAlternateKeysAnnotation(model, type, new[] { AlternateKeysVocabularyModel.AlternateKeysTerm, CoreVocabularyModel.AlternateKeysTerm });
}

/// <summary>
/// Gets the declared alternate keys of the most defined entity with a declared key present.
/// </summary>
/// <param name="model">The model to be used.</param>
/// <param name="type">Reference to the calling object.</param>
/// <param name="alternateKeyTerms">The <see cref="IEdmTerm"/>s that are used within <paramref name="model"/> to represent alternate key annotations</param>
/// <returns>Alternate Keys of this type.</returns>
/// <exception cref="ArgumentNullException">
/// Thrown if <paramref name="model"/> or <paramref name="type"/> or <paramref name="alternateKeyTerms"/> is <see langword="null"/>
/// </exception>
/// <exception cref="ArgumentException">Thrown if <paramref name="alternateKeyTerms"/> contains any <see langword="null"/> values</exception>
public static IEnumerable<IDictionary<string, IEdmProperty>> GetAlternateKeysAnnotation(
this IEdmModel model,
IEdmEntityType type,
IEnumerable<IEdmTerm> alternateKeyTerms)
{
EdmUtil.CheckArgumentNull(model, "model");
EdmUtil.CheckArgumentNull(type, "type");
EdmUtil.CheckArgumentNull(alternateKeyTerms, nameof(alternateKeyTerms));

IEdmEntityType checkingType = type;
while (checkingType != null)
{
IEnumerable<IDictionary<string, IEdmProperty>> declaredAlternateKeys = GetDeclaredAlternateKeysForType(checkingType, model);
IEnumerable<IDictionary<string, IEdmProperty>> declaredAlternateKeys = GetDeclaredAlternateKeysForType(
checkingType,
model,
alternateKeyTerms);
if (declaredAlternateKeys != null)
{
return declaredAlternateKeys;
Expand Down Expand Up @@ -3462,24 +3485,31 @@ internal static bool HasAny<T>(this IEnumerable<T> enumerable) where T : class
/// </summary>
/// <param name="type">Reference to the calling object.</param>
/// <param name="model">The model to be used.</param>
/// <param name="alternateKeyTerms">
/// The <see cref="IEdmTerm"/>s that are used within <paramref name="model"/> to represent alternate key annotations; assumed to not be <see langword="null"/>
/// </param>
/// <returns>Alternate Keys of this type.</returns>
private static IEnumerable<IDictionary<string, IEdmProperty>> GetDeclaredAlternateKeysForType(IEdmEntityType type, IEdmModel model)
/// <exception cref="ArgumentException">Thrown if <paramref name="alternateKeyTerms"/> contains any <see langword="null"/> values</exception>
private static IEnumerable<IDictionary<string, IEdmProperty>> GetDeclaredAlternateKeysForType(
IEdmEntityType type,
IEdmModel model,
IEnumerable<IEdmTerm> alternateKeyTerms)
{
IEdmVocabularyAnnotation annotationValue = model.FindVocabularyAnnotations<IEdmVocabularyAnnotation>(type, AlternateKeysVocabularyModel.AlternateKeysTerm).FirstOrDefault();
IEdmVocabularyAnnotation coreAnnotationValue = model.FindVocabularyAnnotations<IEdmVocabularyAnnotation>(type, CoreVocabularyModel.AlternateKeysTerm).FirstOrDefault();

if (annotationValue != null || coreAnnotationValue != null)
var any = false;
Copy link
Member

Choose a reason for hiding this comment

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

var

We have traditionally avoided use of "var" in production code (we do use it in test code).

var declaredAlternateKeys = new List<IDictionary<string, IEdmProperty>>();
foreach (var alternateKeyTerm in alternateKeyTerms)
{
List<IDictionary<string, IEdmProperty>> declaredAlternateKeys = new List<IDictionary<string, IEdmProperty>>();
if (alternateKeyTerm == null)
{
throw new ArgumentException(Strings.NullTermForAlternateKey(nameof(alternateKeyTerms)));
}

Action<IEdmVocabularyAnnotation> retrieveAnnotationAction = ann =>
var annotationValue = model.FindVocabularyAnnotations<IEdmVocabularyAnnotation>(type, alternateKeyTerm).FirstOrDefault();
if (annotationValue != null)
{
if (ann == null)
{
return;
}
any = true;
Copy link
Member

@mikepizzo mikepizzo Sep 22, 2022

Choose a reason for hiding this comment

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

any = true;

should we wait to set this until we have a value on line 3539 (i.e., versus the collectionexpression being empty)?

It looks like both the existing and new code could return either null or an empty dictionary, so the calling code presumably has to be prepared for either. Is there an advantage to returning one over the other?


IEdmCollectionExpression keys = ann.Value as IEdmCollectionExpression;
IEdmCollectionExpression keys = annotationValue.Value as IEdmCollectionExpression;
Debug.Assert(keys != null, "expected IEdmCollectionExpression for alternate key annotation value");

foreach (IEdmRecordExpression key in keys.Elements.OfType<IEdmRecordExpression>())
Expand Down Expand Up @@ -3510,13 +3540,11 @@ private static IEnumerable<IDictionary<string, IEdmProperty>> GetDeclaredAlterna
}
}
}
};

// For backwards-compability, we merge the alternate keys from community and core vocabulary annotations.

retrieveAnnotationAction(annotationValue);
retrieveAnnotationAction(coreAnnotationValue);
}
}

if (any)
{
return declaredAlternateKeys;
}

Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.OData.Edm/Microsoft.OData.Edm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ internal sealed class EdmRes {
internal const string EdmType_UnexpectedEdmType = "EdmType_UnexpectedEdmType";
internal const string NavigationPropertyBinding_PathIsNotValid = "NavigationPropertyBinding_PathIsNotValid";
internal const string MultipleMatchingPropertiesFound = "MultipleMatchingPropertiesFound";
internal const string NullTermForAlternateKey = "NullTermForAlternateKey";
internal const string Edm_Evaluator_NoTermTypeAnnotationOnType = "Edm_Evaluator_NoTermTypeAnnotationOnType";
internal const string Edm_Evaluator_NoValueAnnotationOnType = "Edm_Evaluator_NoValueAnnotationOnType";
internal const string Edm_Evaluator_NoValueAnnotationOnElement = "Edm_Evaluator_NoValueAnnotationOnElement";
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.OData.Edm/Microsoft.OData.Edm.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Constructable_DependentPropertyCountMustMatchNumberOfPropertiesOnPrincipalType=T
EdmType_UnexpectedEdmType=Unexpected Edm type.
NavigationPropertyBinding_PathIsNotValid=The navigation property binding path is not valid.
MultipleMatchingPropertiesFound=More than one properties match the name '{0}' were found in type '{1}'.
NullTermForAlternateKey=A null term was provided in the '{0}' parameter when enumerating alternate keys on a type

; Evaluation messages
Edm_Evaluator_NoTermTypeAnnotationOnType=Type '{0}' must have a single type annotation with term type '{1}'.
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.OData.Edm/Parameterized.Microsoft.OData.Edm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ internal static string MultipleMatchingPropertiesFound(object p0, object p1)
return Microsoft.OData.Edm.EdmRes.GetString(Microsoft.OData.Edm.EdmRes.MultipleMatchingPropertiesFound, p0, p1);
}

/// <summary>
/// A string like "A null term was provided in the '{0}' parameter when enumerating alternate keys on a type"
/// </summary>
internal static string NullTermForAlternateKey(object p0)
{
return Microsoft.OData.Edm.EdmRes.GetString(Microsoft.OData.Edm.EdmRes.NullTermForAlternateKey, p0);
}

/// <summary>
/// A string like "Type '{0}' must have a single type annotation with term type '{1}'."
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.OData.Edm.Csdl;
using Microsoft.OData.Edm.Vocabularies;
using Microsoft.OData.Edm.Vocabularies.Community.V1;
using Microsoft.OData.Edm.Vocabularies.V1;
using Microsoft.OData.UriParser;
using Xunit;
using ODataErrorStrings = Microsoft.OData.Strings;
Expand Down Expand Up @@ -405,6 +406,72 @@ public void AlternateKeyUsingCoreVocabularyVersionShouldWork()
pathSegment.LastSegment.ShouldBeKeySegment(new KeyValuePair<string, object>("CoreSN", "1"));
}

[Fact]
public void CoreAlternateKeyNotFound()
{
var parser = new ODataUriParser(HardCodedTestModel.TestModel, new Uri("http://host"), new Uri("http://host/People(CoreSN = \'1\')"))
{
Resolver = new AlternateKeysODataUriResolver(HardCodedTestModel.TestModel, new[] { AlternateKeysVocabularyModel.AlternateKeysTerm }),
};
var exception = Assert.Throws<ODataException>(() => parser.ParsePath());
Assert.Contains("The key in the request URI is not valid", exception.Message, StringComparison.OrdinalIgnoreCase);
}

[Fact]
public void CommunityAlternateKeyNotFound()
{
var parser = new ODataUriParser(HardCodedTestModel.TestModel, new Uri("http://host"), new Uri("http://host/People(SocialSN = \'1\')"))
{
Resolver = new AlternateKeysODataUriResolver(HardCodedTestModel.TestModel, new[] { CoreVocabularyModel.AlternateKeysTerm }),
};
var exception = Assert.Throws<ODataException>(() => parser.ParsePath());
Assert.Contains("The key in the request URI is not valid", exception.Message, StringComparison.OrdinalIgnoreCase);
}

[Fact]
public void SideBySideAlternateKeys()
{
var resolver = new AlternateKeysODataUriResolver(HardCodedTestModel.TestModel);

// check for a community alternate key
var communityPath = new ODataUriParser(HardCodedTestModel.TestModel, new Uri("http://host"), new Uri("http://host/People(SocialSN = \'1\')"))
{
Resolver = resolver,
}.ParsePath();

Assert.Equal(2, communityPath.Count);
communityPath.FirstSegment.ShouldBeEntitySetSegment(HardCodedTestModel.TestModel.FindDeclaredEntitySet("People"));
communityPath.LastSegment.ShouldBeKeySegment(new KeyValuePair<string, object>("SocialSN", "1"));

// check for a core alternate key
var corePath = new ODataUriParser(HardCodedTestModel.TestModel, new Uri("http://host"), new Uri("http://host/People(CoreSN = \'1\')"))
{
Resolver = resolver
}.ParsePath();

Assert.Equal(2, corePath.Count);
corePath.FirstSegment.ShouldBeEntitySetSegment(HardCodedTestModel.TestModel.FindDeclaredEntitySet("People"));
corePath.LastSegment.ShouldBeKeySegment(new KeyValuePair<string, object>("CoreSN", "1"));
}

[Fact]
public void AlternateKeyResolverWithNullTerms()
{
Assert.Throws<ArgumentNullException>(() => new AlternateKeysODataUriResolver(HardCodedTestModel.TestModel, null));
}

[Fact]
public void AlternateKeyResolverWithNullTerm()
{
Assert.Throws<ArgumentException>(() => new AlternateKeysODataUriResolver(
HardCodedTestModel.TestModel,
new IEdmTerm[]
{
CoreVocabularyModel.AlternateKeysTerm,
null,
}));
}

[Fact]
public void CompositeAlternateKeyShouldWork()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.OData.Edm.Csdl.CsdlSemantics;
using Microsoft.OData.Edm.Csdl.Parsing.Ast;
using Microsoft.OData.Edm.Vocabularies;
using Microsoft.OData.Edm.Vocabularies.V1;
using Microsoft.OData.Edm.Tests.Validation;
using Microsoft.OData.Edm.Validation;
using Xunit;
Expand Down Expand Up @@ -180,6 +181,24 @@ public void FullTypeNameAndFullNameIEdmTypeReferenceShouldBeEqual()
Assert.Equal(enumType.Definition.FullTypeName(), enumType.FullName());
}

[Fact]
public void GetAlternateKeyAnnotationsWithNullTerms()
{
Assert.Throws<ArgumentNullException>(() => EdmCoreModel.Instance.GetAlternateKeysAnnotation(EdmCoreModelEntityType.Instance, null));
}

[Fact]
public void GetAlternateKeyAnnotationsWithNullTerm()
{
Assert.Throws<ArgumentException>(() => EdmCoreModel.Instance.GetAlternateKeysAnnotation(
EdmCoreModelEntityType.Instance,
new IEdmTerm[]
{
CoreVocabularyModel.AlternateKeysTerm,
null,
}));
}

#region IEdmType FullName tests

[Fact]
Expand Down
Loading