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.

Sign up for GitHub

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

Import vocabulary annotations declared in external referenced models #1987

Merged
merged 11 commits into from
Feb 23, 2021
Merged

Import vocabulary annotations declared in external referenced models #1987

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
36af6c3
Import referenced schemas that include annotations
habbes Jan 25, 2021
256e3b1
Only import annotations with match IncludeAnnotation namespace, targe…
habbes Jan 26, 2021
26a681e
Add tests for IncludeAnnotations with Qualifier and TargetNamespace
habbes Jan 26, 2021
d6087fc
Fix issues in FilterIncludedAnnotations method
habbes Jan 26, 2021
9613de6
Don't import schema types when only vocabulary annotations are refere…
habbes Jan 26, 2021
a3a8877
Refactor adding external schema if referenced
habbes Feb 3, 2021
b838022
Minor optimization when adding annotations to model
habbes Feb 9, 2021
717346e
Add more doc comments
habbes Feb 9, 2021
6ed8841
Fix compiler error in CsdlSemanticsModel
habbes Feb 23, 2021
747511b
Test import of referenced schema elements
habbes Feb 23, 2021
e4279cf
Fix bugs in tests
habbes Feb 23, 2021
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
96 changes: 69 additions & 27 deletions src/Microsoft.OData.Edm/Csdl/Semantics/CsdlSemanticsModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,14 @@ private CsdlSemanticsModel(CsdlModel referencedCsdlModel, IEdmDirectValueAnnotat
foreach (var schema in referencedCsdlModel.Schemata)
{
string schemaNamespace = schema.Namespace;
IEdmInclude edmInclude = referencedCsdlModel.ParentModelReferences.SelectMany(s => s.Includes).FirstOrDefault(s => s.Namespace == schemaNamespace);
if (edmInclude != null)
bool shouldAddTypes = referencedCsdlModel.ParentModelReferences.SelectMany(s => s.Includes).Any(s => s.Namespace == schemaNamespace);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool shouldAddTypes = referencedCsdlModel.ParentModelReferences.SelectMany(s => s.Includes).Any(s => s.Namespace == schemaNamespace);
bool addSchemaElements = referencedCsdlModel.ParentModelReferences.SelectMany(s => s.Includes).Any(s => s.Namespace == schemaNamespace);

IEnumerable<IEdmIncludeAnnotations> includeAnnotations = referencedCsdlModel.ParentModelReferences.SelectMany(s => s.IncludeAnnotations);
Copy link
Member

Choose a reason for hiding this comment

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

includeAnnotations [](start = 52, length = 18)

I'm concerned about the performance of executing this (somewhat expensive) SelectMany LINQ expression multiple times -- we execute it once on line 119, again on line 625, and then for each term on line 641. Probably be better, especially since we are iterating multiple times, to make this a list. removing the linq expression all together and simply iterating through building the collection would be even better, but probably not a significant savings here.

bool shouldAddAnnotations = includeAnnotations.Any();

if (shouldAddTypes || shouldAddAnnotations)
{
this.AddSchema(schema, false /*addAnnotations*/);
this.AddSchema(schema, shouldAddTypes, shouldAddAnnotations, includeAnnotations);
}

// TODO: REF add annotations
}
}

Expand Down Expand Up @@ -522,15 +523,40 @@ private IEdmVocabularyAnnotation WrapVocabularyAnnotation(CsdlAnnotation annotat
ann => new CsdlSemanticsVocabularyAnnotation(schema, targetContext, annotationsContext, ann, qualifier));
}

private void AddSchema(CsdlSchema schema)
private void AddSchema(CsdlSchema schema, bool addSchemaElements = true)
marabooy marked this conversation as resolved.
Show resolved Hide resolved
{
this.AddSchema(schema, true);
this.AddSchema(schema, addSchemaElements, true, null);
Copy link
Contributor

@gathogojr gathogojr Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
this.AddSchema(schema, addSchemaElements, true, null);
this.AddSchema(schema, addSchemaElements, true /*addAnnotations*/, null /*includeAnnotations*/);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I could use named parameters instead, like:

this.AddSchema(schema, addSchemaElements, addAnnotations: true, includeAnnotations: null);

Copy link
Contributor

Choose a reason for hiding this comment

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

@habbes Use John's suggestion. Named parameters aren't supported in some c# versions.

}

