Introduce ingredients as new content structure

[tvdeyen]

What is this pull request for?

This introduces a new model of storing content in Alchemy.

Before we had a mixture of several rather complex objects to store content in Alchemy:

Element has many contents (Alchemy::Content) that belongs to essence (polymorphic Essence classes). That worked well for the last decade but has several issues in large scale applications:

1. You can't properly eager load or join polymorphic associations
2. You can't create or destroy a large amount of polymorphic associations
3. Confusing object tree

Now we make use of Rails' STI to build

Element has many ingredients (Alchemy::Ingredient child classes). That's it 🎉. Now we can

1. Properly eager load and join all ingredients including their associations
2. Can create and destroy large amounts of ingredients
3. Have an understandable object tree

In order to make that work we normalized the alchemy_ingredients table to:

1. A value column that holds the primary value of an ingredient. (A string of text for the Text ingredient or the date of the Datetime ingredient.)
2. A data JSONB (JSON in non-postgresql rdbs) column, that holds additional attributes like the link_class and link_title attributes of the Link ingredient.
3. A related_object belongs_to association for ingredients having related objects like Alchemy::Picture, Alchemy::Page for the Picture and Page ingredients.

We are pretty confident that this is all we need to support the same flexible content structure Alchemy provides you for for over a decade now, but allows us to scale Alchemy for large installations and truly serve as a content API for the JAMStack age.

Upgrading

We introduce this change in parallel without touching the current Content Essence structure at all, making it possible to run a hybrid approach. This is completely opt-in.

I order to upgrade one of your elements just change

 - name: article
-  contents:
-    - name: headline
-      type: EssenceText
-    - name: text
-      type: EssenceRichtext
-    - name: image
-      type: EssencePicture
+  ingredients:
+    - role: headline
+      type: Text
+    - role: text
+      type: Richtext
+    - role: image
+      type: Picture

and run this rake task

rake alchemy:upgrade:6.0:create_ingredients

And you are good to go. 🚀

This task is idempotent, so you can upgrade element by element. It will skip elements already having ingredients (but will destroy all contents and essences of a succesfull migrated element).

During rendering Alchemy will prefer ingredients over contents.

Checklist

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have added tests to cover this change

[codeclimate]

The PR diff size of 6043 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

The PR diff size of 6358 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

The PR diff size of 6681 lines exceeds the maximum allowed for the inline comments feature.

[mamhoff]

In my testing, I ran into some issues with actually displaying ingredients rather than contents.
Also, I had to remove a bunch of cache blocks in page and element partials in order to get a debugger into the ElementsViewHelper. I am guessing that we're still missing touching elements and pages when we update ingredients.

This is all pretty small stuff though, I think with these couple of issues fixed, this might be good to go! Hello performance improvement :)

[codeclimate]

The PR diff size of 6650 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

The PR diff size of 6654 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

The PR diff size of 6734 lines exceeds the maximum allowed for the inline comments feature.

[codeclimate]

The PR diff size of 6759 lines exceeds the maximum allowed for the inline comments feature.

[mamhoff]

Let's get this in. We need this in the main branch to be able to work on our dependent libraries.

[robinboening]

Wow, thats a big change! I like the approach of supporting both ways and allowing to slowly opt-in to the new structure. ❤️

I am glad we are getting rid of the polymorphic association. Simplifying the associations is a big win! I believe it is the right way. Thanks! 👏 🥳

I'd like to add my 2 cents though as replacing polymorphism with STI comes with trade offs. STI has its very own problems, especially later in the future when it comes to extending the data model. The introduced json(b) column helps with that on the first glance but it comes at a cost and shouldn't be underestimated. I've been bitten by json(b) structures many times by now, up to a point where I promised to not use them again in my life (and I did anyways). Very easily and quickly the data integrity can get compromised and aside from that querying those structures is a real pain (even queries that seem simple at first often turn into monsters).

I know you thought about the pros and cons of the change a lot and in depth and I also believe the trade offs are worth the simplifications and the performance gain.

I am looking forward to trying it out, but I will probably need to wait a bit longer as stuff is piling up here...

Thanks again!

[tvdeyen]

Thanks, Robin.

Yes, we are aware about these potential issues and weighted them out. Ingredients as well as essences always have been and will be "typed single value" records. The additional attributes are secondary options like link titles or css class names. Those will most likely never be queried on collections or used in search.

That said, we should think about adding an indexed column where ingredients like the Richtext can put data that will be queried frequently (ie. full text search).

Thanks for the input.

[robinboening]

One thing that came to my mind is the https://github.com/ruby-json-schema/json-schema gem, which allows to validate specific json schemas (as the name suggests 😆) on the application layer. In the case of STI it should be possible to validate a schema per inheriting model, I suppose. It might not be needed to validate every model and also not its entire schema, but maybe some things could be useful. It's just a suggestion to look into.

Since you mentioned full text search and indexing the data column, let me add one more thing to keep an eye on and be cautious with.

DISCLAIMER – the following is only postgres and JSONB related as I don't know how other DBMS handle JSON types.

Even with btree/gin indexes there could easily be cases were the query planner won't be able to be as smart as with normalized data. This is because postgres does not collect statistical information about the distribution of values for JSONB types. This can lead to very expensive queries even with proper indexes in place.

I find this article (11/2017) a very good read on that topic as it explains cases where the query planner makes bad estimates, thus bad choices, but it also shows a workaround using functional indexes and mentions the extendability of postgres statistics from v10. https://blog.anayrat.info/en/2017/11/26/postgresql-jsonb-and-statistics/

[tvdeyen]

Yeah, I thought about json schema integrity as well, but the same comment from above applies here as well. The additional attributes in the data JSON are secondary attributes, besides the Richtexts stripped_content (what is used by search indexes), and does not need that complexity IMO.

What do you think about adding a dedicated column for indexed content from the Richtext ingredient (See https://github.com/AlchemyCMS/alchemy_cms/blob/main/app/models/alchemy/ingredients/richtext.rb#L37 for backgorund). Maybe indexable_value?

Thoughts @mamhoff @rmparr ?