Platform: @ngrx/data DefaultDataService, add config option for trailing slashes.

Created on 20 Aug 2019  路  10Comments  路  Source: ngrx/platform

While normally considered bad practice, there are APIs out there that require a trailing slash on endpoints, and as a front-end dev, we don't always have the ability to change this. I have been unable to use @ngrx/data with a particular API due to this, and looking through the HttpUrlGenerator code, it doesn't appear possible with the current version of @ngrx/data.

I have working code which does the following:

  1. adds optional trailingSlashes: boolean parameter to DefaultDataServiceConfig interface.
  2. adds trailingSlashes: boolean parameter to DefaultDataService<T> class. Defaults to false
  3. uses new DefaultDataService<T>.trailingSlashes to append / to all urls constructed in .add, .delete, etc.

It works great, allowing the use of APIs that require trailing slashes. I am just finishing up writing tests for it, and then can submit a PR.

_My code was a bit limiting. See new proposal in comment below._

Describe any alternatives/workarounds you're currently using

One workaround is to use an interceptor to always add a trailing slash. However this feels like overkill, and becomes tricky if you need to use httpClient for other non-trailing-slash endpoints/apis/urls. It seems it would be easier to inject the slash if required when building the url.

Other information:

If accepted, I would be willing to submit a PR for this feature

[X] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Data

Most helpful comment

Thinking about this a little more, I believe there is a more flexible way to do this. I went through a list of APIs I commonly use, and see the following requirement sets:

  1. No trailing slashes at all. E.g.: api/heroes, api/heroes/{heroId}.
  2. Trailing slashes on collections but not singe entities. E.g.: api/heroes/, api/heroes/{heroId}. This appears to be the current default behavior of @ngrx/data
  3. Trailing slashes on everything. E.g.: api/heroes/, api/heroes/{heroId}/

I propose the following change. Modify HttpResourceUrls to look like:

export interface HttpResourceUrls {
  /**
   * The URL path for a single entity endpoint, e.g, `some-api-root/hero`
   * such as you'd use to add a hero.
   * Example: `httpClient.post<Hero>('some-api-root/hero', addedHero)`.
   * note the lack of a trailing slash.  If you require trailing slashes on your
   * entity url, set the `entityResourceTrailingSlash` parameter to `true`.
   */
  entityResourceUrl: string;
  /**
   * Adds a trailing slash to the end of the entity resource URL if true
   * e.g.: `some-api-root/hero/` or `some-api-root/hero/{heroId}/`
   * Removes trailing slash at the end of entity resource URL if false
   * e.g.: `some-api-root/hero` or `some-api-root/hero/{heroId}`.
   * defaults to false if not defined.
   **/
  entityResourceTrailingSlash?: boolean;

  /**
   * The URL path for a multiple-entity endpoint, e.g, `some-api-root/heroes`
   * such as you'd use when getting all heroes.
   * Example: `httpClient.get<Hero[]>('some-api-root/heroes')`
   * note the lack of a trailing slash.  If you require trailing slashes on your
   * collection urls, set the `collectionResourceTrailingSlash` parameter to `true`.
   */
  collectionResourceUrl: string;
  /**
   * Adds a trailing slash to the end of the collection resource URL if true
   * e.g.: `some-api-root/heroes/`
   * Removes trailing slash at the end of entity resource URL if false
   * e.g.: `some-api-root/heroes`
   * defaults to true if not defined.
   **/
  collectionResourceTrailingSlash?: boolean;
}

HttpUrlGenerator is then modified to strip trailing slashes from entityResourceUrl and collectionResourceUrl, and have DefaultDataService handle injecting slashes where needed (both for separating path segments, and adding a trailing slash if required). By having entityResourceTrailingSlash default to false, and collectionResourceTrailingSlash default to true, this preserves the default behavior with no changes.

All 10 comments

Thinking about this a little more, I believe there is a more flexible way to do this. I went through a list of APIs I commonly use, and see the following requirement sets:

  1. No trailing slashes at all. E.g.: api/heroes, api/heroes/{heroId}.
  2. Trailing slashes on collections but not singe entities. E.g.: api/heroes/, api/heroes/{heroId}. This appears to be the current default behavior of @ngrx/data
  3. Trailing slashes on everything. E.g.: api/heroes/, api/heroes/{heroId}/

I propose the following change. Modify HttpResourceUrls to look like:

