Jsonapi-resources: Link generation performance

[lgebhardt]

This issue is a (choose one):

• [X] Problem/bug report.
• [ ] Feature request.
• [ ] Request for support. Note: Please try to avoid submitting issues for support requests. Use Gitter instead.

Checklist before submitting:

• [ ] I've searched for an existing issue.
• [ ] I've asked my question on Gitter and have not received a satisfactory answer.
• [ ] I've included a complete bug report template. This step helps us and allows us to see the bug without trying to reproduce the problem from your description. It helps you because you will frequently detect if it's a problem specific to your project.
• [ ] The feature I'm asking for is compliant with the JSON:API spec.

Description

1247 introduced route based links. This uses the Rails url_helpers to generate the links. It turns out there are two issues with this. The first is the Rails.application.routes.url_helpers no longer memoizes the generated module, which results in a lot of extra modules being created, and possibly not garbage collected. This is corrected in #1257 by using the controller as it has the url_helper module included.

However there's still a performance issue. In testing using the url_helpers is significantly slower than the old way of generating the links (string concatenation). But the url_helpers generated links are generated from the actual routes, which allows us to ensure the route for the resource or relationship actually exists and issue warnings if that's not the case.

What we need is the speed of the earlier string based LinkBuilder with the ability to detect missing routes of the url_helpers based LinkBuilder.

I'd like to discuss options in this issue before I start coding up the fix.

[lgebhardt]

One possible solution is to provide an option, in development and test environments, to use the url_helpers to generate the route and compare it to the string built up results and warn if different. This is probably the simplest solution, but it isn't elegant. It will slow down development runtimes, but I'm not sure it's a large enough effect to rule this out.

[lgebhardt]

In development we could inspect resources and see that the expected url_helper methods exist. To work the check would need to be done after the routes are loaded (and reloaded) and the resource classes are defined. Ideally it would also handle runtime creation of routes and resources.

[krasnoukhov]

What if JR still uses Rails routes to determine (just once per unique route) if there's a specific resource/relationship route available and then uses the dumb approach to generate the actual URLs? I think with that approach we'll get best of both worlds – current behavior of not including invalid URLs and the speed of dumb link generation. The only drawback I can see right now would be that this route availability cache will have to be cleared somehow for code reload in development. What do you think?

[lgebhardt]

I think we could do that by checking if the url_helper method exists. On resource or resource route definition we could wipe the cache. Adding the checks and a set to track if we've already checked will add some overhead, but it should be minimal. Still I think this mode should be optional to at least allow it to be turned off for production (or maybe better turned on for development and test?).

[krasnoukhov]

That makes sense. If this option is going to be optional, we'd like to be able to enforce it even for production, since we don't define all resource/relationship URLs and we'll want them to not be included since they will 404 anyway.

[lgebhardt]

@krasnoukhov I went with an approach of "marking" the resources and relationships when routes are setup for them. That makes the checks fast. Take a look at #1262 and see if that works for you. I'll review it again with fresh eyes in the morning.

[krasnoukhov]

Thanks @lgebhardt! I think this is great. Our suite is passing, just left 1 comment on the PR

[lgebhardt]

@krasnoukhov I've restored the warn_on_missing_routes configuration option to both PRs.