Fix non-canonical crate name pages

[Turbo87]

Before the Ember Data 5 update it was possible to visit /crates/foo-bar when the crate was actually called foo_bar and vice-versa. The Ember Data 5 update broke this behavior, and this PR is restoring it, by using queryRecord() instead of findRecord(), which can no longer deal with requesting a certain id and getting a different one returned from the server.

Fixes #10368

[Turbo87]

as mentioned in the issue, ideally we would redirect the user to the same route, but with the crate name replaced. this appears to be non-trivial though, since the user could also have requested a child route and we would potentially redirect them to the wrong route. I think this fix matches the previous behavior though, so it's probably good enough for now :)

[eth3lbert]

Fair enough! I have no idea why this would be somewhat non-trivial in Ember, especially for nested routes 😅
Since queryRecord would make a request for every call, should we optimize this to something like peekRecord + queryRecord?

[Turbo87]

should we optimize this to something like peekRecord + queryRecord?

hmm, I guess that makes sense, yeah

[eth3lbert]

Aside from the missing e2e tests, this looks good to me. The optimization part could also be implemented in a separate PR later.

[eth3lbert]

I think we should also check this in e2e.

[eth3lbert]

I got an alternative workaround for this issue this morning. Since we know how to normalize crate.id, the response of the id is predictable. This means we could just make a unique lid for it. According to RFC¹, we could leverage the setIdentifierGenerationMethod to tell the store how to generate a lid, which should be something like the following:

// crates.io/app/initializers/ember-data-identifier.js
import { setIdentifierGenerationMethod } from '@ember-data/store';

function toCanoncialName(name) {
  return name.toLowerCase().replace(/_/g, '-');
}

export function initialize(_app) {
  const generationMethod = (resource, _bucket) => {
    let { id, lid, type } = resource;
    if (lid) {
      return lid;
    }

    if (id) {
      if (resource.type === 'crate') {
        id = toCanoncialName(id);
      }

      return `@lid:${type}-${id}`;
    }

    return crypto.randomUUID();
  };

  setIdentifierGenerationMethod(generationMethod);
}

export default {
  name: 'app.configure-ember-data-identifier',
  initialize,
};

This way, IDs with dashes or underscores would be canonicalized to a single unique lid.

This approach has the benefit of not requiring modifications to other business logic. However, the downside is that if the response id ever becomes unpredictable, the issue would occur again.

Footnotes

1. https://rfcs.emberjs.com/id/0403-ember-data-identifiers/ ↩

[Turbo87]

we could leverage the setIdentifierGenerationMethod to tell the store how to generate a lid

I like the approach, but unfortunately this seems to break our API token creation flow, at least with the proposed implementation. it seems to be missing the NEW_IDENTIFIERS part of https://github.com/emberjs/data/blob/v5.3.9/packages/store/src/-private/caches/identifier-cache.ts, but I'm not sure if reimplementing that is worth the complexity.

[eth3lbert]

I like the approach, but unfortunately this seems to break our API token creation flow, at least with the proposed implementation. it seems to be missing the NEW_IDENTIFIERS part of https://github.com/emberjs/data/blob/v5.3.9/packages/store/src/-private/caches/identifier-cache.ts,

Ah, that's indeed unfortunate. It would be nice if upstream exported the default generation method, then we could just use it as a fallback.

but I'm not sure if reimplementing that is worth the complexity.

Fair enough, unless the generation method approach could be easier to use without modifying other places, I think the adapter approach would make more sense.

[eth3lbert]

🙋 The canonical implementation in the backend(database) actually replaces - with _, but here we replace _ with -. Is this difference intentional?

[Turbo87]

Is this difference intentional?

no, it's not intentional. I guess ultimately it doesn't really matter which way around we do it, but I'll adjust it so that it matches.