Problem/Motivation
Currently, there is no caching of results on the endpoints, which might trigger the same request to the plugin source (ie: www.drupal.org).
There is also no validation of the parameters sent in the query, which will make the plugins' job a bit more difficult, so it is a good idea to filter and check it all in the controller before asking the plugin for the data.
Steps to reproduce
Request any of the endpoints (ie: releases) and a new request will be made each time, even if we are asking for the same endpoint with the same parameters.
Proposed resolution
Cache the results within the controller and validate the inputs received before asking for data to the plugin.
Remaining tasks
- ✅ File an issue about this project
- ☐ Addition/Change/Update/Fix to this project
- ☐ Testing to ensure no regression
- ☐ Automated unit/functional testing coverage
- ☐ Developer Documentation support on feature change/addition
- ☐ User Guide Documentation support on feature change/addition
- ☐ Code review from 1 Drupal core team member
- ☐ Full testing and approval
- ☐ Credit contributors
- ☐ Review with the product owner
- ☐ Release
User interface changes
None.
API changes
Introduced a new cache bin so we can clear the bin if the plugin ever changes, without needing to clear all the Drupal cache.
Data model changes
None.
Release notes snippet
Added validation to the parameters passed to the endpoints and cached the results to avoid multiple requests for the same information.
Issue fork project_browser-3279663
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commentedMR supplied. Marking as "Needs review".
Comment #4
fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commentedIf the related issue gets merged first (it's already RTBC) then we'll need to add some changes here. Changing to "Needs work" until that happens.
Comment #5
fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commentedRelated issue was merged, so I rebased this branch and did the follow-up changes.
This MR validates/standardises the input from the query parameters and also caches the results per query.
If you're testing this locally for the first time, remember to do `drush cr` before anything else.
Ready to review.
Comment #7
chrisfromredfinThis has been rebased off latest and tested for regressions.
Comment #8
fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commentedNeed to add the new filters added here: https://www.drupal.org/project/project_browser/issues/3240320
Comment #9
fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commentedComment #10
chrisfromredfinRe-checked against latest. RTBC as long as the tests pass.
Comment #16
tim.plunkettMerged, thanks!