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

Support optional facets on type definitions #2791

Open
wants to merge 1 commit 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
32 changes: 32 additions & 0 deletions src/Microsoft.OData.Edm/Csdl/Parsing/Ast/CsdlTypeDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,48 @@ namespace Microsoft.OData.Edm.Csdl.Parsing.Ast
internal class CsdlTypeDefinition : CsdlNamedElement
{
private readonly string underlyingTypeName;
private readonly int? maxLength;
Copy link
Member

Choose a reason for hiding this comment

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

I can't understand it, in my opinion, the facts should go to its underly type of the type definition, right?
For example, maxLength is for the underlying "Edm.String" primitive type.

private readonly bool? isUnicode;
private readonly int? precision;
private readonly int? scale;
private readonly int? srid;

public CsdlTypeDefinition(string name, string underlyingTypeName, CsdlLocation location)
: base(name, location)
{
this.underlyingTypeName = underlyingTypeName;
}

public CsdlTypeDefinition(
string name,
string underlyingTypeName,
int? maxLength,
bool? isUnicode,
int? precision,
int? scale,
int? srid,
CsdlLocation location)
: this(name, underlyingTypeName, location)
{
this.maxLength = maxLength;
this.isUnicode = isUnicode;
this.precision = precision;
this.scale = scale;
this.srid = srid;
}

public string UnderlyingTypeName
{
get { return this.underlyingTypeName; }
}

public int? MaxLength => this.maxLength;
public bool? IsUnicode => this.isUnicode;

public int? Precision => this.precision;

public int? Scale => this.scale;

public int? Srid => this.srid;
}
}
8 changes: 7 additions & 1 deletion src/Microsoft.OData.Edm/Csdl/Parsing/CsdlDocumentParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,13 @@ private CsdlTypeDefinition OnTypeDefinitionElement(XmlElementInfo element, XmlEl
string name = Required(CsdlConstants.Attribute_Name);
string underlyingTypeName = RequiredType(CsdlConstants.Attribute_UnderlyingType);

return new CsdlTypeDefinition(name, underlyingTypeName, element.Location);
int? maxLength = OptionalMaxLength(CsdlConstants.Attribute_MaxLength);
bool? isUnicode = OptionalBoolean(CsdlConstants.Attribute_Unicode);
int? precision = OptionalInteger(CsdlConstants.Attribute_Precision);
int? scale = OptionalInteger(CsdlConstants.Attribute_Scale);
int? srid = OptionalSrid(CsdlConstants.Attribute_Srid, CsdlConstants.Default_UnspecifiedSrid);

return new CsdlTypeDefinition(name, underlyingTypeName, maxLength, isUnicode, precision, scale, srid, element.Location);
}

private CsdlNamedElement OnNavigationPropertyElement(XmlElementInfo element, XmlElementValueCollection childValues)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@

namespace Microsoft.OData.Edm.Csdl.CsdlSemantics
{
using System.Diagnostics;
using Microsoft.OData.Edm;
using Microsoft.OData.Edm.Csdl.Parsing.Ast;

/// <summary>
/// Provides semantics for CsdlEntitySet.
/// </summary>
[DebuggerDisplay("CsdlSemanticsEntitySet({Name})")]
internal class CsdlSemanticsEntitySet : CsdlSemanticsNavigationSource, IEdmEntitySet
{
public CsdlSemanticsEntitySet(CsdlSemanticsEntityContainer container, CsdlEntitySet entitySet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.OData.Edm.Csdl.CsdlSemantics
/// <summary>
/// Provides semantics for a CsdlNavigationProperty.
/// </summary>
[DebuggerDisplay("CsdlSemanticsNavigationProperty({Name})")]
internal class CsdlSemanticsNavigationProperty : CsdlSemanticsElement, IEdmNavigationProperty, IEdmCheckable
{
private readonly CsdlNavigationProperty navigationProperty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.OData.Edm.Csdl.CsdlSemantics
/// <summary>
/// Provides semantics for CsdlTypeDefinition.
/// </summary>
internal class CsdlSemanticsTypeDefinitionDefinition : CsdlSemanticsTypeDefinition, IEdmTypeDefinition, IEdmFullNamedElement
internal class CsdlSemanticsTypeDefinitionDefinition : CsdlSemanticsTypeDefinition, IEdmTypeDefinition, IEdmFullNamedElement, IEdmFacetedTypeDefinition
Copy link
Contributor

Choose a reason for hiding this comment

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

Since IEdmFacetedTypeDefinition implements IEdmTypeDefinition, do we need IEdmTypeDefinition here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KenitoInc I'd be more comfortable leaving it as is since there's no harm. If we merge IEdmFacetedTypeDefinition into IEdmTypeDefinition in the next major release, the action will be seamless.

{
private readonly CsdlSemanticsSchema context;
private readonly CsdlTypeDefinition typeDefinition;
Expand Down Expand Up @@ -74,6 +74,16 @@ public override CsdlElement Element
get { return this.typeDefinition; }
}

public int? MaxLength => this.typeDefinition.MaxLength;

public bool? IsUnicode => this.typeDefinition.IsUnicode;

public int? Precision => this.typeDefinition.Precision;

public int? Scale => this.typeDefinition.Scale;

public int? Srid => this.typeDefinition.Srid;

protected override IEnumerable<IEdmVocabularyAnnotation> ComputeInlineVocabularyAnnotations()
{
return this.Model.WrapInlineVocabularyAnnotations(this, this.context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1176,8 +1176,20 @@ internal override void WriteTypeDefinitionElementHeader(IEdmTypeDefinition typeD
// The type definition object MUST contain the member $Kind with a string value of TypeDefinition
this.jsonWriter.WriteRequiredProperty("$Kind", CsdlConstants.Element_TypeDefinition);

// The type definition object MUST contain he member $UnderlyingType.
// The type definition object MUST contain the member $UnderlyingType.
this.jsonWriter.WriteRequiredProperty("$UnderlyingType", typeDefinition.UnderlyingType, TypeDefinitionAsJson);

if (typeDefinition is IEdmFacetedTypeDefinition facetedTypeDefinition)
{
this.jsonWriter.WriteOptionalProperty("$MaxLength", facetedTypeDefinition.MaxLength);
Copy link
Member

Choose a reason for hiding this comment

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

we should use the underlyingType to write the facts

this.jsonWriter.WriteOptionalProperty("$Unicode", facetedTypeDefinition.IsUnicode);
this.jsonWriter.WriteOptionalProperty("$Precision", facetedTypeDefinition.Precision);
this.jsonWriter.WriteOptionalProperty("$Scale", facetedTypeDefinition.Scale);
if (facetedTypeDefinition.UnderlyingType.IsSpatial())
{
this.jsonWriter.WriteOptionalProperty("$SRID", facetedTypeDefinition.Srid, CsdlConstants.Default_UnspecifiedSrid);
}
}
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,18 @@ internal override void WriteTypeDefinitionElementHeader(IEdmTypeDefinition typeD
this.xmlWriter.WriteStartElement(CsdlConstants.Element_TypeDefinition);
this.WriteRequiredAttribute(CsdlConstants.Attribute_Name, typeDefinition.Name, EdmValueWriter.StringAsXml);
this.WriteRequiredAttribute(CsdlConstants.Attribute_UnderlyingType, typeDefinition.UnderlyingType, this.TypeDefinitionAsXml);

if (typeDefinition is IEdmFacetedTypeDefinition facetedTypeDefinition)
Copy link
Member

Choose a reason for hiding this comment

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

we should use the underlyingType to write the facts.

{
this.WriteOptionalAttribute(CsdlConstants.Attribute_MaxLength, facetedTypeDefinition.MaxLength, EdmValueWriter.IntAsXml);
this.WriteOptionalAttribute(CsdlConstants.Attribute_Unicode, facetedTypeDefinition.IsUnicode, EdmValueWriter.BooleanAsXml);
this.WriteOptionalAttribute(CsdlConstants.Attribute_Precision, facetedTypeDefinition.Precision, EdmValueWriter.IntAsXml);
this.WriteOptionalAttribute(CsdlConstants.Attribute_Scale, facetedTypeDefinition.Scale, EdmValueWriter.IntAsXml);
if (facetedTypeDefinition.UnderlyingType.IsSpatial())
{
this.WriteOptionalAttribute(CsdlConstants.Attribute_Srid, facetedTypeDefinition.Srid, CsdlConstants.Default_UnspecifiedSrid, SridAsXml);
}
}
}

internal override void WriteEndElement()
Expand Down
17 changes: 17 additions & 0 deletions src/Microsoft.OData.Edm/Csdl/Utf8JsonWriterExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,23 @@ public static void WriteOptionalProperty(this Utf8JsonWriter writer, string name
}
}

/// <summary>
/// Write the optional property (name/value).
/// </summary>
/// <param name="writer">The JSON writer.</param>
/// <param name="name">The property name.</param>
/// <param name="value">The property value.</param>
public static void WriteOptionalProperty(this Utf8JsonWriter writer, string name, bool? value)
{
EdmUtil.CheckArgumentNull(writer, nameof(writer));

if (value != null)
{
writer.WritePropertyName(name);
writer.WriteBooleanValue(value.Value);
}
}

/// <summary>
/// Write the optional property (name/value).
/// </summary>
Expand Down
12 changes: 11 additions & 1 deletion src/Microsoft.OData.Edm/Schema/BadTypeDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.OData.Edm
/// <summary>
/// Represents a semantically invalid EDM type definition.
/// </summary>
internal class BadTypeDefinition : BadType, IEdmTypeDefinition, IEdmFullNamedElement
internal class BadTypeDefinition : BadType, IEdmTypeDefinition, IEdmFullNamedElement, IEdmFacetedTypeDefinition
{
private readonly string namespaceName;
private readonly string name;
Expand Down Expand Up @@ -58,5 +58,15 @@ public string FullName
{
get { return this.fullName; }
}

public int? MaxLength => null;

public bool? IsUnicode => null;

public int? Precision => null;

public int? Scale => null;

public int? Srid => null;
}
}
59 changes: 58 additions & 1 deletion src/Microsoft.OData.Edm/Schema/EdmTypeDefinition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,17 @@ namespace Microsoft.OData.Edm
/// <summary>
/// Represents the definition of an Edm type definition.
/// </summary>
public class EdmTypeDefinition : EdmType, IEdmTypeDefinition, IEdmFullNamedElement
public class EdmTypeDefinition : EdmType, IEdmTypeDefinition, IEdmFullNamedElement, IEdmFacetedTypeDefinition
{
private readonly IEdmPrimitiveType underlyingType;
private readonly string namespaceName;
private readonly string name;
private readonly string fullName;
private readonly int? maxLength;
private readonly bool? isUnicode;
private readonly int? precision;
private readonly int? scale;
private readonly int? srid;

/// <summary>
/// Initializes a new instance of the <see cref="EdmTypeDefinition"/> class with <see cref="EdmPrimitiveTypeKind.Int32"/> underlying type.
Expand Down Expand Up @@ -45,6 +50,43 @@ public EdmTypeDefinition(string namespaceName, string name, IEdmPrimitiveType un
this.fullName = EdmUtil.GetFullNameForSchemaElement(this.namespaceName, this.name);
}

/// <summary>
/// Initializes a new instance of the <see cref="EdmTypeDefinition"/> class with
/// <see cref="EdmPrimitiveTypeKind.Int32"/> underlying type and optional type definition facets.
/// </summary>
/// <param name="namespaceName">Namespace this type definition belongs to.</param>
/// <param name="name">Name of this type definition.</param>
/// <param name="underlyingType">The underlying type of this type definition.</param>
/// <param name="maxLength">Optional type definition facet indicating the maximum length
/// of a string property on a type instance.</param>
/// <param name="isUnicode">Optional type definition facet indicating whether a string
/// property might contain and accept string values with Unicode characters beyond
/// the ASCII character set.</param>
/// <param name="precision">Optional type definition facet indicating the maximum
/// number of significant decimal digits allowed on a decimal property's value, or the number
/// of decimal places allowed in the seconds portion on a temporal property's value.</param>
/// <param name="scale">Optional type definition facet indicating the maximum
/// digits allowed on the right of the decimal point on a decimal property's value.</param>
/// <param name="srid">Optional type definition facet indicating the spatial reference system
/// applied on a geometry or geography property's value on a type instance.</param>
public EdmTypeDefinition(
string namespaceName,
string name,
IEdmPrimitiveType underlyingType,
int? maxLength,
bool? isUnicode,
int? precision,
int? scale,
int? srid)
: this(namespaceName, name, underlyingType)
{
this.maxLength = maxLength;
this.isUnicode = isUnicode;
this.precision = precision;
this.scale = scale;
this.srid = srid;
}

/// <summary>
/// Gets the kind of this type.
/// </summary>
Expand Down Expand Up @@ -92,5 +134,20 @@ public IEdmPrimitiveType UnderlyingType
{
get { return this.underlyingType; }
}

/// <inheritdoc/>
public int? MaxLength => this.maxLength;

/// <inheritdoc/>
public bool? IsUnicode => this.isUnicode;

/// <inheritdoc/>
public int? Precision => this.precision;

/// <inheritdoc/>
public int? Scale => this.scale;

/// <inheritdoc/>
public int? Srid => this.srid;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
//---------------------------------------------------------------------
// <copyright file="IEdmFacetedTypeDefinition.cs" company="Microsoft">
// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information.
// </copyright>
//---------------------------------------------------------------------

namespace Microsoft.OData.Edm
{
/// <summary>
/// Represents a definition of an EDM type definition that supports facets.
/// </summary>
public interface IEdmFacetedTypeDefinition : IEdmTypeDefinition
{
/// <summary>
/// Gets the optional maximum length type definition facet.
/// </summary>
int? MaxLength { get; }

/// <summary>
/// Gets the optional unicode type definition facet.
/// </summary>
bool? IsUnicode { get; }

/// <summary>
/// Gets the optional precision type definition facet.
/// </summary>
int? Precision { get; }

/// <summary>
/// Gets the optional scale type definition facet.
/// </summary>
int? Scale { get; }

/// <summary>
/// Gets the optional spatial reference identifier type definition facet.
/// </summary>
int? Srid { get; }
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Edm/Vocabularies/CoreVocabularies.xml
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ Any simple identifier | Any type listed in `Validation.OpenPropertyTypeConstrain
<Annotation Term="Core.Description" String="A symbolic name for a model element" />
</Term>

<TypeDefinition Name="SimpleIdentifier" UnderlyingType="Edm.String">
<TypeDefinition Name="SimpleIdentifier" UnderlyingType="Edm.String" MaxLength="128">
<Annotation Term="Core.Description" String="A [simple identifier](https://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_SimpleIdentifier)" />
<Annotation Term="Validation.Pattern" String="^[\p{L}\p{Nl}_][\p{L}\p{Nl}\p{Nd}\p{Mn}\p{Mc}\p{Pc}\p{Cf}]{0,}$" />
</TypeDefinition>
Expand Down
Loading
Loading