private void AddSchema(CsdlSchema schema, bool addAnnotations)
private void AddSchema(CsdlSchema schema, bool addSchemaElements, bool addAnnotations, IEnumerable<IEdmIncludeAnnotations> includeAnnotations = null)
{
CsdlSemanticsSchema schemaWrapper = new CsdlSemanticsSchema(this, schema);
this.schemata.Add(schemaWrapper);

if (addSchemaElements)
{
AddSchemaElements(schema, schemaWrapper);
}

if (!string.IsNullOrEmpty(schema.Alias))
{
this.SetNamespaceAlias(schema.Namespace, schema.Alias);
}

if (addAnnotations)
{
AddOutOfLineAnnotationsFromSchema(schema, schemaWrapper, includeAnnotations);
}

var edmVersion = this.GetEdmVersion();
if (edmVersion == null || edmVersion < schema.Version)
habbes marked this conversation as resolved.
Show resolved Hide resolved
{
this.SetEdmVersion(schema.Version);
}
}

private void AddSchemaElements(CsdlSchema schema, CsdlSemanticsSchema schemaWrapper)
{
foreach (IEdmSchemaType type in schemaWrapper.Types)
{
CsdlSemanticsStructuredTypeDefinition structuredType = type as CsdlSemanticsStructuredTypeDefinition;
Expand Down Expand Up @@ -574,34 +600,50 @@ private void AddSchema(CsdlSchema schema, bool addAnnotations)
{
RegisterElement(container);
}
}

if (!string.IsNullOrEmpty(schema.Alias))
private void AddOutOfLineAnnotationsFromSchema(CsdlSchema schema, CsdlSemanticsSchema schemaWrapper, IEnumerable<IEdmIncludeAnnotations> includeAnnotations)
{
foreach (CsdlAnnotations schemaOutOfLineAnnotations in schema.OutOfLineAnnotations)
{
this.SetNamespaceAlias(schema.Namespace, schema.Alias);
}
string target = this.ReplaceAlias(schemaOutOfLineAnnotations.Target);

if (addAnnotations)
{
foreach (CsdlAnnotations schemaOutOfLineAnnotations in schema.OutOfLineAnnotations)
List<CsdlSemanticsAnnotations> annotations;
if (!this.outOfLineAnnotations.TryGetValue(target, out annotations))
{
string target = this.ReplaceAlias(schemaOutOfLineAnnotations.Target);

List<CsdlSemanticsAnnotations> annotations;
if (!this.outOfLineAnnotations.TryGetValue(target, out annotations))
{
annotations = new List<CsdlSemanticsAnnotations>();
this.outOfLineAnnotations[target] = annotations;
}

annotations.Add(new CsdlSemanticsAnnotations(schemaWrapper, schemaOutOfLineAnnotations));
annotations = new List<CsdlSemanticsAnnotations>();
this.outOfLineAnnotations[target] = annotations;
}

CsdlAnnotations filteredAnnotations = FilterIncludedAnnotations(schemaOutOfLineAnnotations, target, includeAnnotations);
annotations.Add(new CsdlSemanticsAnnotations(schemaWrapper, filteredAnnotations));
}
}

var edmVersion = this.GetEdmVersion();
if (edmVersion == null || edmVersion < schema.Version)
private CsdlAnnotations FilterIncludedAnnotations(CsdlAnnotations csdlAnnotations, string target, IEnumerable<IEdmIncludeAnnotations> includeAnnotations)
{
if (includeAnnotations == null || !includeAnnotations.Any())
habbes marked this conversation as resolved.
Show resolved Hide resolved
{
this.SetEdmVersion(schema.Version);
return csdlAnnotations;
}

var filteredAnnotations = csdlAnnotations.Annotations.Where(annotation =>
ShouldIncludeAnnotation(annotation, target, includeAnnotations));
return new CsdlAnnotations(filteredAnnotations, csdlAnnotations.Target, csdlAnnotations.Qualifier);
Copy link
Member

Choose a reason for hiding this comment

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

csdlAnnotations.Target [](start = 60, length = 22)

should this use csdlAnnotations.Target, or "target" (which has the alias already resolved)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The aim here is to return a copy of the CsdlAnnotations instance but with the actual annotations filtered. So every other property should be identical to the original CsdlAnnotations instance, hence csdlAnnotations.Target. The target with the alias already resolved is used when comparing against the include.TargetNamespace.

}

private bool ShouldIncludeAnnotation(CsdlAnnotation annotation, string target, IEnumerable<IEdmIncludeAnnotations> includeAnnotations)
{
// include annotation if it matches one of the edmx:IncludeAnnotation references
// we check if the namespace matches, and (if applicable) also the target namespace and the qualifier
// see: http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_IncludedAnnotations

return includeAnnotations.Any(include =>
Copy link
Member

Choose a reason for hiding this comment

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

Any [](start = 38, length = 3)

LINQ is expensive -- AGS has gone through a LINQ hunt and removed all use of LINQ from their code. We should avoid it where we can.

Instead, it would be more efficient to iterate through the collection and evaluate the options.

(this.ReplaceAlias(annotation.Term).StartsWith(include.TermNamespace, System.StringComparison.Ordinal))
&& (string.IsNullOrEmpty(include.Qualifier) || annotation.Qualifier == include.Qualifier)
&& (string.IsNullOrEmpty(include.TargetNamespace)
|| target.StartsWith(include.TargetNamespace, System.StringComparison.Ordinal))
habbes marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
}
Expand Down
237 changes: 237 additions & 0 deletions test/FunctionalTests/Microsoft.OData.Edm.Tests/Csdl/CsdlReaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1230,5 +1230,242 @@ public void ValidateEdmxVersions(string odataVersion)
Assert.Empty(edmErrors);
Assert.Equal(edmModel.GetEdmVersion(), odataVersion == "4.0" ? EdmConstants.EdmVersion4 : EdmConstants.EdmVersion401);
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

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

Fact [](start = 9, length = 4)

Do you have any tests that validate referenced types (not annotations) from referenced namespace.

For example, in your tests, you could test loading permissionsCsdl and validating that it imported types from "Default" and "Exmaple.Types" from main.csdl, but didn't import types from main.csdl defined in 3rd namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the test.

public void ImportsAnnotationsFromExernalReferencedModel()
{
string permissionsCsdl = @"<?xml version=""1.0"" encoding=""utf-8""?>
<edmx:Edmx Version=""4.0"" xmlns:edmx=""http://docs.oasis-open.org/odata/ns/edmx"">
<edmx:Reference Uri=""SimpleModel.xml"">
<edmx:Include Namespace=""Default""/>
<edmx:Include Namespace=""Example.Types"" />
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a test where the annotation schema defines an alias for the referenced namespace that is used locally in specifying the target, and make sure that annotations restricted to that target are still (exclusively) included.

i.e., define an alias for the "Example.Types" namespace in permissions.Csdl (say "examples") and use it in the Annotations Target, and then in SimpleModel.xml specify a TargetNamespace of "Example.Types" in your include annotations and make sure it still returns the right set of annotations (I think your code handles that, just want to make sure it's in the tests...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added the test

</edmx:Reference >
<edmx:DataServices>
<Schema Namespace=""Example.Permissions"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<EntityType Name=""BasicType"">
<Key>
<PropertyRef Name=""Id"" />
</Key>
<Property Name=""Id"" Type=""Edm.Int32"" Nullable=""false"" />
</EntityType>
<Annotations Target=""Default.Container/Products"">
<Annotation Term=""NotIncludedNS.MyAnnotationPathTerm"" AnnotationPath=""abc/efg"" />
<Annotation Term=""Org.OData.Capabilities.V1.InsertRestrictions"">
<Record>
<PropertyValue Property=""Permissions"">
<Collection>
<Record>
<PropertyValue Property=""SchemeName"" String=""Scheme"" />
<PropertyValue Property=""Scopes"">
<Collection>
<Record>
<PropertyValue Property=""Scope"" String=""Product.Create"" />
</Record>
</Collection>
</PropertyValue>
</Record>
</Collection>
</PropertyValue>
</Record>
</Annotation>
<Annotation Term=""Org.OData.Capabilities.V1.DeleteRestrictions"">
<Record>
<PropertyValue Property=""Permissions"">
<Collection>
<Record>
<PropertyValue Property=""SchemeName"" String=""Scheme"" />
<PropertyValue Property=""Scopes"">
<Collection>
<Record>
<PropertyValue Property=""Scope"" String=""Product.Delete"" />
</Record>
</Collection>
</PropertyValue>
</Record>
</Collection>
</PropertyValue>
</Record>
</Annotation>
</Annotations>
</Schema>
</edmx:DataServices>
</edmx:Edmx>
";
string mainCsdl = @"<?xml version=""1.0"" encoding=""utf-8""?>
<edmx:Edmx Version=""4.0"" xmlns:edmx=""http://docs.oasis-open.org/odata/ns/edmx"">
<edmx:Reference Uri=""SimplePermissions.xml"">
<edmx:IncludeAnnotations TermNamespace=""Org.OData.Capabilities.V1""/>
</edmx:Reference >
<edmx:DataServices>
<Schema Namespace=""Example.Types"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<EntityType Name=""Product"">
<Key>
<PropertyRef Name=""Id"" />
</Key>
<Property Name=""Id"" Type=""Edm.Int32"" Nullable=""false"" />
<Property Name=""Name"" Type=""Edm.String"" />
<Property Name=""Price"" Type=""Edm.Int32"" Nullable=""false"" />
</EntityType>
</Schema>
<Schema Namespace=""Default"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<EntityContainer Name=""Container"">
<EntitySet Name=""Products"" EntityType=""Example.Types.Product"" />
</EntityContainer>
</Schema>
</edmx:DataServices>
</edmx:Edmx>
";

Func<Uri, XmlReader> getReferenceModelReader = uri =>
{
string csdl = uri.OriginalString == "SimplePermissions.xml" ? permissionsCsdl : mainCsdl;
var stringReader = new System.IO.StringReader(permissionsCsdl);
habbes marked this conversation as resolved.
Show resolved Hide resolved
return XmlReader.Create(stringReader);
};


var reader = XmlReader.Create(new System.IO.StringReader(mainCsdl));
IEdmModel model = CsdlReader.Parse(reader, getReferencedModelReaderFunc: getReferenceModelReader);

var entitySet = model.FindDeclaredEntitySet("Products");
var annotations = model.FindVocabularyAnnotations(entitySet);
Assert.Equal(2, annotations.Count());

// only imports annotations terms in the Org.OData.Capabilities.V1 namespace
IEdmVocabularyAnnotation insertRestrictions = annotations.FirstOrDefault(a => a.Term.Name == "InsertRestrictions");
Assert.NotNull(insertRestrictions);
IEdmVocabularyAnnotation deleteRestrictions = annotations.FirstOrDefault(a => a.Term.Name == "DeleteRestrictions");
Assert.NotNull(deleteRestrictions);

IEdmRecordExpression record = insertRestrictions.Value as IEdmRecordExpression;
Assert.NotNull(record);
var insertPermissions = record.FindProperty("Permissions").Value as IEdmCollectionExpression;
Assert.NotNull(insertPermissions);

var permission = insertPermissions.Elements.FirstOrDefault() as IEdmRecordExpression;
Assert.NotNull(permission);
var scheme = permission.FindProperty("SchemeName");
Assert.NotNull(scheme);
Assert.Equal("Scheme", ((IEdmStringConstantExpression)scheme.Value).Value);

// do not import other schema elements since there's
// no explicit <edmx:Include Namespace="Example.Permissions" /> declaration
var basicType = model.FindType("Example.Permissions.BasicType");
Assert.Null(basicType);
}

[Fact]
public void ImportsAnnotationsFromReferencedModelsOnlyIfTargetNamespaceAndQualifierMatch()
{
string permissionsCsdl = @"<?xml version=""1.0"" encoding=""utf-8""?>
<edmx:Edmx Version=""4.0"" xmlns:edmx=""http://docs.oasis-open.org/odata/ns/edmx"">
<edmx:Reference Uri=""SimpleModel.xml"">
<edmx:Include Namespace=""Default""/>
<edmx:Include Namespace=""Example.Types"" />
</edmx:Reference >
<edmx:DataServices>
<Schema Namespace=""Example.Permissions"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<Annotations Target=""Example.Types.Product"">
<Annotation Term=""OtherIncludedNS.SomeAnnotationPath"" AnnotationPath=""abc/efg"" />
</Annotations>
<Annotations Target=""Default.Container/Products"">
<Annotation Term=""OtherIncludedNS.SomeAnnotationPath"" AnnotationPath=""abc/efg"" />
<Annotation Term=""NotIncludedNS.MyAnnotationPathTerm"" AnnotationPath=""abc/efg"" />
<Annotation Term=""Org.OData.Capabilities.V1.InsertRestrictions"" Qualifier=""Insert"">
<Record>
<PropertyValue Property=""Permissions"">
<Collection>
<Record>
<PropertyValue Property=""SchemeName"" String=""Scheme"" />
<PropertyValue Property=""Scopes"">
<Collection>
<Record>
<PropertyValue Property=""Scope"" String=""Product.Create"" />
</Record>
</Collection>
</PropertyValue>
</Record>
</Collection>
</PropertyValue>
</Record>
</Annotation>
<Annotation Term=""Org.OData.Capabilities.V1.DeleteRestrictions"">
<Record>
<PropertyValue Property=""Permissions"">
<Collection>
<Record>
<PropertyValue Property=""SchemeName"" String=""Scheme"" />
<PropertyValue Property=""Scopes"">
<Collection>
<Record>
<PropertyValue Property=""Scope"" String=""Product.Delete"" />
</Record>
</Collection>
</PropertyValue>
</Record>
</Collection>
</PropertyValue>
</Record>
</Annotation>
</Annotations>
</Schema>
</edmx:DataServices>
</edmx:Edmx>
";
string mainCsdl = @"<?xml version=""1.0"" encoding=""utf-8""?>
<edmx:Edmx Version=""4.0"" xmlns:edmx=""http://docs.oasis-open.org/odata/ns/edmx"">
<edmx:Reference Uri=""SimplePermissions.xml"">
<edmx:IncludeAnnotations TermNamespace=""Org.OData.Capabilities.V1"" Qualifier=""Insert""/>
<edmx:IncludeAnnotations TermNamespace=""OtherIncludedNS"" TargetNamespace=""Example.Types""/>
</edmx:Reference >
<edmx:DataServices>
<Schema Namespace=""Example.Types"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<EntityType Name=""Product"">
<Key>
<PropertyRef Name=""Id"" />
</Key>
<Property Name=""Id"" Type=""Edm.Int32"" Nullable=""false"" />
<Property Name=""Name"" Type=""Edm.String"" />
<Property Name=""Price"" Type=""Edm.Int32"" Nullable=""false"" />
</EntityType>
</Schema>
<Schema Namespace=""Default"" xmlns=""http://docs.oasis-open.org/odata/ns/edm"">
<EntityContainer Name=""Container"">
<EntitySet Name=""Products"" EntityType=""ODataAuthorizationDemo.Models.Product"" />
</EntityContainer>
</Schema>
</edmx:DataServices>
</edmx:Edmx>
";

Func<Uri, XmlReader> getReferenceModelReader = uri =>
{
string csdl = uri.OriginalString == "SimplePermissions.xml" ? permissionsCsdl : mainCsdl;
var stringReader = new System.IO.StringReader(permissionsCsdl);
return XmlReader.Create(stringReader);
};


var reader = XmlReader.Create(new System.IO.StringReader(mainCsdl));
IEdmModel model = CsdlReader.Parse(reader, getReferencedModelReaderFunc: getReferenceModelReader);

var entitySet = model.FindDeclaredEntitySet("Products");
var entitySetAnnotations = model.FindVocabularyAnnotations(entitySet);
Assert.Single(entitySetAnnotations);


// imports annotation terms in the Org.OData.Capabilities.V1 namespace that match the qualifier Insert
IEdmVocabularyAnnotation insertRestrictions = entitySetAnnotations.FirstOrDefault(a => a.Term.Name == "InsertRestrictions");
Assert.NotNull(insertRestrictions);

// imports annotation terms in the OtherIncludedNS namespace that match the target namespace Example.Types
var product = model.FindDeclaredType("Example.Types.Product");
var productAnnotations = model.FindVocabularyAnnotations(product);
Assert.Single(productAnnotations);
IEdmVocabularyAnnotation productAnnotation = productAnnotations.FirstOrDefault(a => a.Term.Name == "SomeAnnotationPath");
Assert.NotNull(productAnnotation);
}


}
}
habbes marked this conversation as resolved.
Show resolved Hide resolved