Merge pull request #18 from Rudd-O/main

Fix folder_contents being broken with VHM and Multilingual

------------------------------

When Multilingual is enabled and VHM is enabled too, serving any folderish's `folder_contents` under a language folder as a top level domain name is currently broken (if `en.site.com/archives` is VHMed to `PloneSite/en/archives` then `@@qsOptions` URL is `/archives/@@qsOptions` instead of being `/@@qsOptions` as it should be.

The fundamental problem is that the function `get_top_site_from_url(...)` is failing to retrieve the actual top site because it can't find a site in the traversal structure, so it picks the last folderish as the site, which is obvs broken because `/@@qsOptions` only works on site roots, not on folderish content.

This code makes it so that any site found in the structure is returned as the top site from the URL, but if no top site is found in that traversal, the actual site from `getSite()` is returned instead, which is the correct behavior based on how the function is named, and actually fixes the `/@@qsOptions` conundrum.

# Reproducer

Odd intersection of circumstances to reproduce bug:

1. Create classic `Plone` site.
2. Enable Multilingual with `en-US` or some other language.
3. Verify visiting `/Plone` redirects to `Plone/en-US` or your language folder of choice.
4. VHM your site so that `/Plone/en-US` is accessible as `/` (a standard, supported Multilingual rewrite configuration typically used by Ploners for language-code subdomain hosting).  `Plone/en-US/VirtualHostRoot` is the relevant VHM rewrite.
5. Visit the site.  Verify that site is accessible as `/`.
6. Create folder `xxx` on that location.
7. Visit folder `xxx` on that location.
8. Click on *Folder contents* in the editor toolbar.
9. Verify that the folder contents screen for `xxx` loads, but the actual listing remains empty.
10. Verify that there is an HTTP 404 for `@@qsOptions` in the network panel of your browser inspector.

# Rationale

This code makes the method sometimes return a content object in some cases, rather than an actual site root.

Why?

All callsites (see below) need a "root object" *from the perspective of the browser* to compute a root and generate various URLs for their own purposes, because all their purposes are XHR browser stuff (fill lists of objects and search results, chiefly).  This method I'm fixing determines the "root" used for that tree which is used in related items, image insertion, and folder contents browsing, and if that "root" is incorrect / does not correspond to what the browser sees as "root", **nothing works**.

Needless to say, this method should never be used to traverse anything server-side.

# Quality control

## Callsites 

My omelette is big.  There are not that many uses, and all its uses currently broken are fixed by this patch.  I count two uses:

1. `plone.app.content.browser.contents`:`ContentsBaseAction` / `get_options` / `ContextInfo.call`
2. `plone.app.widgets`:`get_relateditems_options`

## Unit and regression testing

All current unit tests pass now.  I have added a test case to cover the "root URL object is not actually a site" case, which was never tested before.  In addition to that, I have tested this with a local build (most up to date code) and here is what I can verify:

* Related items works now!
* Image insertion works as well.
* Browsing folders in folder contents works too.

So pretty much everything that was broken before, is no longer broken thanks to this modest change.  And that is the complete manual regression test of all users of this code.

news/18.bugfix

@@ -0,0 +1 @@
+fix `@@qsOptions` view (essentially, listing of `folder_contents`) when VHM roots the site on a plone.app.multilingual language folder as noted in https://github.com/plone/plone.app.content/issues/159 [Rudd-O]

src/plone/base/tests/test_utils.py

@@ -127,6 +127,13 @@ def physicalPathFromURL(self, url):
         ctx.vh_root = "/approot/PloneSite/folder/SubSite"
         self.assertEqual(get_top_site_from_url(ctx, req).id, "SubSite")
 
+        # Case 6 (VHM points to child of subsite, this bug existed 4 years):
+        req = MockRequest()
+        req.vh_root = "/approot/PloneSite/folder/SubSite/en"
+        ctx = MockContext("/approot/PloneSite/folder/SubSite/en/archives")
+        ctx.vh_root = "/approot/PloneSite/folder/SubSite/en"
+        self.assertEqual(get_top_site_from_url(ctx, req).id, "en")
+
     def test_human_readable_size_int(self):
         from plone.base.utils import human_readable_size
 

