-
Notifications
You must be signed in to change notification settings - Fork 10
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 support for "inheriting" fields from a parent type #113
Comments
I like the 1st approach. I wanted to share my opinion on some of the questions you raised:
Also, for the second approach you mentioned we won't be able to get the type definition but just the name of the fields. |
It does get the type definitions for the fields, but I see now that I didn't explain that part in my original post. I've added some more comments to my original post that help explain how it works. |
FWIW, I found approach 1 to be very hard for me to reason about, but approach 2 reminds me of prototype-based programming to me, which I can sort of see how it works. My questions are mostly around do we see this as only operating on |
That was all I was thinking, but I am open to suggestions.
Let's use this example again:
Does that explain it a little better? We're getting the field names that are defined in the
It might be possible to generalize this to some of the other constraints. E.g. with
However, I don't think it's particularly useful for those constraints. It's easy enough with e.g.
I think that a similar concept could be useful for
|
Thanks for the clarification, so it only creates a |
Value constraints would need to be part of the
Now, if Another way to think of this is that
That's correct, and the schema author can choose to handle it however they like using the existing logical constraints. E.g.:
|
Okay, I think this is fine then, the "prototyping" is explicit for one kind of constraint and only that kind. We are free to add different ones in the future. |
The confusing part is that the type is inherited, but what does that inheritance do? Right now it inherits the field's constraints, but not the other map constraints, which is strange to me. Also, it seems weird to close the type from extension. When I say it's closed, I'm thinking the map is closed. So if a subtype inherits the fields, and adds more, I'd expect it to be closed to the union of fields. So here:
I would think that e and f are allowed along with the other inherited types, but that the type is closed from having more fields. Our personal use-case, to put more context, is that we want all fields on our API input to be closed to catch typos as we insert things in our DB. All our types inherits from a few base types that has common fields like createdDate and all that. Currently this is impossible, if you close the base type, all the derived type that define additional fields error. If you close the derived type, it errors because the base type adds fields as well. What was surprising to us was that the fields constraints inherits and merge, but the content closed didn't inherit. The fields_of makes sense to me, but what doesn't is the current inheritance, because I see it similarly to a "fields_of". When we say foo is of type bar, it means it has the fields of bar no? Or what's the distinction? |
Ion Schema has no inheritance or abstract types—only composition—and the thing we're composing is rules, rather than members of a class. When we say that |
We have a usecase where we want to federate the schema creation. As a system owner, we define a base type and our multiple clients can define their custom type extending with additional fields and constraints. And the data for each clients should satisfy the constraints of their custom type as well as base type. When a new client creates a new custom type we shouldn't have to update our base type. From my perspective, I find the alternate approach more intuitive. |
Here's an example of a product listing for tires to make the federation use case more concrete.
In this case, you might have a base type (
It is probably cleaner to create a federated model using composition, such as this example, but in cases where the schema is being defined after a federated system is already running production workloads, you may not have the flexibility to model the data in an optimal way because of the cost of migrating existing data.
|
I understand that now, so it's similar to all_of, it was confusing because it uses inheritance-like syntax. We ended up implementing custom inheritance ourself. We simply copy rules from the parent into the child, and then the effective type becomes just the new type which has the rules of everything in the chain combined. This doesn't fail if the child wants to close fields, because it doesn't need to be independently valid as all_of does, it simply need to remain valid when merging all rules together. Maybe what I would propose is a
Where foobar here is the same as just copy/pasting their constraints together:
Now This isn't same as inheritance, because you don't need to handle merge conflicts, you could just duplicate. For example:
Would be same as:
This isn't a valid syntax I don't think, but it's for demonstration purpose of the behavior, so Maybe that would satisfy our use-case of combining fields and letting the content: closed behavior close over the union of fields, but without having to introduce real inheritance? |
@didibus A If you're just suggesting it for |
@popematt You're right I didn't think about all constraints yet, so not sure if there is a nice way to tackle them all. I'll put some thoughts into it. One thing maybe I missed from your proposal, can you enforce the closeness? One thing that was important for us, is that if I close in any one of them, it should be closed, but its the union of the fields that should be closed. |
In my proposal, the "closed-ness" across multiple type definitions is enforced by the
However, when using this approach, the
|
Originally posted by @didibus in #47 (comment)
Ion Schema does not have any OOP-style inheritance because constraints can only subtract from the set of values that are allowed for a given type definition. That being said, it is inconvenient to have to copy/paste fields from one type definition to another, and it is a maintainability issue to have to manually keep the fields of multiple type definitions in sync.
Any solution to this problem should consider whether the solution would make it easier or harder to generate code (e.g. POJOs or Ion 1.1 macros) for a type definition.
An Idea
Here is a strawman proposal of something that could work without needing to introduce any OOP-style inheritance into Ion Schema.
Some open questions with this approach:
foo
isclosed::
? We could say that "closed" is a modifier, not content, of thefields
constraint, but that might be a confusing behavior if you want to "inherit" the fields and their "closed-ness". On the other hand, if you want to "inherit" from a type with closed fields, you can just usetype: some_type_with_closed_fields
.foo
has more than onefields
constraint? We would probably want to grab all of them.foo
has nofields
constraint? Would this be an error or a no-op?all_of
, but how would we handle theoccurs
?union
withoutclosed
? Maybeunion
should imply that the fields are closed, and thenclosed
andunion
would be mutually exclusive.An alternate approach
Instead of using
fields
to create unions of field definitions, we could create a special syntax for thefield_names
constraint since you can close the fields usingfield_names
. E.g.:In this case, the
fields_of
annotation is a flag that indicates that the Ion Schema System should automatically generate an inline type based on the fields of the listed types. I.e.{ valid_values: [ a, b, c, d ] }
.This seems like an elegant solution to me, especially since it avoids the question of how to combine the
occurs
or type definitions of each field.However, this does seem like it might violate the tenet of having orthogonal constraints. Tenets are made to serve us, not us serve the tenets, but it is a good indicator that we need to be careful. There's also the unanswered question of how an anonymous/inline type could refer to its own
fields
constraint(s).The text was updated successfully, but these errors were encountered: