Routes, resources & urls -> Centralised Services #9192

ErisDS · 2017-10-31T11:51:55Z

This is going to be a catch-all issue for a whole bunch of refactoring I'm working on.

Below are 3 specific problems I want to solve, and 3 specific use cases I hope will be achievable at the end.

Note: The goal is to move towards these things being possible internally & in internal apps to see what sort of structures/APIs we end up with. Once the refactoring work is done, we'll address exposing functionality in other ways.

Problems:

Resource <-> URL 2 way generation E.g. what is the url for a post? which post does a url represent?
Easy way to generate sitemaps, or other lists of urls, with meta data
Having routing be enumerable, and then configurable, extensible, etc

3 main use cases we currently cannot support

custom home pages
rendering a resource at a non-standard URL, e.g. /tag/recipes/ -> /recipes/
custom routes and their meta data, e.g. whether they should be in the sitemap, E.g. /subscribe/, /featured/, /recipes/, /private/, /*/amp/.

The expected outcome of this refactoring work is that we will have new services which control the ability to register routes, describe what happens at that route, and provide meta data about the route. Also that can then manage individual URLs, when they are added or removed from the system, and track some meta data about them.

Rough task list:

Then:

Rewrite sitemap code
Revisit controllers, how can we generalise them?
Revisit full configuration and API, what else can be improved?
Tags & Authors get urls in JSON API responses

Notes:

ensure we also refactor post.published events to never trigger on import as part of this.
get rid of the weird secure/setRequestIsSecure everywhere (was Refactor & improve reliability of "secure" handling for urls #6593)
figure out how route keywords fit into the route system (was Make route keywords configureable #8492)
improve what is included in sitemaps (revisit ideas in [Discussion] Sitemaps - what should be included? #4588)

The text was updated successfully, but these errors were encountered:

refs TryGhost#9192 - Each setting is saved individually - Update this to only happen on import, or when a value changes - Reduces the amount of work Ghost does on every setting change

refs #9192 - Each setting is saved individually - Update this to only happen on import, or when a value changes - Reduces the amount of work Ghost does on every setting change

refs #9192 - Instead of `setupRoutes` function in apps that gets passed a router, there is now a registerRouter function as part of the proxy - Moved towards a route service, which will know about all routes - Using classes to abstract away shared behaviour Notes: - changing the app proxy didn't result in a test failure! - structure of route service is totally new and may change a lot yet

refs #9192 To anyone seeing this go by - I'm about to start some fairly major refactoring work on the url utility. Before I do that, I wanted to make sure I had 100% coverage, and understanding of some of the weird cases. The majority of the changes I've made are adding tests, but I was also able to clean up a little bit, remove a few lines or change them to make use of other tools.

refs #9192, refs #5091 - Using a class allows for easy shared logic - Loading is designed to work from config right now, but could be DB driven, etc - Provided configuration can be simplified and extended in the constructor / class methods - Update tests, move custom assertions to utils

refs #5091, refs #9192 - render channel was always a weird file - now it's clearly 2 things - we're slowly getting towards closing #5091... 🎉 - added some extra tests

refs #5091, refs #9192 - This is similar to #9218, in that I'm revealing bits of code that are "controllers" in our codebase. As opposed to routes, services, renderers etc. - This also reveals some code which is identical to the channels controller - There is more to do here, but for now I've got the module split up, and the tests split and improved. - Next I'll split RSS into controller + service, DRY up the controller code, etc

refs #5091, #9192, #9178 - Get the RSS module into a much better shape - Controller -> /controllers/rss - Remainder -> /services/rss - Moved tests to match & updated requires

refs #9192 - The AMP app is nothing more than a custom controller - this will come clear soon - Moved enabled/disabled logic into router - Removed error-related code, as this wasn't used - Changed logic for static pages to be based on req.body, not context - Improved the tests to match

refs #5091, refs #9192 - There are several theme template "renderers" all over the codebase - Some are in apps, and were called "controllers" - One is in error handling - All of them now have comments marking out how they share logic/steps - Other comments describe routes & controllers where they live

refs #9192 - Admin redirects should really happen first, up with custom redirects - Later we can package this up, maybe - For now, let's focus the site router on site-related things

refs #9192 - an entry is a post or a page, represented by a post model

refs TryGhost#5091, refs TryGhost#9192 - There are several theme template "renderers" all over the codebase - Some are in apps, and were called "controllers" - One is in error handling - All of them now have comments marking out how they share logic/steps!

refs #9192, refs #5091, refs #9178 - moved channels from controllers to a service - split out the parent router from the remaining individual router logic - moved the tests to match

refs #9192 - Moving towards a centralised concept of routing / routes - The base router now wraps express router, and offers us the features we need - Site Router is the parent router, it gets initialised with all of our default routing - App Router is a sub router for apps - apps register their routes/routers onto it. - TODO: refactor channels subrouter to work this same way - MAYBE: move the app router to the apps service

refs #9192, #5091 - changed channels to use our new base class - keep the flexible structure, so that channels can be reloaded - I had to move the router into the route service otherwise we get circular dependencies - Don't _really_ want to keep it like this - need a way to define base classes as shared

refs #9192, refs #9178 After trying to progress with current implementation, it became clear that the route service can't control the boot sequence, because then we end up with circular dependencies between the route service and the channel service. The route service now exposes: - a siteRouter - a way for apps to register routes. - ParentRouter base class for other modules to use - the registry ... - moved the default route setup back to site/routes.js 🙈 - moved the parent channel router back to the channel service (this makes way more sense imo) - this structure prevents circular dependencies - split the registry out into it's own thing - fixed-up various bits of tests and comments - DEBUG will print a list of routes 🎉

refs #9192, refs #5091 - Moved all url generation into generate-feed.js, so we can see as much data processing as possible in a single place. - Refactored the way res.locals were used, to be more like how express uses them prior to rendering - Removed a bunch of code & tests todo with context for RSS - I can't see any way that'd be used, unless we switched the rendering to use a template. - moved the RSS rendering to be part of the service, not controller - updated the tests significantly Note: RSS generate-feed has a complete duplication of the code used in the excerpt helper in order to create an item description

refs TryGhost#5091, TryGhost#9192 - Renderer figures out templates, contexts, and does a render call - Renderer will handle everything after data is fetched and semi-processed - For now it only does the last 2 steps - TODO: it will eventually do more steps!!

refs #5091, #9192 - Renderer figures out templates, contexts, and does a render call - Templating is now handled with a single function - Context call is made in the renderer Note: to make this work, all controllers now define a little bit of config, currently stored in res._route. (That's a totally temporary location, as is res._template... when a sensible naming convention reveals itself I'll get rid of the weird _). This exposes a type and for custom routes a template name & default.

refs #9192 - Introduces a url service that can be initialised - Added a concept of Resources and resource config.json that contains details about the resources in the system that we may want to make customisable - Note that individual resources know how to create their own Urls... this is important for later - Url Service loads all of the resources, and stores their URLs - The UrlService binds to all events, so that when a resource changes its url and related data can be updated if needed - There is a temporary config guard so that this can be turned off easily

ErisDS · 2018-04-24T10:00:15Z

Closing in favour of #9601

ErisDS added refactoring/cleanup server / core Issues relating to the server or core of Ghost labels Oct 31, 2017

ErisDS self-assigned this Oct 31, 2017

This was referenced Oct 31, 2017

[Discussion] Sitemaps - what should be included? #4588

Closed

Make route keywords configureable #8492

Closed

Refactor & improve reliability of "secure" handling for urls #6593

Closed

ErisDS mentioned this issue Oct 31, 2017

Refactored to save settings only if value changes #9194

Merged

ErisDS mentioned this issue Nov 2, 2017

Increased URL utility coverage to 100% #9201

Merged

This was referenced Nov 3, 2017

Wrapped express router & expose from route service #9206

Merged

Represent channels as class instances #9209

Merged

ErisDS mentioned this issue Nov 6, 2017

Split renderChannel into controller + renderer #9218

Merged

ErisDS added a commit that referenced this issue Nov 6, 2017

Split renderChannel into controller + renderer (#9218)

5dac1c9

refs #5091, refs #9192 - render channel was always a weird file - now it's clearly 2 things - we're slowly getting towards closing #5091... 🎉 - added some extra tests

ErisDS mentioned this issue Nov 7, 2017

Split RSS module into controller + other #9224

Merged

ErisDS added a commit that referenced this issue Nov 8, 2017

Moved RSS module -> services & controllers

86c6cec

refs #5091, #9192, #9178 - Get the RSS module into a much better shape - Controller -> /controllers/rss - Remainder -> /services/rss - Moved tests to match & updated requires

ErisDS added a commit that referenced this issue Nov 8, 2017

Move admin redirects out of site router

187c389

refs #9192 - Admin redirects should really happen first, up with custom redirects - Later we can package this up, maybe - For now, let's focus the site router on site-related things

ErisDS added a commit that referenced this issue Nov 8, 2017

Renamed single / post -> entry

f0f0735

refs #9192 - an entry is a post or a page, represented by a post model

ErisDS added a commit that referenced this issue Nov 8, 2017

Moved Channels module -> services

90cfdbe

refs #9192, refs #5091, refs #9178 - moved channels from controllers to a service - split out the parent router from the remaining individual router logic - moved the tests to match

ErisDS mentioned this issue Nov 9, 2017

Rework route service responsibilities #9229

Merged

ErisDS mentioned this issue Nov 10, 2017

RSS service + controller improved for consistency #9233

Merged

ErisDS mentioned this issue Nov 10, 2017

Introduced renderer to DRY up controllers #9235

Merged

ErisDS mentioned this issue Nov 15, 2017

Added Url Service to track all URLs in the system #9247

Merged

ErisDS mentioned this issue Nov 27, 2017

Primary Tags are not available in the Sitemap #9272

Closed

ErisDS mentioned this issue Apr 24, 2018

Dynamic Routing #9601

Closed

15 tasks

ErisDS closed this as completed Apr 24, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Routes, resources & urls -> Centralised Services #9192

Routes, resources & urls -> Centralised Services #9192

ErisDS commented Oct 31, 2017 •

edited

Loading

ErisDS commented Apr 24, 2018

Routes, resources & urls -> Centralised Services #9192

Routes, resources & urls -> Centralised Services #9192

Comments

ErisDS commented Oct 31, 2017 • edited Loading

ErisDS commented Apr 24, 2018

ErisDS commented Oct 31, 2017 •

edited

Loading