Skip to content

Commit

Permalink
Use target path annotations to determine whether a path item should b…
Browse files Browse the repository at this point in the history 
…e generated (#533)

* Use target path annotations to determine whether a path item should be added

* Reduce cognitive complexity

* Minor updates for code quality

* Add tests

* Bump version

* Add more tests
  • Loading branch information
@millicentachieng
millicentachieng authored May 27, 2024
1 parent 18883fe commit 41929b5
Show file tree
Hide file tree
Showing 40 changed files with 568 additions and 382 deletions.
13 changes: 13 additions & 0 deletions src/Microsoft.OpenApi.OData.Reader/Edm/EdmAnnotationExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ internal static class EdmVocabularyAnnotationExtensions
});
}

public static bool? GetBoolean(this IEdmModel model, string targetPath, string qualifiedName)
{
Utils.CheckArgumentNull(model, nameof(model));
Utils.CheckArgumentNull(targetPath, nameof(targetPath));
Utils.CheckArgumentNull(qualifiedName, nameof(qualifiedName));

IEdmTargetPath target = model.GetTargetPath(targetPath);
if (target == null)
return default;

return model.GetBoolean(target, qualifiedName);
}

/// <summary>
/// Gets the string term value for the given <see cref="IEdmVocabularyAnnotatable"/>.
/// </summary>
Expand Down
24 changes: 22 additions & 2 deletions src/Microsoft.OpenApi.OData.Reader/Edm/ODataPath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ public string GetPathItemName(OpenApiConvertSettings settings)
Utils.CheckArgumentNull(settings, nameof(settings));

// From Open API spec, parameter name is case sensitive, so don't use the IgnoreCase HashSet.
// HashSet<string> parameters = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
HashSet<string> parameters = new();
StringBuilder sb = new();

Expand Down Expand Up @@ -238,7 +237,6 @@ internal IDictionary<ODataSegment, IDictionary<string, string>> CalculateParamet
IDictionary<ODataSegment, IDictionary<string, string>> parameterMapping = new Dictionary<ODataSegment, IDictionary<string, string>>();

// From Open API spec, parameter name is case sensitive, so don't use the IgnoreCase HashSet.
// HashSet<string> parameters = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
HashSet<string> parameters = new HashSet<string>();

foreach (var segment in Segments)
Expand All @@ -259,6 +257,28 @@ internal IDictionary<ODataSegment, IDictionary<string, string>> CalculateParamet
return parameterMapping;
}

/// <summary>
/// Get string representation of the Edm Target Path for annotations
/// </summary>
/// <param name="model">The Edm model.</param>
/// <returns>The string representation of the Edm target path.</returns>
internal string GetTargetPath(IEdmModel model)
{
Utils.CheckArgumentNull(model, nameof(model));

var targetPath = new StringBuilder(model.EntityContainer.FullName());

bool skipLastSegment = LastSegment is ODataRefSegment
|| LastSegment is ODataDollarCountSegment
|| LastSegment is ODataStreamContentSegment;
foreach (var segment in Segments.Where(segment => segment is not ODataKeySegment
&& !(skipLastSegment && segment == LastSegment)))
{
targetPath.Append($"/{segment.Identifier}");
}
return targetPath.ToString();
}

/// <summary>
/// Output the path string.
/// </summary>
Expand Down
46 changes: 34 additions & 12 deletions src/Microsoft.OpenApi.OData.Reader/Edm/ODataPathProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ private void RetrieveNavigationSourcePaths(IEdmNavigationSource navigationSource
// for entity set, create a path with key and a $count path
if (entitySet != null)
{
count = _model.GetRecord<CountRestrictionsType>(entitySet, CapabilitiesConstants.CountRestrictions);
string targetPath = path.GetTargetPath(_model);
count = _model.GetRecord<CountRestrictionsType>(targetPath, CapabilitiesConstants.CountRestrictions)
?? _model.GetRecord<CountRestrictionsType>(entitySet, CapabilitiesConstants.CountRestrictions);
if(count?.Countable ?? true) // ~/entitySet/$count
CreateCountPath(path, convertSettings);

Expand Down Expand Up @@ -310,16 +312,22 @@ private void RetrieveComplexPropertyPaths(IEdmEntityType entityType, ODataPath c
.Where(x => x.Type.IsComplex() ||
x.Type.IsCollection() && x.Type.Definition.AsElementType() is IEdmComplexType))
{
if (!ShouldCreateComplexPropertyPaths(sp, convertSettings)) continue;

currentPath.Push(new ODataComplexPropertySegment(sp));
var targetPath = currentPath.GetTargetPath(_model);
if (!ShouldCreateComplexPropertyPaths(sp, targetPath, convertSettings))
{
currentPath.Pop();
continue;
}
AppendPath(currentPath.Clone());

if (sp.Type.IsCollection())
{
CreateTypeCastPaths(currentPath, convertSettings, sp.Type.Definition.AsElementType() as IEdmComplexType, sp, true);
var count = _model.GetRecord<CountRestrictionsType>(sp, CapabilitiesConstants.CountRestrictions);
if (count?.IsCountable ?? true)
var isCountable = _model.GetRecord<CountRestrictionsType>(targetPath, CapabilitiesConstants.CountRestrictions)?.IsCountable
?? _model.GetRecord<CountRestrictionsType>(sp, CapabilitiesConstants.CountRestrictions)?.IsCountable
?? true;
if (isCountable)
CreateCountPath(currentPath, convertSettings);
}
else
Expand Down Expand Up @@ -363,7 +371,9 @@ private bool RetrieveComplexTypeNavigationPropertyPaths(IEdmComplexType complexT

foreach (var np in navigationProperties)
{
var count = _model.GetRecord<CountRestrictionsType>(np, CapabilitiesConstants.CountRestrictions);
var targetPath = currentPath.GetTargetPath(_model);
var count = _model.GetRecord<CountRestrictionsType>(targetPath, CapabilitiesConstants.CountRestrictions)
?? _model.GetRecord<CountRestrictionsType>(np, CapabilitiesConstants.CountRestrictions);
RetrieveNavigationPropertyPaths(np, count, currentPath, convertSettings);
}

Expand Down Expand Up @@ -407,19 +417,26 @@ private void TraverseComplexProperty(IEdmStructuralProperty structuralProperty,
/// Evaluates whether or not to create paths for complex properties.
/// </summary>
/// <param name="complexProperty">The target complex property.</param>
/// <param name="targetPath">The annotation target path for the complex property.</param>
/// <param name="convertSettings">The settings for the current conversion.</param>
/// <returns>true or false.</returns>
private bool ShouldCreateComplexPropertyPaths(IEdmStructuralProperty complexProperty, OpenApiConvertSettings convertSettings)
private bool ShouldCreateComplexPropertyPaths(IEdmStructuralProperty complexProperty, string targetPath, OpenApiConvertSettings convertSettings)
{
Utils.CheckArgumentNull(complexProperty, nameof(complexProperty));
Utils.CheckArgumentNull(convertSettings, nameof(convertSettings));

if (!convertSettings.RequireRestrictionAnnotationsToGenerateComplexPropertyPaths)
return true;

bool isReadable = _model.GetRecord<ReadRestrictionsType>(complexProperty, CapabilitiesConstants.ReadRestrictions)?.Readable ?? false;
bool isUpdatable = _model.GetRecord<UpdateRestrictionsType>(complexProperty, CapabilitiesConstants.UpdateRestrictions)?.Updatable ?? false;
bool isInsertable = _model.GetRecord<InsertRestrictionsType>(complexProperty, CapabilitiesConstants.InsertRestrictions)?.Insertable ?? false;
bool isReadable = _model.GetRecord<ReadRestrictionsType>(targetPath, CapabilitiesConstants.ReadRestrictions)?.Readable
?? _model.GetRecord<ReadRestrictionsType>(complexProperty, CapabilitiesConstants.ReadRestrictions)?.Readable
?? false;
bool isUpdatable = _model.GetRecord<UpdateRestrictionsType>(targetPath, CapabilitiesConstants.UpdateRestrictions)?.Updatable
??_model.GetRecord<UpdateRestrictionsType>(complexProperty, CapabilitiesConstants.UpdateRestrictions)?.Updatable
?? false;
bool isInsertable = _model.GetRecord<InsertRestrictionsType>(targetPath, CapabilitiesConstants.InsertRestrictions)?.Insertable
?? _model.GetRecord<InsertRestrictionsType>(complexProperty, CapabilitiesConstants.InsertRestrictions)?.Insertable
?? false;

return isReadable || isUpdatable || isInsertable;
}
Expand Down Expand Up @@ -525,9 +542,13 @@ private void RetrieveNavigationPropertyPaths(
AppendPath(currentPath.Clone());
visitedNavigationProperties.Push(navPropFullyQualifiedName);

// For fetching annotations
var targetPath = currentPath.GetTargetPath(_model);

// Check whether a collection-valued navigation property should be indexed by key value(s).
// Find indexability annotation annotated directly via NavigationPropertyRestriction.
bool? annotatedIndexability = _model.GetBoolean(navigationProperty, CapabilitiesConstants.IndexableByKey);
bool? annotatedIndexability = _model.GetBoolean(targetPath, CapabilitiesConstants.IndexableByKey)
?? _model.GetBoolean(navigationProperty, CapabilitiesConstants.IndexableByKey);
bool indexableByKey = true;

if (restriction?.IndexableByKey != null)
Expand All @@ -550,7 +571,8 @@ private void RetrieveNavigationPropertyPaths(
if (count == null)
{
// First, get the directly annotated restriction annotation of the navigation property
count = _model.GetRecord<CountRestrictionsType>(navigationProperty, CapabilitiesConstants.CountRestrictions);
count = _model.GetRecord<CountRestrictionsType>(targetPath, CapabilitiesConstants.CountRestrictions)
?? _model.GetRecord<CountRestrictionsType>(navigationProperty, CapabilitiesConstants.CountRestrictions);
createCountPath = count?.Countable;
}

Expand Down
68 changes: 67 additions & 1 deletion src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiParameterGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,6 @@ public static OpenApiParameter CreateSearch(this ODataContext context, IEdmVocab

return null;
}

/// <summary>
/// Create the $search parameter for Edm target path.
/// </summary>
Expand Down Expand Up @@ -558,6 +557,18 @@ public static OpenApiParameter CreateFilter(this ODataContext context, string ta
return context.CreateFilter(target);
}

public static OpenApiParameter CreateOrderBy(this ODataContext context, string targetPath, IEdmEntityType entityType)
{
Utils.CheckArgumentNull(context, nameof(context));
Utils.CheckArgumentNull(targetPath, nameof(targetPath));

IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateOrderBy(target, entityType);
}

public static OpenApiParameter CreateOrderBy(this ODataContext context, IEdmEntitySet entitySet)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down Expand Up @@ -592,7 +603,17 @@ public static OpenApiParameter CreateOrderBy(this ODataContext context, IEdmNavi
public static OpenApiParameter CreateOrderBy(this ODataContext context, IEdmVocabularyAnnotatable target, IEdmEntityType entityType)
{// patchwork to avoid breaking changes
return context.CreateOrderBy(target, entityType as IEdmStructuredType);
}

public static OpenApiParameter CreateOrderBy(this ODataContext context, string targetPath, IEdmStructuredType structuredType)
{
IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateOrderBy(target, structuredType);
}

public static OpenApiParameter CreateOrderBy(this ODataContext context, IEdmVocabularyAnnotatable target, IEdmStructuredType structuredType)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down Expand Up @@ -653,6 +674,18 @@ public static OpenApiParameter CreateOrderBy(this ODataContext context, IEdmVoca
};
}

public static OpenApiParameter CreateSelect(this ODataContext context, string targetPath, IEdmEntityType entityType)
{
Utils.CheckArgumentNull(context, nameof(context));
Utils.CheckArgumentNull(targetPath, nameof(targetPath));

IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateSelect(target, entityType);
}

public static OpenApiParameter CreateSelect(this ODataContext context, IEdmEntitySet entitySet)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down Expand Up @@ -688,6 +721,16 @@ public static OpenApiParameter CreateSelect(this ODataContext context, IEdmVocab
{ // patchwork to avoid breaking changes
return context.CreateSelect(target, entityType as IEdmStructuredType);
}

public static OpenApiParameter CreateSelect(this ODataContext context, string targetPath, IEdmStructuredType structuredType)
{
IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateSelect(target, structuredType);
}

public static OpenApiParameter CreateSelect(this ODataContext context, IEdmVocabularyAnnotatable target, IEdmStructuredType structuredType)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down Expand Up @@ -736,6 +779,19 @@ public static OpenApiParameter CreateSelect(this ODataContext context, IEdmVocab
Explode = false
};
}

public static OpenApiParameter CreateExpand(this ODataContext context, string targetPath, IEdmEntityType entityType)
{
Utils.CheckArgumentNull(context, nameof(context));
Utils.CheckArgumentNull(targetPath, nameof(targetPath));

IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateExpand(target, entityType);
}

public static OpenApiParameter CreateExpand(this ODataContext context, IEdmEntitySet entitySet)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down Expand Up @@ -771,6 +827,16 @@ public static OpenApiParameter CreateExpand(this ODataContext context, IEdmVocab
{ // patchwork to avoid breaking changes
return context.CreateExpand(target, entityType as IEdmStructuredType);
}

public static OpenApiParameter CreateExpand(this ODataContext context, string targetPath, IEdmStructuredType structuredType)
{
IEdmTargetPath target = context.Model.GetTargetPath(targetPath);
if (target == null)
return null;

return context.CreateExpand(target, structuredType);
}

public static OpenApiParameter CreateExpand(this ODataContext context, IEdmVocabularyAnnotatable target, IEdmStructuredType structuredType)
{
Utils.CheckArgumentNull(context, nameof(context));
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<PackageId>Microsoft.OpenApi.OData</PackageId>
<SignAssembly>true</SignAssembly>
<Version>1.6.4</Version>
<Version>1.6.5</Version>
<Description>This package contains the codes you need to convert OData CSDL to Open API Document of Model.</Description>
<Copyright>© Microsoft Corporation. All rights reserved.</Copyright>
<PackageTags>Microsoft OpenApi OData EDM</PackageTags>
<RepositoryUrl>https://github.com/Microsoft/OpenAPI.NET.OData</RepositoryUrl>
<PackageReleaseNotes>
- Add support for Edm.Untyped #511
- Use annotations with path as target to dermine whether to add an operation #535
</PackageReleaseNotes>
<AssemblyName>Microsoft.OpenApi.OData.Reader</AssemblyName>
<AssemblyOriginatorKeyFile>..\..\tool\Microsoft.OpenApi.OData.snk</AssemblyOriginatorKeyFile>
Expand Down
20 changes: 8 additions & 12 deletions src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyGetOperationHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,8 @@ protected override void Initialize(ODataContext context, ODataPath path)

_readRestrictions = Context.Model.GetRecord<ReadRestrictionsType>(TargetPath, CapabilitiesConstants.ReadRestrictions);
var complexPropertyReadRestrictions = Context.Model.GetRecord<ReadRestrictionsType>(ComplexPropertySegment.Property, CapabilitiesConstants.ReadRestrictions);

if (_readRestrictions == null)
{
_readRestrictions = complexPropertyReadRestrictions;
}
else
{
_readRestrictions.MergePropertiesIfNull(complexPropertyReadRestrictions);
}
_readRestrictions?.MergePropertiesIfNull(complexPropertyReadRestrictions);
_readRestrictions ??= complexPropertyReadRestrictions;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -108,22 +101,25 @@ protected override void SetParameters(OpenApiOperation operation)
// of just providing a comma-separated list of properties can be expressed via an array-valued
// parameter with an enum constraint
// $order
parameter = Context.CreateOrderBy(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
parameter = Context.CreateOrderBy(TargetPath, ComplexPropertySegment.ComplexType)
?? Context.CreateOrderBy(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
if (parameter != null)
{
operation.Parameters.Add(parameter);
}
}

// $select
parameter = Context.CreateSelect(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
parameter = Context.CreateSelect(TargetPath, ComplexPropertySegment.ComplexType)
?? Context.CreateSelect(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
if (parameter != null)
{
operation.Parameters.Add(parameter);
}

// $expand
parameter = Context.CreateExpand(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
parameter = Context.CreateExpand(TargetPath, ComplexPropertySegment.ComplexType)
?? Context.CreateExpand(ComplexPropertySegment.Property, ComplexPropertySegment.ComplexType);
if (parameter != null)
{
operation.Parameters.Add(parameter);
Expand Down
11 changes: 2 additions & 9 deletions src/Microsoft.OpenApi.OData.Reader/Operation/ComplexPropertyPostOperationHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,8 @@ protected override void Initialize(ODataContext context, ODataPath path)

_insertRestrictions = Context.Model.GetRecord<InsertRestrictionsType>(TargetPath, CapabilitiesConstants.InsertRestrictions);
var complexPropertyInsertRestrictions = Context.Model.GetRecord<InsertRestrictionsType>(ComplexPropertySegment.Property, CapabilitiesConstants.InsertRestrictions);

if (_insertRestrictions == null)
{
_insertRestrictions = complexPropertyInsertRestrictions;
}
else
{
_insertRestrictions.MergePropertiesIfNull(complexPropertyInsertRestrictions);
}
_insertRestrictions?.MergePropertiesIfNull(complexPropertyInsertRestrictions);
_insertRestrictions ??= complexPropertyInsertRestrictions;
}
/// <inheritdoc />
public override OperationType OperationType => OperationType.Post;
Expand Down
Loading

0 comments on commit 41929b5

Please sign in to comment.