raml-org / raml-spec

RAML Specification
http://raml.org
3.87k stars 859 forks source link

Adding capability to restrict properties on inherited object types #589

Open araguacaima opened 7 years ago

araguacaima commented 7 years ago

Even when an entity should be always the same, we do not always use it entirety in all use cases. A typical case of what I try to explain is the difference between the input and output of any endpoint with a POST method. For these, typically the "id" attribute of the entity is not provided at the POST input, but is usually returned at the output. For both, the entity should be the same, however it would be good to denote that the input for that POST call is forbidden to enter the "id". If we would like to use the same entity (with no duplication), how could we make an exception for the "id" attribute in the case of the input in a POST call?

My proposal is as follows: Adding some modifier that allows to "subtract", "restrict", "omit" or "not consider as an existing or valid" those attributes that belongs to the parent entity and not directly to the child. In other words, considerer that not all attributes of the parent are automatically inherited to the children. Thus, it is possible to define a child for the input of the "POST", and another for the output, but both inheriting from the same base entity, something like this:

entity.raml

#%RAML 1.0 Library

types:
  entity: # This is the base entity for both input and output post call
    type: object
    properties:
      entityId?:
        type: string
        description: Entity identifier.
      field?:
        type: boolean
        description: Field.

entity-child.raml

#%RAML 1.0 Library

uses:
  entity: entity.raml

types:
  post.in:  # This is the inherited entity which determines the specific structure for the input of the post call
    type: entity.entity
    properties:
      field:
  post.out:  # This is the inherited entity which determines the specific structure for the output of the post call
    type: entity.entity
    properties:
      entityId:
      field?:

The problem here is that, right now, by inheriting both the same base entity, all attributes of the parent entity are inherited to "entity.post.in" and "entity.post.out" children. Regardless of the fact that "entity.post.in" does not define the "entityId" attribute, it is absolutely valid because it is declared by the parent.

In order to reach the behavior mentioned above, I propose to add the facet "heritable" at "Object Type Declarations" level with the following meaning:

heritable: A boolean that indicates if this type is inherited automatically or if it should be explicitly declared on every type that inherit it in order to consider it as a part of it. True means the child does not need to overwrite it in order to inherite it, it is inherited automatically. False means that if the child does not overwrite it explicitly, the same won't be available on it, thus it's not usable by it, in other words, the property is not inherited automatically. Default value: true

Use Case:

types:
  entity:
    type: object
    properties:
      field1?:
        type: string
        description: field 1.
      field2?:
        type: boolean
        description: field 2.
      field3?:
        type: string
        description: field 3.   
        heritable: true
    heritable: false

  inheritedEntity:
    type: entity
    properties:
      field1:
      field4: number
        description: field 4.

Examples:

entity:         # valid
  field1: "John"
  field2: true
  field3: "Doe"   

inheritedEntity:    # invalid because of heritable is false for entity which means by default all attributes are excluded at least it is explicitly declared on the child, and field2 is not.
  field1: "John"   
  field2: true  
  field4: 1   

inheritedEntity:    # valid because of the only available attributes are: entity.field3 and inheritedEntity.field4 and entity.field1 because of it is overwritten. The entity.field2 is not usable because it is excluded in the parent and no overwritten in the child
  field1: "John"
  field3: "Doe"     
  field4: 1  

inheritedEntity:    # invalid because of  entity.field2 is not usable because it is excluded in the parent and no overwritten in the child
  field2: true
  field3: "Doe"     
  field4: 1 
sichvoge commented 7 years ago

I remember discussions around something similar and add final for properties that cannot be inherited. It comes from OO languages and probably has the same meaning as heritable. Is that right

@petrochenko-pavel-a @usarid do you remember where we end up with this?

araguacaima commented 7 years ago

You're absolutelly right Cristian, it means basically the same that the final modifier in many OO languages.

Thank you very much for your interest on this proposal. I would have no problem in changing the approach from "heritable" to "final" if necessary, perhaps that would be even more understandable for those coming from the world OO

Best regards

El 23/1/2017 7:01, "Christian Vogel" notifications@github.com escribió:

I remember discussions around something similar and add final for properties that cannot be inherited. It comes from OO languages and probably has the same meaning as heritable. Is that right?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/raml-org/raml-spec/issues/589#issuecomment-274482209, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7zWaNhhOcBuqxxmR8z-fSG5JXBu9e1ks5rVKSMgaJpZM4K2p-M .

sichvoge commented 7 years ago

Sure. I moved it into idea to see what others say about that. Let's make sure we hear other opinions as well.

I believe that the only problem here would be to translate the semantics of final into other schema languages. It would basically get lost since non of the most common, JSON and XSD, have that as well. Might be wrong tho :D

KevinMitchell commented 7 years ago

This seems an odd use of the term 'final' as a modifier. Doesn't this normally mean a definition can't be changed, not that the definition is not visible in the subclass?

I raised a question a while ago in the RAML forum that might be relevant?

http://forums.raml.org/t/required-properties-and-resource-collections/1199

In the "What's New in RAML 1.0" page they show how to use a collection of types, and multiple inheritance, to capture some of the differences you are interested in. This seemed to work OK for their example. In my case it was supporting the PATCH operation that seemed problematic. The types for a GET, POST and PATCH are all slightly different, and it seems hard to accurately type these without a lot of repetition. As you have observed, some properties shouldn't be specified in a POST, but should be returned by a GET. But also some properties should be required in a GET response, but optional in a PATCH request (assuming we are supporting partial updates). As a subtype can't make a required property in a supertype optional, in the current language it's hard to see how to build a type hierarchy that accurately captures all these differences without a lot of repetition. So you end up either giving up on accurate typing, or else creating a family of types, one for each context the type is used in, without using inheritance. Which leads to very verbose specs that are hard to maintain.

So any proposal in this space should perhaps also consider the interaction of required properties and PATCH operations? And how to maximise the use of inheritance whilst keeping the distinctions we need. In small examples, repeating the properties in a subclass (to make optional ones required for example) isn't too hard. But we need to consider the more realistic case where there may be large numbers of properties. Maybe we need a variety of inheritance mechanisms. For example, maybe we should be able to inherit from a type, but where we declare that all optional properties in the supertype should be required in the subtype. Or inherit from a type, but explicitly declare certain supertype properties should be hidden in the subtype (which presumably would have a similar effect to what you are trying to do, but specified on the subtype rather than the supertype).

My concern with all these kinds of things is that they probably don't play nicely with the normal semantics of inheritance. The family of types we need to accurately capture all the different contexts a "type" will be used in perhaps don't have a neat inheritance hierarchy in the conventional sense. Perhaps most of them really are just different types. Maybe that's not a bad thing. After all, unlike in traditional OO domains, we aren't trying to build a list of "entities" which may contain a mixture of "GET" entities and "POST" entities etc. So perhaps we don't need inheritance here in the traditional sense. We just need ways to build a collection of unrelated types (from a type hierarchy perspective) but where we want to reuse the definitions of sets of properties, but transforming them as we include them in various ways (required to optional, excluding some of them etc).

sichvoge commented 7 years ago

I agree. There are a lot use cases where someone might get to focused on providing types with inheritance and tries to cover all sorts of scenarios. I think that might be too much and they should potential think about creating separated types for that.

Do you see the use case you mentioned very often?

KevinMitchell commented 7 years ago

I think most interesting types will end up being used in slightly different ways depending on whether they are used in a POST request, a GET response, a PATCH partial update etc. And trying to accurately capture these differences in a traditional inheritance hierarchy really doesn't work in many cases. But I'm not convinced inheritance is the way to go here anyway. I presume when generating client or server stubs from the spec we wouldn't really want to generate multiple classes anyway; we'd just have a single class with context-specific validation, serialization etc. And whilst we could work around the inheritance problems by just creating a set of disjoint types, this would create a maintenance problem, and would still result in multiple types being generating in any stub code. So at the moment we end up having to compromise, with inaccurate typing, and comments in the spec that describe the context-specific constraints.

I wonder if it might be better to explicitly support such context-constraints in the type definitions themselves. For example, being able to specify that a property should be visible when used in the context of a GET but not a POST, a property being required in a GET, but optional in a PATCH etc. That way you potentially avoid duplication, avoid trying to (ab)use inheritance etc, stub generators would generate a single class, documentation tools could use this information to generate the different "views" of the type depending on the context etc.

Kevin

