-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
REST API serves configuration inefficiently #9056
Comments
This was discussed in today's DevMtg. Generally consensus that we need to improve the manner in which we make backend configurations available to the frontend. All agree this is inefficient & this design doesn't scale well. Option 1: Some discussion of @J4bbi 's proposal (see "Solution" section of description above) to potentially remove the
Option 2: Another option is to update the existing endpoint to return all exposed configurations when calling
@abollini, @benbosman and @KevinVdV : I want to ping all three of you regarding this discussion as you / your teams were all involved with the original design of the |
This was discussed again in today's DevMtg. During that meeting, @abollini pointed out that this might be better resolved via a new /search endpoint like the following:
This would be a "canned search" (similar to /communities/search/top) which doesn't allow for any additional parameters. The behavior would be identical to "Option 2" in my prior comment. The two main benefits to this search endpoint are as follows:
|
Describe the bug
I think the configuration REST endpoint is inefficient.
It was created for a single use case, back in 2020. To pass the Google Analytics key to the front end from the backend. By now there are 17 backend configuration variables exposed via the rest end point
You can only get one variable per request at the moment, the findAll method is not implemented. A separate call is needed for each config you want, this creates overhead work and uses resources.
The RSS component, which is embedded by the way in the Pagination component makes at least two requests for metadata properties every time it's loaded.
A request for the Google Analytics key is being made on every single load of a page because the front end does not know if GA is enabled and needs to ask the backend every single time (see #9014).
It's not surprising that the new frontend is resource intensive if it is making several requests per page load.
Example
https://sandbox.dspace.org/browse/dateissued, half a sec wasted on GA. There is also another request for a metadata property there,
registration.verification.enabled
taking 185 ms and 1.1 kb, bandwidth and resourcesSolution
To deprecate the configuration REST endpoint as it will have no practical use.
Instead, repurpose the whitelisting functionality for the REST configuration endpoint to expose those properties in the root REST endpoint.
/server/api
.The root REST endpoint already fetches config to display so conceptually the root endpoint already exposes configuration properties.
Another reason to use the root endpoint is that, to the best of my understanding, every page load requests that endpoint early in it's build stage. So there is no need for a separate request for config.
it already has:
why not add
This would also resolve the issue of returning 404 or anything at all if a request is made for a non defined property.
Finally, this will increase efficiency by reducing the number of request per page load by anywhere from two to several, depending on the page.
The text was updated successfully, but these errors were encountered: