-
Notifications
You must be signed in to change notification settings - Fork 1
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
[FQTM-11] New entity type join model #82
base: master
Are you sure you want to change the base?
Conversation
Quality Gate passedIssues Measures |
entityTypeSource: | ||
$ref: schemas/entityTypeSource.json | ||
entityTypeSourceDatabase: | ||
$ref: schemas/entityTypeSourceDatabase.json | ||
entityTypeSourceEntityType: | ||
$ref: schemas/entityTypeSourceEntityType.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discriminator
in openapi-generator
behaves a bit oddly across multiple files (as does the whole concept of polymorphism in JSON schemas), as it has to know of the sub-types ahead of time, and gets really confused if you try to use file names directly in the mapping
s. If it can't find them, it'll make guesses at names based on property values, or any number of weird undesirable outcomes.
After a lot of fiddling, I have found the secret incantation we must do to keep it happy:
- Define ALL subtypes as schemas in our central yaml file
- Refer to these subtypes in the supertype's mapping as though we are in the same file with the yaml (???, genuinely spooky)
- Theoretically, these can also go in a
oneOf
on the supertype, but I was never able to get that to work
- Theoretically, these can also go in a
- To have the supertype with its own properties:
- Define an
allOf
in the supertype, with one object for the discriminator information, and the other for the properties. These cannot mix, for some reason. - Create each subtype as an
allOf
pointing back to the supertype
- Define an
"$ref": "./field-joins/join.json" | ||
} | ||
}, | ||
"originalEntityTypeId": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used by the flattening service to keep track of which column came from where, to ensure compatibility
"description": "Alias of this source to be used in query/later joins", | ||
"type": "string" | ||
}, | ||
"joinedViaEntityType": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used by the flattening service to determine if an entity type was inherited vs top-level. We could probably do something more clever (check alias for dots, compare original list, etc), but this makes the logic a little more straightforward
"discriminator": { | ||
"propertyName": "type", | ||
"mapping": { | ||
"db": "#/components/schemas/entityTypeSourceDatabase", | ||
"entity-type": "#/components/schemas/entityTypeSourceEntityType" | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goodbye if (source.getType().equals("db"))
, hello instanceof
and being able to pass them around with guarantees of which type they are
"sourceField": { | ||
"description": "If joining, fully qualified field of the source entity type that targetId/targetField will join to", | ||
"type": "string" | ||
}, | ||
"targetId": { | ||
"description": "ID of the entity type newly included with this source", | ||
"type": "string", | ||
"format": "uuid" | ||
}, | ||
"targetField": { | ||
"description": "If joining, field of the new entity type that will be joined to the source entity type", | ||
"type": "string" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here is something like:
sources: [
{ type: 'entity-type', alias: 'users', targetId: 'users-entity-type-id' },
{
type: 'entity-type',
alias: 'groups',
// sourceField is intelligently searched for in the other source(s), and join configuration is stored on the field itself.
// This prevents composite entity types from having to care at all about the DB tables/names below;
// all that matters is fields, which I feel _much_ more comfortable considering part of a simple's API
sourceField: 'users.patron_group_id',
targetId: 'groups-entity-type-id',
targetField: 'id' // relative to the target entity type
}
As an extra note, the join be defined on either side of the relationship. users.patron_group_id
can point to groups.id
or vice versa, and they can be joined in either order here — everything is automatically handled, including the direction of the join
"type": "object", | ||
"properties": { | ||
"sql": { | ||
"description": "Raw SQL to join the columns (use :this and :that to refer to this and target columns, respectively)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, of course, a trace of the true self must exist in the false self
This PR creates a new join model which reinforces the idea of simple entity types each representing 1 DB source being joined together in arbitrary ways by higher-order entity types. This is accomplished by using a new set of pre-defined join types as part of column definitions themselves, enabling entity types to know how to join with each other, rather than requiring higher-order entities to explicitly specify raw SQL every time.
What does that actually mean though??
Simple entities
Column definitions will now describe what other columns they can join to. For example, in the user's entity type:
There is a set of predefined
type
s (this one compares to UUIDs after casting, i.e.:this::uuid = :that::uuid
), in addition to one which allows custom SQL, similar to our previous join conditions.These
joinsTo
definitions can go either side of the relationship (although IMO it should go on the child side, e.g.user.group_id -> group.id
rather thangroup.id -> user.group_id
). Things like the direction will automatically be handled as expected.Entity sources
Nesting all the way down 😎. Composites can point to composites can point to composites can point to composites can..., even with reused alias names, so we don't have to worry about any conflicts as our entity type trees grow.
Now, rather than defining the join conditions for
entity-type
sources, we just specify the fields we want to link: