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

Add the validation rules to the abstract types #1058

Merged
merged 4 commits into from Feb 7, 2018

Conversation

Projects
None yet
5 participants
@xuzhg
Member

xuzhg commented Jan 31, 2018

Issues

*See Issue #1027 & PR #1051

Description

Add the validation rules for the abstract types

  1. Edm.EntityType
  • cannot be used as the type of a singleton in an entity container because it doesn’t define a structure, which defeats the purpose of a singleton.
  • cannot be used as the type of an entity set because all entities in an entity set must have the same key fields to uniquely identify them within the set.
  • cannot be the base type of an entity type or complex type.
  1. Edm.ComplexType
  • cannot be the base type of an entity type or complex type.
  1. Edm.PrimitiveType
  • cannot be used as the type of a key property of an entity type.
  • cannot be used as the underlying type of a type definition or enumeration type.
  1. Collection(Edm.PrimitiveType) and Collection(Edm.ComplexType)
  • cannot be used as the type of a property.
  • cannot be used as the return type of a function.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@@ -536,10 +536,10 @@ public enum Microsoft.OData.Edm.EdmOnDeleteAction : int {
}
public enum Microsoft.OData.Edm.EdmPathTypeKind : int {
Annotation = 1

This comment has been minimized.

@robward-ms

robward-ms Feb 2, 2018

Contributor

This seems like a breaking change.

This comment has been minimized.

@xuzhg

xuzhg Feb 2, 2018

Member

EdmPathTypeKind is added in my previous PR. #1051 . It's not in public official release.

It's a breaking change for my previous PR, not a breaking change for the office release. :)

Thanks for catch this.

#Won't fix.

@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Feb 2, 2018

The only rule that I didn't see was that an entity type used in an entity set, singleton, or nav prop must not use any of the path types (directly, or through complex-typed properties). #Closed

/// Returns true if this reference refers to a path type.
/// </summary>
/// <param name="type">Type reference.</param>
/// <returns>This reference refers to a pah type.</returns>

This comment has been minimized.

@biaol-odata

This comment has been minimized.

@xuzhg

xuzhg Feb 2, 2018

Member

#Resolved.

/// <summary>
/// A string like "The 'Edm.PrimitiveType' cannot be used as the underlying type of '{0}' type '{1}'."
/// </summary>
internal static string EdmModel_Validator_Semantic_EdmPrimitiveTypeCannotBeUsedAsUnderlyingType(object p0, object p1) {

This comment has been minimized.

@biaol-odata

biaol-odata Feb 2, 2018

Member

looks like p0 is always "type definition" only one argument typeDefinition.FullName() is needed?
Please check the chained definitions as well.

This comment has been minimized.

@xuzhg

xuzhg Feb 2, 2018

Member

It can be used as "enumeration type" as well.
The rule is:
*The 'Edm.PrimitiveType' cannot be used as the underlying type of type definition or enumeration type. *

@xuzhg xuzhg changed the base branch from xuzhg-NavigationPropertyPath to master Feb 2, 2018

@xuzhg xuzhg changed the base branch from master to xuzhg-NavigationPropertyPath Feb 2, 2018

@xuzhg xuzhg changed the base branch from xuzhg-NavigationPropertyPath to master Feb 2, 2018

xuzhg added some commits Feb 2, 2018

add the validation rule for path type property in declaring type of e…
…ntity set, singleton or type of navigaiton property
@xuzhg

This comment has been minimized.

Member

xuzhg commented Feb 6, 2018

@mikepizzo I added the new rules for the path type property. Please take a look, thanks. #Closed

@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Feb 6, 2018

Do we have a general test that a (nullable) property of a complex type whose type is the same complex type (i.e., ), an entity containing such a recursive complex type, and a singleton/entity set/nav prop whose type is an entity containing such a recursive type, doesn't cause infinite recursion in validation? I see you handle it in code, but want to make sure we have a general test for it. #Closed

@xuzhg

This comment has been minimized.

Member

xuzhg commented Feb 6, 2018

@mikepizzo I added the new rules for the recursive type with path type property. Please take a look, thanks #Resolved

@mikepizzo

Looks good; thanks Sam!

EntityTypeOfEntitySetCannotBeEdmEntityType = 385,
/// <summary>
/// The dclaring type of navigation source cannot have path type property.

This comment has been minimized.

@AlanWong-MS

AlanWong-MS Feb 7, 2018

Contributor

typo: dclaring -> declaring

@xuzhg xuzhg merged commit 156840b into OData:master Feb 7, 2018

2 checks passed

continuous-integration/vsts The build trigger has fired.
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment