The performance decreased because of a combination of calls that were supposed to increase performance.
We have a mechanism in place which automatically eager loads models needed in the element representers when a collection of them is rendered. This is to avoid N+1 queries of course. But, if eager loading is combined with e.g., a LIMIT, which we do because we paginate, rails automatically falls back to issuing two instead of just one SQL statement. Which makes sense as otherwise LEFT JOINS might mess with the result set.
But Rails does so in a somewhat simple fashion. It uses the first query to get the DISTINCT ids. The second is used to load the values (without a limit). But instead of removing all WHERE statements in the second SQL statement and then apply just the one for the ids, it keeps the original WHERE statement and applies the one for the ids on top. The problem with that is that the database trips on that (I didn't check the why) and uses a less than optimal query plan.
That was the problem here as well. The first query remained reasonable quick (300ms) but the second one took 25s.
The fix is to split the two statements by hand in the representer whenever eager loading is defined. The first query has all the filters but no eager loading and fetches the ids. The second takes the ids, and with eager loading included loads the data. Et voila, second query takes 10ms.
That at least works for relations, work_packages and projects. But there are other representers that also seem to have custom behaviour. I'm looking into whether they can be easily adapted.
We delegate the resource name to the object itself, that way we can
override it in our record and no knowledge about the backlogs module is
necessary in the core.
Adds `GET /api/v3/projects/:id/configuration` endpoint that returns
all global configuration properties plus project-specific settings.
This allows client apps to check both enterprise token features
(availableFeatures) and project settings (enabledInternalComments)
in a single API call.
Index endpoints will automatically discover and use
the `to_eager_load` and `to_preload` declarations of their
representers. This takes away the possibility of forgetting
to add those to the scope.
I found a few endpoint that didn't use them, when other endpoints
rendering the same collection did.
Interestingly the SCM integration modules consistently declared
those methods but never used them.
The manual call to #includes came from a time,
when the constraint was not #merge'd, but rather
enforced through a `where(id:)` call. It's confusing today,
because the #merge call makes sure to take over all kinds of
eager loading:
* includes
* eager_load
* preload
However, the code could leave the impression, that only includes
is supported.
Adding this method to the result makes it available to any
component involved in the SqlWalker implementation, so it
should be more reusable.
The code that's calculating the actual SELECT SQL statements is
not making use of the new helper, so some duplication still exists,
but hopefully we'll not deduplicate more than the current state.
* remove unnecessary scope from phase api
* remove Arel
* remove unnecessary module definition
* remove unused paths from api helper
also added a missing spec
"A reminder may be related to something that's not a work package conceptually.
Lean towards simpler canonical URLs for reminders (`/api/v3/reminders/:id`) where operations like `DELETE` and `PATCH` are targeted.
Because then we only need to define those routes once and the only thing we have to add when attaching reminders to a new container is `/api/v3/<container-name>/:id/reminders`."
~ https://github.com/opf/openproject/pull/19063#issuecomment-2935288456
Oliver Günther