src/plone/base/utils.py

@@ -243,36 +243,95 @@ def _top_request(req):
 
 
 def get_top_site_from_url(context, request):
-    """Find the top-most site, which is still in the url path.
+    """
+    Find the first ISite object that appears in the pre-virtual-hosting URL
+    path, falling back to the object pointed by the virtual hosting root.
+    
+    Use this method if you need a "root object" reference to generate URLs
+    that will be used by, and will work correctly from the standpoint of,
+    *browser* code (JavaScript / XML HTTP requests) after virtual hosting has
+    been applied.  *Never* use this to get to a site root in Plone server code
+    -- it is not appropriate for that use.
 
     If the current context is within a subsite within a PloneSiteRoot and no
     virtual hosting is in place, the PloneSiteRoot is returned.
     When at the same context but in a virtual hosting environment with the
     virtual host root pointing to the subsite, it returns the subsite instead
-    the PloneSiteRoot.
+    the PloneSiteRoot.  Finally, if the virtual hosting environment points to
+    a *child* of a site/subsite, that child returns instead of the site/subsite.
 
     For this given content structure:
 
-    /Plone/Subsite
+    /Plone/Subsite:
+      /file
+      /en-US
+        /folder
+          /image
 
     It should return the following in these cases:
 
-    - No virtual hosting, URL path: /Plone, Returns: Plone Site
-    - No virtual hosting, URL path: /Plone/Subsite, Returns: Plone
-    - Virtual hosting roots to Subsite, URL path: /, Returns: Subsite
+    - No virtual hosting
+      URL path:              /Plone
+      Object accessed:       /Plone
+      Returns:               Plone
+
+    - No virtual hosting
+      URL path:              /Plone/Subsite
+      Object accessed:       /Plone/Subsite
+      Returns:               Plone
+
+    - Virtual hosting root:  /Plone/Subsite
+      URL path:              /
+      Object accessed:       /Plone/Subsite
+      Returns:               Subsite
+
+    - Virtual hosting root:  /Plone/Subsite
+      URL path:              /file
+      Object accessd:        /Plone/Subsite/file
+      Returns:               Subsite
+
+    - Virtual hosting root:  /Plone/Subsite/en-US
+      URL path:              /folder/image
+      Object accessed:       /Plone/Subsite/en-US/folder/image
+      Returns:               Subsite/en-US
+      (in this last case -- common with p.a.multilingual and usually described
+       as subdomain hosting for languages -- no Site object is visible TTW,
+       so it must return the topmost visible container, since the callees
+       need an object with a valid, TTW-visible URL to do their work.)
     """
     site = getSite()
     try:
+        # This variable collects all sites found during the traversal that
+        # takes place below, which only includes objects visible via VHM.
+        subsites = []
+        # This variable collect the topmost objects found during the
+        # traversal below, as fallback in case there are no sites found
+        # during the traversal.
+        topmosts = []
         url_path = urlparse(context.absolute_url()).path.split("/")
         for idx in range(len(url_path)):
             _path = "/".join(url_path[: idx + 1]) or "/"
             site_path = "/".join(request.physicalPathFromURL(_path)) or "/"
             _site = context.restrictedTraverse(site_path)
             if ISite.providedBy(_site):
-                break
-        if _site:
-            site = _site
-    except (ValueError, AttributeError):
+                subsites.append(idx)
+            else:
+                topmosts.append(idx)
+        # Pick the subsite to return.
+        # If no subsite was found, return the top site.
+        # If at some point a subsite was found, return the
+        # rootmost of all subsites.
+        # With VHM, sometimes the topmost site is not actually
+        # in the client URL, so in that case we fall back to
+        # the topmost accessible object within the client URL.
+        try:
+            _path_idx = subsites[0]
+        except IndexError:
+            _path_idx = topmosts[0]
+        _path = "/".join(url_path[: _path_idx + 1]) or "/"
+        site_path = "/".join(request.physicalPathFromURL(_path)) or "/"
+        site = context.restrictedTraverse(site_path)
+    except (ValueError, AttributeError) as exc:
         # On error, just return getSite.
         # Refs: https://github.com/plone/plone.app.content/issues/103
         # Also, TestRequest doesn't have physicalPathFromURL