[MAGNOLIA-6176] Checking to see if prefix might have been prepended incorrectly is no longer needed Created: 21/Apr/15 Updated: 15/Dec/16 Resolved: 15/Dec/16 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | None |
| Affects Version/s: | 5.3.8 |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Richard Gange | Assignee: | Unassigned |
| Resolution: | Duplicate | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||
| Story Points: | 8 | ||||||||||||||||||||||||||||
| Description |
|
Snippet from URI2RepositoryMapping getHandle() method: try{ final Session session = MgnlContext.getJCRSession(this.repository); if (!session.itemExists(handle)) { String maybeHandle = (this.handlePrefix.endsWith("/") ? "/" : "") + StringUtils.removeStart(handle, this.handlePrefix); // prefix might have been prepended incorrectly. Second part of the condition is there to match links to binary nodes if (session.itemExists(maybeHandle) || (maybeHandle.lastIndexOf("/") > 0 && session.itemExists(StringUtils.substringBeforeLast(maybeHandle, "/")))) { return maybeHandle; } } This creates a situation where an item in the dms could have 2 potential request URLs, which is bad for SEO. Plus the dms doesn't exist in Magnolia 5 anyway. The new DAM has its own version of the mapping class DamURI2RepositoryMapping. This class doesn't even consider a configured handlePrefix in the method getHandle(). So for these reasons we should remove this this erroneous code from Magnolia 5. |
| Comments |
| Comment by Jan Haderka [ 12/Nov/15 ] |
|
If you remove said code, old trees stop working in multisite env ... and we still ship/support those with 5.4 and admincentral-legacy module. The only thing we can do until the above is resolved, would be to introduce flag in the mapping, say 4.xCompatibilityMode that would be set to true by default and whomever feels badly affected by the code could just switch it off ... and then w/ 5.5 we would remove the admincentrallegacy module and remove the offending block of code for good. |
| Comment by Nils Breunese [ 12/Nov/15 ] |
|
We have a multi-site env on Magnolia 4.5.26 with this code already removed. For this to work we also changed the order of the rules under config:/modules/extended-templating-kit/config/rules. See SUPPORT-4535 and other issues linked there for more info. |
| Comment by Jan Haderka [ 12/Nov/15 ] |
|
breun exactly, the removal of the code is not harmless to the old trees, unless some other changes like changing the rules is done too. Therefore we can't just remove the code. However we can allow you to disable it when you make your own custom order of rules in multisite/etk. |
| Comment by Nils Breunese [ 04/Jan/16 ] |
|
After migrating to M5 we discovered that URI2ResourcesRepositoryMapping copies the maybeHandle code in its own copy of getHandle, which caused some more mapping problems for us, so we'll probably need to override that class as well. |
| Comment by Richard Gange [ 05/Jan/16 ] |
|
breun can you explain a little further what kind of problems? FYI: URI2ResourcesRepositoryMapping is deprecated in magnolia 5.4. /**
* Uri to repository mapping for resources. Extension in node name is taken into account and has
* precedence before extension defined in the property called extension.
*
* @deprecated since 2.4 Resources are now served through the {@link info.magnolia.resourceloader.ResourceOrigin} API and {@link info.magnolia.module.resources.ResourcesServlet}; URI2RepositoryMapping isn't involved anymore.
*/
|
| Comment by Nils Breunese [ 05/Jan/16 ] |
|
The documentation at https://documentation.magnolia-cms.com/display/DOCS/Resources#Resources-Youcanstilluse/resourcesforprocessedresources says processed resources are still supported via the /resources prefix. This is handled by the configuration at config:/server/URI2RepositoryMapping/mappings/resources, which uses URI2ResourcesRepositoryMapping as its class. Sadly this class copies the strange 'maybeHandle' logic from URI2RepositoryMapping in its getHandle() method, which caused lots of headaches for us. (See SUPPORT-4535 for more details.) |
| Comment by Richard Gange [ 02/Aug/16 ] |
|
had do you think we can get this into 5.5? |
| Comment by AntonĂn Juran [ 15/Dec/16 ] |
|
Fixed by |