[MGNLRES-144] Implement new origin-based ResourcesServlet Created: 07/May/15  Updated: 29/Mar/22  Resolved: 28/May/15

Status: Closed
Project: Magnolia Resources Module
Component/s: None
Affects Version/s: None
Fix Version/s: 2.4

Type: New Feature Priority: Major
Reporter: Mikaël Geljić Assignee: Magnolia International
Resolution: Fixed Votes: 0
Labels: platform-cell, servlet
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
causality
is causing MULTISITE-44 Review default bypasses for CrossSite... Open
dependency
depends upon MGNLRES-152 Generate links to resources, and use ... Closed
depends upon MAGNOLIA-6218 Expose resource's last modification date Closed
depends upon MAGNOLIA-6219 ClasspathOrigin: filter resources out... Closed
depends upon MAGNOLIA-6220 Resources: clarify pathPattern usage ... Closed
is depended upon by MAGNOLIA-6229 Deprecate ClasspathSpool servlet Closed
supersession
supersedes MGNLRES-136 Add ability to serve resources direct... Closed
is superseded by MGNLRES-149 Concept for supporting processing res... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
MGNLRES-161 Integration test for ResourcesServlet Sub-task Closed Michael Mühlebach  
Template:
Acceptance criteria:
Empty
Date of First Response:
Epic Link: Streamline working with resources
Team: Nucleus

 Description   

Should use the new magnolia-resource-loader module from main, and in particular use the LayeredOrigin which implements the unified resource-loading cascade.

Is expected to replace ClasspathSpool servlet and be similarly mapped to /.resources.

Random thoughts/things to clarify:

Processed resources

  • What to do with Resource#modelClass
  • What to do with nested processed resources as sub-nodes (e.g. sample-css)
  • Encourage additional generators as filters (e.g. Sass)
  • see MGNLRES-149

Security: What should the servlet be allowed to serve?

  • restriction e.g. to serve css/js, not serve ftls, yamls
  • relevant for the whole resource-loading cascade
  • should leave it to URI security where applicable

Caching

  • apparently ok /.resources is cached in production mode

Compatibility options

  • Deprecate ResourceLoaders
  • Single resource handle for fresh installs
  • Still support /resources with meaningful warnings otherwise


 Comments   
Comment by Michael Mühlebach [ 18/May/15 ]

Because it would introduce a dependency cycle at the moment, the resources servlet can not replace the class path spool in the install/setup phase (FilterManagerImpl).

Comment by Michael Mühlebach [ 18/May/15 ]

The processed resources are out of scope for the moment. They can still be used but over the old NonInstalledResourcesFilter (aka /resources/ path).
A follow up task will add processed resource support to the resources servlet.

Comment by Magnolia International [ 23/May/15 ]

Did a bunch of cleanups with Michael. Still reopened for tests, integration tests, and missing update tasks.

Comment by Michael Mühlebach [ 27/May/15 ]

Finished the clean up and adapted all tests accordingly.
Added update tasks for core to remove the class path spool servlet.
Added update tasks for the resources module to add the new resources servlet.

Comment by Magnolia International [ 27/May/15 ]
  • ResourcesModuleVersionHandler does not compile
  • I'd consider having a single "replace classpathSpool servlet by this new one" in the resources module only, and drop the task from core (if one updates core without updating resources module, they'll have no resources servlet)
  • I'd suggest adding log.warn messages in the now deprecated Servlet, in case it still gets used.
  • Some of the changes we did together (in a branch that's now been removed) are gone
  • You re-introduce stripPrefixesBeforeResourcesURLMapping specifically because one of those crucial change is gone (using pathInfo instead of requestURI)
  • NonInstalledResourcesFilter needs to be removed !
Comment by Magnolia International [ 27/May/15 ]

Additionally, I don't think we should release this without any sort of validation (at the very least by content-type, extension, and/or pattern)

Comment by Magnolia International [ 27/May/15 ]

Cleaned up, rebased and squashed on feature/MGNLRES-144-resources-servlet-2.
Still to do:

  • consider having a single "replace classpathSpool servlet by this new one" update task in the resources module only, and drop the task from core (if one updates core without updating resources module, they'll have no resources servlet)
  • add log.warn messages in the now deprecated Servlet, in case it still gets used.
  • validation/filtering of what the servlet can serve
  • I did not run the integration tests. Preliminary manual tests look good, provided we fix MGNLRES-152. CMS team should be able to replace their links to /resources by /.resources
Comment by Michael Mühlebach [ 28/May/15 ]

Moved update task for class path spool servlet to the resources module.
Added log message to class path spool servlet.
Added restriction pattern for forbidden resources like ftl files.
Added integration test for content type.

Comment by Magnolia International [ 28/May/15 ]

Looks good, cheers.
Note to self when integrating:

  • re-test #86da633 - report in separate ticket if the existing cache config doesn't cover this
  • don't forget feature/MGNLRES-144-resource-servlet-integration-test branch on ce-bundle
  • un-ignore 3rd test in said branch (ideally, everything will be integrated by then)
  • add a test that validates forbidden resources are in fact forbidden
Comment by Magnolia International [ 29/May/15 ]

Integrated on master, but created MGNLRES-161 to cover integration tests

Generated at Mon Feb 12 06:47:50 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.