export interface HttpResourceUrls {
  /**
   * The URL path for a single entity endpoint, e.g, `some-api-root/hero`
   * such as you'd use to add a hero.
   * Example: `httpClient.post<Hero>('some-api-root/hero', addedHero)`.
   * note the lack of a trailing slash.  If you require trailing slashes on your
   * entity url, set the `entityResourceTrailingSlash` parameter to `true`.
   */
  entityResourceUrl: string;
  /**
   * Adds a trailing slash to the end of the entity resource URL if true
   * e.g.: `some-api-root/hero/` or `some-api-root/hero/{heroId}/`
   * Removes trailing slash at the end of entity resource URL if false
   * e.g.: `some-api-root/hero` or `some-api-root/hero/{heroId}`.
   * defaults to false if not defined.
   **/
  entityResourceTrailingSlash?: boolean;

  /**
   * The URL path for a multiple-entity endpoint, e.g, `some-api-root/heroes`
   * such as you'd use when getting all heroes.
   * Example: `httpClient.get<Hero[]>('some-api-root/heroes')`
   * note the lack of a trailing slash.  If you require trailing slashes on your
   * collection urls, set the `collectionResourceTrailingSlash` parameter to `true`.
   */
  collectionResourceUrl: string;
  /**
   * Adds a trailing slash to the end of the collection resource URL if true
   * e.g.: `some-api-root/heroes/`
   * Removes trailing slash at the end of entity resource URL if false
   * e.g.: `some-api-root/heroes`
   * defaults to true if not defined.
   **/
  collectionResourceTrailingSlash?: boolean;
}

HttpUrlGenerator is then modified to strip trailing slashes from entityResourceUrl and collectionResourceUrl, and have DefaultDataService handle injecting slashes where needed (both for separating path segments, and adding a trailing slash if required). By having entityResourceTrailingSlash default to false, and collectionResourceTrailingSlash default to true, this preserves the default behavior with no changes.

This will be useful. especially when your backend is based on django-rest-framework

It sounds like there is some interest in this, so I'll start working on a PR to implement this functionality.

@mxdmedia
You should write your own HttpUrlGenerator or extend from it and add your wish feature (@ngrx/data is fully extensible and customizable). When you extend from it, you just to hook into https://github.com/ngrx/platform/blob/0e9c4f4a20ca92a71ee331889109c7861708725f/modules/data/src/dataservices/http-url-generator.ts#L78 or as i said your write your own UrlGenerator (should be simple)

It is also important that you patch your service over the existing HttpUrlGenerator service in your module!

{ provide: HttpUrlGenerator, useClass: YourHttpUrlGenerator },

Modifying the HttpResourceUrls should not lead to breakingchange and it should be backward compatible.

This isn't documented https://ngrx.io/guide/data/extension-points#replace-the-httpurlgenerator

But it is really needed, I can't imagine anyone's real world rest api follows the out of the box conventions. In ours, we post to the plural endpoint e.g /api/heroes to create a new entity at that point.

I had success by:

`

import { DefaultHttpUrlGenerator, HttpResourceUrls, normalizeRoot, Pluralizer } from '@ngrx/data';
import { Injectable } from '@angular/core';

@Injectable()
export class AppHttpUrlGenerator extends DefaultHttpUrlGenerator {

constructor(private pluralize: Pluralizer) {
    super(pluralize);
}

public getResourceUrls(entityName: string, root: string): HttpResourceUrls {
    let resourceUrls = this.knownHttpResourceUrls[entityName];
    if (!resourceUrls) {
        const nRoot = normalizeRoot(root);
        resourceUrls = {
            entityResourceUrl: `${nRoot}/${this.pluralize.pluralize(
                entityName
            )}/`.toLowerCase(),
            collectionResourceUrl: `${nRoot}/${this.pluralize.pluralize(
                entityName
            )}/`.toLowerCase(),
        };
        this.registerHttpResourceUrls({ [entityName]: resourceUrls });
    }
    return resourceUrls;
}

}

`

And then overriding the provider like:

{ provide: HttpUrlGenerator, useClass: AppHttpUrlGenerator },

@httpete would you like to submit a PR to add the example to the docs?

We have a case where we need the root to be /api, not api, so the existence of normalizeRoot() in the flow caused us a bit of grief. Fortunately, once we understood what was happening, it was easy for us to skip past it because we had already replaced the HttpUrlGenerator, but it would have been onerous to change this if we hadn't already overridden the code that called normalizeRoot().

Why is normalizeRoot() used? Why does @ngrx/data assume that we can't have leading slashes?

And could there be a simpler way of disabling normalizeRoot(), when needed?

Yeah, it breaks absolute paths (like mentioned above) and protocol-relative URLs (e.g. //example.com/api), which we're in need of. So it would be great to either be able to configure the normalisation or change it to make less assumptions of how the API endpoint URLs should look like.

I'll take a look into this soon. If anything we can look at making it configurable

Has there been any update on making this option configurable?

I'm hitting an issue using AWS API Gateway, as that disallows URL endpoints with a trailing slash (not sure why) so the default Ngrx Data data service calls fail (with a 404).

Was this page helpful?
0 / 5 - 0 ratings