Skip to content
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

Database layer for the CMS objects #3908

Open
wants to merge 28 commits into
base: develop
from

Conversation

@LukeTowers
Copy link
Member

commented Nov 5, 2018

Moved from rainlab/pages-plugin#307. Related: octobercms/library#356, rainlab/pages-plugin#368

NOTE: Enhancement is currently a work-in-progress. Comments & suggestions welcome.

Problems that will be solved:

  • Multi-server deploys of October have issues with syncing CMS content (pages, static pages, content, etc) between the multiple instances of the application servers. This would be solved by allowing that content to be managed in the database, enabling the use of a central DB server to act as the source of truth. Managing developer updates to the theme files can be handled through the use of the theme:sync command.
  • Projects that are actively touched by both developers and end users have issues with content getting out sync where the production server has content entered by the client and there are template / theming changes made by the developer. This would enable the client's content to remain untouched in the DB layer while the developer still has the ability to selectively refresh specified files after they've been modified by the client.
  • Content is currently tied directly to individual theme directories, this makes tasks like sharing content between themes and multi-tenant applications that rely on CMS content very difficult to achieve by default. Moving CMS content files into the DB will simplify these tasks for plugin developers, although this is still not a full implementation / resolution for either of those cases.
  • As content is currently represented as flat files and has no presence in the DB, this makes certain extensions quite difficult to achieve (such as adding fileupload, relation, recordfinder, etc fields to CMS objects). This is a step in the right direction for supporting those features however is not a full solution to those issues.
  • There are also some issues with search performance on larger sites that heavily utilize CMS content, this will be alleviated somewhat by having the ability to store and search that content in the DB.

Outline of how the feature will work:

  1. Progressive: This enhancement will be fully progressive, i.e. existing installations and new installations will continue to use the filesystem datasource by default; only those instances that choose to enable cms.enableDatabaseLayer and have a database already will see this feature set.
  2. Templative?: This enhancement will change the role of the filesystem to that of a base template to fall back on when content is not present in the DB layer. All creates, updates, and deletes will be performed on the DB layer when this is enabled, the filesystem will remain untouched (excluding the use of theme:sync with the --target=filesystem flag).
  3. Performant: Performance is a top priority. As such, CMS objects that are available in the DB will be cached as such, and objects that have been explicitly removed from the site will be cached as such in the cache layer and the DB layer.
  4. Extensible: Feature will be extensible with the initial focus on events being supporting multi-tenant usage.

Task list:

  • Implement DbDatasource as a Halcyon Datasource in the October\Rain library
  • Add cms.enableDatabaseLayer configuration option. Defaults to false. Other values: true => enabled; null => inverse of app.debug
  • Implement AutoDatasource as a Halcyon Datasource in the CMS module (will handle choosing which datasource needs to be interacted with based on the situation)
  • Add theme:sync themedir --paths=path/to/specific/file/to/sync.md,path/to/other/file.md --target=filesystem, defaulting to the current theme, all paths in themedir for the --paths flag and database for the target flag. Replaces all paths specified in the target with contents from the source. If no paths specified will empty the target entirely before populating with source files.
  • Add Reset button to CMS & RainLab.StaticPages. Appears when DB & FS copy exists, removes the DB version therefore populating content from the FS.
  • Add Commit button to CMS & RainLab.StaticPages. Appears when app.debug enabled (meant only for developers) & DB content exists, populates the FS version and then removes the DB version from the DB for performance.
  • Ensure feature works with the RainLab.Pages plugin (will need to replace hard coded filesystem usage with CmsObject usage instead)
  • Test all functionality thoroughly
  • Add unit tests for all implemented functionality

DB layer can be described as the client, filesystem as dev. If the client has said "I don't want home.htm" then that will be true forever. (such records will be marked as deleted in the DB layer and then cached as a removed item in the AutoDatasource). Dev can delete and replace as much as they want, it won't change that fact that the client doesn't want that object.

If the client later inserts that file again into the DB layer through the CMS then the existing flagged record will simply be unflagged and populated with new content.

Current issues:

  • No unit tests yet

Test this feature

For those who would like to test this, simply follow these steps:

  1. Clone the octobercms/october repo.

  2. Switch the main branch to wip/halcyon-db-datasource (ex: git checkout wip/halcyon-db-datasource).

  3. Set cms.enableDatabaseLayer to true in /config/cms.php.

  4. Run composer update to install dependencies.

  5. Run php artisan october:env to generate an .env file (from project root).

  6. Input your local DB connection info in the .env file.

  7. In /plugins/rainlab/pages (create folder if it doesn't exist), clone the repo for the rainlab/pages plugin.

  8. Switch the plugin's branch to wip/halcyon-db-datasource (ex: git checkout wip/halcyon-db-datasource).

  9. In /vendor/october/rain, remove the existing contents and clone the repo for the october/library package.

  10. Switch the package's branch to wip/halcyon-db-datasource (ex: git checkout wip/halcyon-db-datasource).

  11. Run php artisan october:up (from project root) to install the local RainLab.Pages plugin from the cloned repo.

@osmanzeki

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2018

@LukeTowers Having this in the framework layer seems like a wonderful alternative to patching the pages plugin. 👌

I'm available to test the progression of this PR if you need me.

For reference, this plugin approached the same idea I believe (adding tables and intercepting saves towards DB instead of filesystem with the ability of pointing back to FS).

https://octobercms.com/plugin/axmit-storage

Since you're at the CMS/Framework level you can definitely make this a lot more "invisible", which is very nice.

LukeTowers added some commits Nov 5, 2018

AutoDatasource bug fixes and performance improvements
Properly remove deleted paths from results returned by AutoDatasource->select() and utilize the cache exclusively when only selecting the fileName column preventing calls to the source datasources.
@LukeTowers

This comment was marked as outdated.

Copy link
Member Author

commented Nov 6, 2018

@osmanzeki it should be ready enough to test with just CMS content. RainLab.Pages isn't playing nicely with it right now (for some reason it's only listing the first available page and ignores deleted layouts, not to mention some data is stored with direct filesystem writes instead of through the datasource abstraction).

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Nov 8, 2018

@seanthepottingshed @ayumihamsaki @w20k @Teranode Would you guys be willing to help test this out as it's worked on?

@w20k

This comment has been minimized.

Copy link
Member

commented Nov 9, 2018

@LukeTowers, count me in!

@seanthepottingshed

This comment has been minimized.

Copy link

commented Nov 9, 2018

@LukeTowers I would absolutely love to!

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2018

@w20k @seanthepottingshed feel free to get started. You'll need octobercms/library#356 as well, and I don't think RainLab.Pages works right now but everything else in the CMS section should work fine (excluding the items that I haven't checked off the todo list yet)

@seanthepottingshed

This comment has been minimized.

Copy link

commented Nov 12, 2018

@LukeTowers

Excellent, I will have a play with it this week.

@w20k

This comment has been minimized.

Copy link
Member

commented Nov 12, 2018

@LukeTowers

Thanks, I'll tag along and start testing on this week. Busy month all of a sudden.

@LukeTowers

This comment was marked as outdated.

Copy link
Member Author

commented Nov 13, 2018

Update: found one of the issues with the StaticPage plugin, the page hierarchy is stored in the meta/static-pages.yaml file. This causes two issues:

  1. The file is accessed directly through the filesystem, not through a datasource abstraction, meaning that the database layer can't interject it's own contents
  2. The file is populated directly from the PageList class, triggered after create and delete actions on the individual page records.

Potential solutions:
Implement a CmsObject that stores and retrieves data in the YAML format and switch the RainLab.Pages plugin to use that CmsObject type instead of accessing the file directly.

@seanthepottingshed

This comment has been minimized.

Copy link

commented Nov 16, 2018

@LukeTowers

I haven't forgotten about this - big site launch this week which I've been helping my colleague with, will have a play thereafter.

@Teranode

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2018

I haven't forgotten this either, i've just been preoccupied with my own project since i just started getting into Vue. Just ping me if you need me to do something.

@w20k

This comment was marked as resolved.

Copy link
Member

commented Dec 14, 2018

Hi, @LukeTowers,
Did a test run, things I've found:

UI Issue:

  • When you make a commit or reset. Buttons (reset/commit) will be hidden from UI nav-bar. Reset button should be hidden only if you don't have any changes in DB.
    SideNote: Might be a good idea to make two buttons: 'save' & 'save and commit'.
    Gif:
    dec-14-2018 12-47-26

Static Page Issue:

  • Doesn't work with newly added items.

Steps to reproduce:

  • Go to Pages tab -> Press 'Add New' static page or Menu.
    Screenshot:
    screenshot 2018-12-14 at 12 32 20
    Debug:
    screenshot 2018-12-14 at 12 33 21

Bugs:

  • When you do a reset for a file and then would like to sync with new changes. UI shows that's everything is fine, but no rows are added to the DB. Theiy're in the theme_logs, but not in the theme_contents
    Screenshots:
    screenshot 2018-12-14 at 12 46 32
    screenshot 2018-12-14 at 12 46 44
@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@bennothommo @mjauvin could you guys test this out?

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@w20k

When you make a commit or reset. Buttons (reset/commit) will be hidden from UI nav-bar. Reset button should be hidden only if you don't have any changes in DB.
SideNote: Might be a good idea to make two buttons: 'save' & 'save and commit'.

This is intentional behaviour, if the file is committed or reset, then the version of the file in both the database and the filesystem is now identical. The difference between the two buttons is which version of the file is used to make the sync, reset means the filesystem version is used, commit means the database version is used. As the file is now identical in both the DB and the filesystem, it cannot be synced again, as that would be pointless. Hence, both the commit and reset buttons disappear.

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@LukeTowers I've given this a quick test through with just the CMS pages area for now. Works really well, but I could not see the "Commit" and "Reset" buttons in the CMS pages area until I made the changes above that I've suggested in the reviews.

Fixed typo
Credit to @bennothommo

Co-Authored-By: LukeTowers <github@luketowers.ca>
@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

@bennothommo awesome! Merged one of your suggestions, clarified the other. Could you test it with the StaticPages plugin and let me know if you can replicate / fix @w20k's reported issue here: #3908 (comment)

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

Static Pages is working well as well. Just suggested a fix for the issue spotted by @w20k above.

Fix for models that don't yet exist
Credit to @bennothommo. Fixes #3908 (comment)

Co-Authored-By: LukeTowers <github@luketowers.ca>
@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

Awesome! @w20k could you run another test on this? @bennothommo Do you have any input on the theme:sync command? I think where I left off I was struggling to figure out the best way to determine what datasource paths should be mirrored when no explicit paths are provided given that the available datasource paths returned by a datasource (specifically the filesystem) could include binary files that don't necessarily have a Halycon object to handle them, in which case we would want to ignore them.

However, I'm stuck on what would be the best way of determining that and filtering out the files that don't have Halcyon classes to reference them. I don't really want the command to be syncing binary asset files into the DB after all 😉

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

@LukeTowers Would it be a case where you can make some presumptions about the scope of paths that should be checked and synchronised? Given the conventions already established with the CMS and Static Page objects in themes, you would realistically only need to sync the following folders (and any sub-folders and files) in a theme:

  • content/static-pages
  • content/placeholders
  • layouts
  • meta (for static menus)
  • pages
  • partials

With each of these folders, you would just need to limit the files scanned and synchronised to files with the htm(l), yaml, txt or json extensions (and possibly check the MIME type of each file to make sure it appears to be what its extension claims it to be). Anything else could likely be safely ignored, for example, any assets or files that are explicitly text-based.

Is there any examples of content that exist outside of these folders that you would also want to be synchronised?

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 15, 2019

@bennothommo I've been thinking about having an event that allows you to include paths / extensions to be listened to. Core shouldn't know about static-pages, and default extensions could be htm, yaml, txt, md, and potentially others if provided by an external plugin.

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

@LukeTowers Ah yeah, that could work, maybe as a list of globs to match. For example:

public function registerDatabaseLayerPaths()
{
    // Relative to theme directory
    return [
        'layouts/**/*.{htm,html}',
        'pages/**/*.{htm,html}',
        'partials/**/*.{htm,html}',
        'content/**/*.{htm,html,txt,md}'
    ];
}
@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 16, 2019

@bennothommo good idea, and then the event can process that as default input allowing it to be added to or removed from. Do you think you could take a stab at implementing that on this branch?

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

@LukeTowers sure thing, will try have a crack at it in the next day or 2.

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2019

@LukeTowers what was your main concern(s) in regards to the code you already have implemented that retrieves and filters the source paths? Looking over it, I think it might actually make more sense continuing with your implementation, as the CmsObjects already define the directories and valid extensions, and the filter you have in place is already stripping out any other paths (ie. assets).

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

Hmm, not sure exactly. Perhaps you could take a stab at finishing it off and then we'll see where that gets us?

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

@LukeTowers My first pass at theme:sync is here: #4276. I've tested it both directions, and with/without paths specified. There's likely some cleanup needed, but it works :)

@LukeTowers

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

@bennothommo looks pretty good! I've added a comment to it, then it can be merged and we can test it out!

@bennothommo

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

@LukeTowers all sorted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.