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.

Sign up for GitHub

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

Jump to bottom

URL service decoupling #10360

Closed
naz opened this issue Jan 10, 2019 · 3 comments
Closed

URL service decoupling #10360

naz opened this issue Jan 10, 2019 · 3 comments
Labels
affects:api Affects the Ghost API affects:routing related to the url service & dynamic routing features later [triage] Things we intend to work but are not immediate priority server / core Issues relating to the server or core of Ghost

Comments

@naz
Copy link
Contributor

naz commented Jan 10, 2019 •
edited
Loading

Known Problems

  1. The URL service uses the model layer directly. The decision was made before we invented API versions. The decision was made because raw knex queries are so much faster than using Bookshelf. We couple the URL Service (frontend) with the model layer.

  2. The URL service listens on Ghost events and sometimes we have to re-fetch the data from the database because these events are API independent (!) e.g. v2 does only show authors with posts. But the model layer doesn't know that.

  3. Because of (2), we run into the problem reported here. We can't do any async operations if the Ghost events are triggered, because otherwise, the API will return 404 urls. We couple the API with the URL service.

  4. URL Utils are used throughout codebase which couples it to every module in the system. (solved in Extract URL Service utils into SDK #10773)

Future Architecture

Imagine you split Ghost into two repositories:

  • API (models...)
  • Frontend (Handlebars, Dynamic Routing ....all the frontend features....)

You can't because it's too coupled.

Known Challenges:

  • How to listen on API events? Webhooks?
  • Use the Content API directly
  • Can we continue using knex_raw? (described here) or should we look for an alternative approach?

Also to consider

Some tests were disabled due to delay introduced by additional db call in resources service and would be re-enabled after the new mechanism is in place. Related convo
@naz naz added server / core Issues relating to the server or core of Ghost affects:api Affects the Ghost API refactoring/cleanup affects:routing related to the url service & dynamic routing features labels Jan 10, 2019
@kirrg001 kirrg001 changed the title URL service decoupling and improvements URL service decoupling Jan 10, 2019
@kirrg001
Copy link
Contributor

Updated the issue description a little. More coming soon...

naz added a commit to naz/Ghost that referenced this issue Jan 10, 2019
@naz
🐛 Fixed url not being immediately available after post publish
e9144ae 
closes TryGhost#10360

- This change makes sure posts and pages urls are populated in
synchronous manner
- Further cleanup and restrucruting of this hacky solution is planned to
be done in TryGhost#10360
@naz naz mentioned this issue Jan 10, 2019
Merged
naz added a commit that referenced this issue Jan 11, 2019
@naz
🐛 Fixed url not being immediately available after post publish (#10361)
aa49f77 
🐛  Fixed URL not being immediately available after post publish

closes #10360

- This change makes sure posts and pages URLs are populated in
synchronous manner
- Further cleanup and restructuring of this hacky solution is planned to
be done in #10360
@kirrg001 kirrg001 reopened this Jan 11, 2019
naz referenced this issue Jan 14, 2019
@naz
Added ability to notify and update url service about changes in relat…
df1ba8a 
…ed resources (#10336)

refs #10124

- This PR introduced additional db calls in URL service due to the need for a model recalculation (we can't rely on the objects that come with events)
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Jan 23, 2019
@ErisDS ErisDS closed this as completed Jan 23, 2019
@kirrg001
Copy link
Contributor

@ErisDS 🤔 Could you pls leave a reason why you've closed this issue?

@ErisDS
Copy link
Member

ErisDS commented Jan 24, 2019

All issues tagged with routing have been tagged with later and closed until we prioritise working on the Routing feature again.

@naz naz reopened this May 16, 2019
@naz naz removed the later [triage] Things we intend to work but are not immediate priority label May 16, 2019
This was referenced Jun 14, 2019
Closed
Closed
@ErisDS ErisDS added the later [triage] Things we intend to work but are not immediate priority label Aug 21, 2019
@ErisDS ErisDS closed this as completed Aug 21, 2019
naz added a commit to naz/Ghost that referenced this issue Jun 3, 2022
@naz
Added a hacky matcher to overcome 404 URL problem
b40ca22 
refs https://github.com/TryGhost/Toolbox/issues/341
refs https://github.com/TryGhost/Toolbox/issues/320

- The snapshot for post's url property is causing test flakyness. The reason is due to the async nature of url processing in the URL Service (TryGhost#10360). Once the underlying issue is solved this hack could be removed.
- Having a snapshot tes even in this form is better than having none!
naz added a commit that referenced this issue Jun 3, 2022
@naz
Added a hacky matcher to overcome 404 URL problem
9c3ed1a 
refs https://github.com/TryGhost/Toolbox/issues/341
refs https://github.com/TryGhost/Toolbox/issues/320

- The snapshot for post's url property is causing test flakyness. The reason is due to the async nature of url processing in the URL Service (#10360). Once the underlying issue is solved this hack could be removed.
- Having a snapshot tes even in this form is better than having none!
@naz naz mentioned this issue Oct 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
affects:api Affects the Ghost API affects:routing related to the url service & dynamic routing features later [triage] Things we intend to work but are not immediate priority server / core Issues relating to the server or core of Ghost
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ErisDS @naz @kirrg001