KevinMitchell commented 7 years ago

PS, I'd been contemplating using annotations for this, but using "non-standard" annotations would then require me to implement custom code generators, custom documentation generators etc. Not impossible, but having something standard supported by the community would obviously be preferable.

KevinMitchell commented 7 years ago

As a strawman proposal, what would happen if we allowed boolean expressions in the places where only boolean literals are currently allowed in the specification. And we allowed the method verbs, plus req and rsp, to be used as boolean values. This would allow us to define things like

Person:
    properties:
      name:
        required: get | post | !req
        type: string

Which would mean that the name property was required if the type is used in the context of a get, a post, or when returned as part of a response. Given such an ability we could then also introduce an additional visible? facet on property declarations.

These changes would presumably allow people to express the fine-grained distinctions between the different "variants" of a datatype required in the different contexts. Online documentation tools could evaluate such expressions, replacing them with boolean constants or filtering as appropriate. And stub generators could use such expressions in mechanically generated validation code etc.

araguacaima commented 7 years ago

Both Christian and Kevin propose something like

We should potential think about creating separated types for....

and... I agree... There are some cases that separated types are the most suitable solution according to the use case that it's tried to solve...

I do not attempt to discuss "designing adequately or inadequately." Valuing the quality of a "RESTful design" falls within the "art" part of the design. Topic for other forums xD

What I am trying to expose here is a very, very frequent case, surely it happend to all of us whenever we design. But before summarizing the case, I want to know if we all agree on the following premise: Structurally speaking a business entity should be one and the same always, regardless of the use that it is wanted to give to it, ie: A book, structurally and conceptually is always a book, when it is created, consulted or modified. Despite reading Chapter 2, that does not make Chapter 1 do not exist, I'm just not looking at it at the moment. The index will always be there, regardless of whether all the chapters have yet to be completed. That is, the attributes that make a thing a book: title, author, index, chapters, pages, etc., are present in every book, even if its value is empty, or even if in at a certain moment we are acting over only one of them_.

Only if we agree on that, what next will make sense. If some people think that this premise is false, it probably will not support what follows

Facts:

Without a mechanism to segmentate which attributes of the entity can be used on origination, requesting or modification we have two choices:

1.- Letting the type (and consequently the API) structure to be unclear by suggesting it's possible to pass id attribute on entity origination

  entity: 
    type: object
    properties:
      id: # Is not possible to say whether this attribute could be used or not on POST input
        type: string
        required: true | false # If true, it is required for all cases, otherwise is always optional
      name: string
      others: ....

2.- Duplicate the definition of the entity, a portion for origination entry and another for request, modification or origintation output

  entityPostInput: 
    type: object
    properties:
                  #Is is excluded for POST input
      name: string
      others: ....
  entityPostOutput_Get_Patch_Put: 
    type: object
    properties:
      id: string 
      name: string
      others: ....

I wrote above that I agreed with Christian that "heritable" facet would mean something similar to "final" modifier in OO languages, but after thinking a little bit, now I agree with Kevin when he said:

final: definition that can't be changed, not definition that is not visible in the subclass...

In reality my proposal is to give a way to parent "force" some behavior of the children.

In the example,

types:
  entity:
    type: object
    properties:
      id?:
        type: string
        description: id.
        heritable: false #This is telling that every child that inherit this type will not have the "id" attribute, unless child declare it explicity.
      name?:
        type: string
        description: name.  

  entityPostInput:
    type: entity
    properties:
               # Due parent declared this attribute as "not heritable" this child does not have the "id" attribute
      <whatever>: string.

  entityPostOutput_Get_Patch_Put:
    type: entity
    properties:
      id: # Due parent declared this attribute as "not heritable", every child that wants to use it SHOULD declare it explicity

, the "effective" entityPostInput entity would be:

types:
  entityPostInput:
    type: object
    properties:
      name?: string
      <whatever>: string.

and the "effective" entityPostOutput_Get_Patch_Put entity would be:

types:
  entityPostOutput_Get_Patch_Put:
    type: entity
    properties:
      id?: string
      name?: string

We have defined the "entity" once... The next definitions (children) can "play" with options such parent has left available to them

Note that the "final" modifier at the "entity" level would prevent the "\<whatever>" attribute could even exist