[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:
Relates
relates to MULTISITE-48 When I modify the site evaluation rul... Closed
causality
dependency
depends upon MGNLADMLEG-60 Phase out Admin-interface Legacy Closed
supersession
is superseded by MAGNOLIA-6882 Remove legacy code that allows to acc... Closed
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   

MGNLDMS-188 introduced a fallback mechanism for the DMS to check if the handle prefix had been prerended incorrectly if the request came using the complete path to the item.

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 code is there not because of dms, but because of the handle/uri that the old tree was generating when you double clicked on something to open it. DMS was of course affected as well, but not only.
Long story short, if we get rid of it, means we are effectively killing those trees. And before we do that, we should stop using/shipping them. And to stop shipping admincentral legacy (and I fully agree that we should have done that while ago), we need to get rid of our own legacy code we still use. The legacy code removal is already happening, but perhaps we can accelerate that.

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.
Here are the notes I have about it:

/**
 * 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 MAGNOLIA-6882

Generated at Mon Feb 12 04:12:01 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.