Problem/Motivation

Some sites want to have relative URLs in the responses and not absolute. Make this configurable.

Proposed resolution

  • Add a config object to hold a setting to configure this.
  • Use this to determine whether to return absolute URLs
  • Add update to create config object for existing sites
  • Default value should maintain current behaviour
  • Add test coverage

Remaining tasks

User interface changes

I propose we do not add a form for configuring this but add docs to the readme to config with Drush.

API changes

New parameters for \Drupal\decoupled_router\EventSubscriber\RouterPathTranslatorSubscriber::resolvedPathIsHomePath() to ensure cacheability info is correct.

Data model changes

New decoupled_router.settings configuration.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
krystalcode’s picture

This may be a matter of preference or semantics. The same way that within Drupal we don't define internal redirects using the full URL including the scheme/hostname etc., we just use the path, similarly on the client side - for example on Next server - you define a redirect like NextResponse.redirect(new URL('/my-content-url', request.url)). It does not make much sense to say NextResponse.redirect(new URL('https://www.example.com/my-content-url', request.url)) if you are already on that domain. That said, passing the full URL would work as well. I don't know Next well enough to say whether internally there's any functional or performance-related difference.

Also, the backend resolves the path based on its own URL. That is, the resolved URL would be https://api.example.com/my-content-url, while what we need is https://www.example.com/my-content-url. This means that the frontend would have to extract the path, and that's unnecessary.

Semantically - together with the isRedirect property, this basically says that "you're staying on the same site but you're redirected to another page". Otherwise, if the frontend just wanted the final internal path it would either have to extract it from the resolved property (not that it's a big deal), or from the last item of the redirect array which is unnecessary code for the frontend to handle. The former option i.e. resolved property actually - and if I remember correctly - would not properly carry over query parameters and fragments, while - together with some other fixes on this module - they would be carried over in the redirect array.

Together with some other changes to carry over query parameters and fragments in the resolved that I have not added here on this module, all that the frontend has to do is to grab that path. For the bigger context that may be useful when reviewing other issues that I opened in this module, here is what we were trying to achieve.

This came up as part of a bigger effort to make things as simple as possible for the frontend. We found that a decoupled frontend would have to write quite a bit of code to handle different cases e.g. path given as a URL alias, path given as the internal path, path or URL alias that gets redirected, redirects to internal vs to external URLs, issues with carrying over query parameters from the given path and from any applicable redirects, some special handling for the home page that can be found as / or as internal path or as URL alias of the node etc.

To have logic for handling all these different cases in the frontend ended up being time consuming and error prone i.e. all sorts of bugs were found, fixing something only to break something else etc.

At the end, here is the final structure that we ended up with. We found that this structure minimizes the logic that has to be done in the frontend on all cases. Example response:

{
  "resolved": "/my-content-url-alias?query=parameter",
  "isRedirect": true,
  "isExternal": false,
  "isHomePath": false,
  "entity": {
    "canonical": "http://www.example.com/my-content-url",
    "type": "node",
    "bundle": "page",
    "id": "1234",
    "uuid": "73c03483-XXXY-YYYY-d9acbdf4d5c0",
    "path": "/my-content-url"
  },
  "label": "My content item",
  "jsonapi": {
    "individual": "http://www.example.com/jsonapi/node/page/73c03483-XXXY-YYYY-d9acbdf4d5c0",
    "resourceName": "node--page",
    "pathPrefix": "jsonapi",
    "basePath": "/jsonapi",
    "entryPoint": "http://www.example.com/jsonapi"
  },
  "meta": {
    "deprecated": {
      "jsonapi.pathPrefix": "This property has been deprecated and will be removed in the next version of Decoupled Router. Use basePath instead."
    }
  },
  "redirect": [
    {
      "from": "/node/5678?v=1",
      "to": "/my-content-url-alias?v=1",
      "status": "301"
    }
  ]
}
alexpott’s picture

Okay so resolved and redirect urls will obey the absolute setting but the urls inside jsonapi and entity will remain absolute. The one that is interesting is the canonical URL. I would guess that the front end think it exposes the canonical URL for the entity and not the backend.

alexpott’s picture

The current MR reflects what is wanted by #4.

alexpott’s picture

Title: Allow responses to contain relative URLs » Allow responses to resolve to relative URLs
alexpott’s picture

Issue summary: View changes
mglaman’s picture

Status: Needs review » Needs work

Tried to rebase via the UI and it failed on conflicts, otherwise good to go

alexpott’s picture

I'll update this once https://www.drupal.org/project/decoupled_router/issues/3543594 lands as that one conflicts with this one.

alexpott’s picture

Status: Needs work » Needs review
alexpott’s picture

Fixed up the MR after the latest merges

mglaman’s picture

Status: Needs review » Fixed

Thanks! Merge train ahead

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

  • mglaman committed 1a562c01 on 2.x authored by alexpott
    [#3543536] feat: Allow responses to resolve to relative URLs
    
    By:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.