-
Notifications
You must be signed in to change notification settings - Fork 14
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
RFC-30 / Data Model #30
Comments
Had a catchup with Serge on this yesterday, and I agree with Overall, the focus of this should be on creating a more user friendly UI (GridFields add too much friction). And to choose a data model which can support that. |
Thanks for the updated use SilverStripe\LinkField;
class Page extends SiteTree
{
private static $db = [
'Link' => LinkField\ORM\DBField\Link::class,
];
} This would create the following database columns (roughly):
Without a "container" class like this, I don't have faith in devs explicitly opting in to all potential link types. They'll just assume that a banner would only need links to pages, and once CMS authors identify a need to link to a PDF file it'll become an expensive code change. |
Thanks for the feedback, Ingo! That's a valid concern. I have also considered the use case of choosing any link type, although I didn't add it to the initial PoC. I have just added a prototype of such implementation to the PoC. See the AnyLink dbfield. use SilverStripe\LinkField;
class Page extends SiteTree
{
private static $db = [
'Link' => LinkField\ORM\DBField\AnyLink::class,
];
} Produces the following data model: `LinkDBLink_Type` enum('SilverStripe\\LinkField\\ORM\\DBField\\AnyLink') COLLATE utf8mb4_unicode_ci DEFAULT 'SilverStripe\\LinkField\\ORM\\DBField\\AnyLink',
`LinkAnyLink_f2f0ebb1_LinkType` enum('SilverStripe\\LinkField\\ORM\\DBField\\Link','SilverStripe\\LinkField\\ORM\\DBField\\AnyLink','SilverStripe\\LinkField\\ORM\\DBField\\HyperLink','SilverStripe\\LinkField\\ORM\\DBField\\SiteTreeLink') COLLATE utf8mb4_unicode_ci DEFAULT 'SilverStripe\\LinkField\\ORM\\DBField\\Link',
`LinkHyperLink_dba830f3_Title` varchar(255) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`LinkHyperLink_dba830f3_URL` varchar(255) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
`LinkSiteTreeLink_09b5f94f_SiteTreeId` int(11) NOT NULL DEFAULT '0', |
Yep that looks better. Can you explain why we need a specific So in your example above, the |
Maybe we don't need it. That's just a prototype implementation to prove the concept. The final implementation may be better optimised. The current implementation with a separate field for a link type of AnyLink, however, ensures the data integrity on the DB level. It may prevent some logical errors in the application (e.g. you won't be able to assign a HyperLink instance to the Page.Link field and will have to always provide it with an instance of AnyLink, because it's of type I believe it is possible to have either implementation later and we don't have to "incorporate" that decision in the Data Model RFC. |
In my view, the purpose of a "data model" centric RFC is to work through the data model issues, right? And the verbose "just in case" type fields in the database are a big part of that. You are correct that on an ORM level, accessing This brings up a somewhat related templating issue: How do you expect that we handle potential additional HTML attributes required by this new object? The most natural template implementation is |
I feel that decision is not critical for this RFC, because both AnyType implementations may co-exist with the same data-model described in this RFC. Eventually, the users may decide which one to use or implement their own implementations addressing even more edgy cases in their projects. I agree that it is important to decide the
The AnyLink prototype in the PoC addresses that via casting AnyLink to HyperLink first and then rendering HyperLink in the place where AnyLink is put within the template. That means developers would have control over HyperLink template rendering via its own templates and class implementation. The section
That may depend on the particular link type implementation (e.g. devs can extend HyperLink with their own implementation and provide different templates or XML method implementation)
I would suggest the CMS authors should be able to configure these via FormField in CMS when they choose the type of |
I imagine the base link model could follow the FormField API, |
Had a glance at the RFC PR. I don't have major beef with it. Developer experienceMy main concern is making sure it's easy and straight forward for dev to define links on their data objects. I'm thinking we could create a So people could just define their links with: private static $db = [
'MyLink' => 'Link'
]; Or we could rename Basically, the average dev shouldn't have to invest a lot of time to understand the gut of the module to use it. Type concernsMy gut feeling would be to have to keep a FQCN just to avoid any potential collision later on. Other concernsAll the other concerns raised about templating and additional link attributes are secondary and can be handled later on. We're pretty far from shipping a stable product. If we make a bad decision we can come back and fix it later. |
We identified the following technical concerns regarding the PoC AnyLink implementation: It aggregates database fields from every link type in the application. If we have 10 link types registered in the application, every one of those would have 3 fields (in average), the AnyLink would create 30 database fields (even though only 3 of those would be used for every particular link type). Multiplying that by several links for a data object makes that very inefficient. We are going to work on an optimised AnyLink implementation in the following issue: #31 |
RFC-30 / Data Model
About the RFC
The RFC describes the data model of the module, the decisions made and captures the reasoning behind those.
The data model defines
Resources
Forum discussion (the module idea) - New links module?
Github discussion (this RFC) - issue-30
User Stories
Seeking some feedback from the community and internal teams we identified two main user stories that people would like to cover
Technical concerns
For a potential implementation the following technical concerns have been stated:
If a Link DataObject needs subclasses for each link type, the number of SQL queries may become
too high and the number of tables will rise really quick in projects with versioned and fluent
modules. The overhead load to the Database is the concern in that scenario.
Also, the number of database tables will slow down dev/build and history viewing.
Extra dependencies that the module may require. E.g. gorriecoe/silverstripe-linkfield requires
silvershop/silverstripe-hasonefield, giggsey/libphonenumber-for-php and unclecheese/display-logic,
which are not security audited
Persisting links in DB as data blobs (e.g. JSON or Serialized) is very opaque and hard to work
with later on. Querying owners containing particular links may be impossible to achieve.
Existing solutions
sheadawson/silverstripe-linkable
The most popular community module at the time of writing, with just over 220k downloads.
No longer maintained. Recommends switching to
gorriecoe/silverstripe-linkfield
.Implementation:
A link is a DataObject (class Link, table LinkableLink). DataObjects use
has_one
to implement links.Custom link types implemented via
SilverStripe\ORM\DataExtension
for the Link DataObject, whichadds new custom database fields and extends the
updateCMSFields
method.UI implementation:
Backend: LinkField class extending
SilverStripe\Forms\TextField
Frontend: Entwine extension bundled with the module
gorriecoe/silverstripe-linkfield
The second most popular community module at the time of writing, with just over 37k downloads.
Is maintained at the moment of writing.
Built on top of
gorriecoe/silverstripe-link
. Has a dozen of satellite modules extending the functionality in a decoupled way.Implementation:
A link is a DataObject (class Link, table Link, module gorriecoe/silverstripe-link). DataObjects use
has_one
ormany_many
.Custom link types implemented via
SilverStripe\ORM\DataExtension
for the Link DataObject, whichadd new custom database fields and extend the
updateCMSFields
method.UI implementation:
Backend: LinkField extends a
SilverStripe\Forms\FormField
(module gorriecoe/silverstripe-linkfield)Frontend: only CSS. Relies on gridfields and OOTB framework components.
Considered implementation options
Option A
Link is a DataObject.
Implemented by sheadawson and gorriecoe. Easiest to extend via DataExtension. Can be shaped to has_one, has_many, many_many. More complex relationship versioning and handling (although we have that solved through cascading publish etc.). More performance impact on query time. Can use lots of built-in editing UIs directly (e.g. GridField). Works with other core features like diff views, reports, etc. But also adds more database tables and schema complexity. Most consistent, SilverStripe devs know how to deal with this.
Pros:
Cons
Notes:
Option B
Link is a DBField with serialised data (e.g. JSON).
Implemented by bannerblock. Harder to extend. Serialised JSON makes it hard to query individual fields (without using something like GitHub - phptek/silverstripe-jsontext: JSON storage and querying 1 and JSON database support). Harder to enforce consistent schema. Creates new dependency if done properly (e.g. through phptek’s module). Less native form handling (e.g. validation errors?)
Cons:
Option C
Link is a CompositeField with individual database columns.
Harder to extend. Easy to query. Strong schema. Can be wrapped in arbitrary DataObject structures or relationships. Can “inline” columns into a DataObject to avoid relationship in cases where only one link is required. Can use lots of built-in editing UIs indirectly (e.g. GridField through DataObject). Less native form handling (e.g. validation errors?)
Cons:
Potential data structure (columns): see PoC below
Extending: see PoC below
Frontend:
Notes:
Comparing the options
Option C
is the most flexible way to store the data.Option B
could exist on top ofOption C
, if necessary, as a separate link type implementation.Option A
could exist on top ofOption C
, if necessary, as a separate data object containing theDBLink
filed (Option C
)For users there's no much difference between
Option A
andOption B/C
. The main difference is for devs implementing new link types.The conclusion
Option C
is preferableProof of Concept for the Option C (PoC)
Implemented in dev/rfc30/poc branch.
Base DbField implementation
ORM\DBField\Link is the base class extending
DBComposite
and providing the basic API for the children (particular link implementations).The class is abstract, which guarantees all owners know exact link types they contain.
The abstract
Link
overridesDBComposite::compositeDatabaseFields()
method returning a single own DB field -DBLink_Type
plus the fields provided by child implementations viagetCompositeDbFields
.For every link of the
DataObject
the fieldDBLink_Type
stores the particular link implementation (class name).The extension point for children is
abstract public static function getCompositeDbFields(): array;
. Every child (link type implementation) defines its own list of fields that would be added to the DataObject utilising the LinkField.DataObject Extension
Extensions\DataObjectExtension provides extra capability on top of
DataObject::populateDefaults
that ensures allDBLink_Type
fields are correctly initialized and contain correct values when creating empty DataObjects.Link Type implementations
The PoC implements 3 examples:
Front-end rendering options
XML
method for rendering the link on frontend (is a fallback if no templates exist)ORM\DBField\HyperLink
has a template templates/SilverStripe/LinkField/ORM/DBField/HyperLink.ssCMS rendering
The PoC is missing the CMS implementation for the fields, but it should be possible to provide the usual implementations for both Entwine and React front end methods.
Any particular FrontEnd implementation details are out of scope for this RFC (irrelevant for Data Model).
Validation
Backend Validation
- may be implemented by FormFields. Extra validation may be implemented for everyORM\DBField\Link
implementation (via overridingsetValue
method if necessary)Frontend Validation
- can be implemented via overriding FormField component implementationsDatabase Validation
- data integrity enforced viaDBLink_Type
field and the strong schema structure for every link type. That won't allow persisting incorrect link types, even if a logical error happens in the application.Data Structure
The provided PoC was tested with the following
Page extends SiteTree
DataObject model:That
Page
model produced the following database structure:Both fields
HyperLink
andSiteLink
only add their own fields to the finalDataObject
structure.The only overhead in the generated schema is an extra
*DBLink_Type
field for everyLink
.This means we
only pay for what we use
.AnyLink
The suggested data model design (the Option C) is flexible enough to achieve AnyLink as a link type implementation on top of DBField\Link. See an example prototype implementation in the PoC - AnyLink.
The following Page implementation:
Produces the following data model:
Thus,
AnyLink
is just another link type implementation that provides a composition of fields of all the other link type implementation and then can behave as a particular link type chosen by end users.The text was updated successfully, but these errors were encountered: