Bound the tree children query and CONSTRUCT only what a node renders

src/main/webapp/static/com/atomgraph/linkeddatahub/xsl/client/navigation.xsl

@@ -269,7 +269,16 @@ ORDER BY DESC(?created)
     <xsl:function name="ldh:doc-tree-children-query" as="xs:string">
         <xsl:param name="uri" as="xs:anyURI"/>
 
-        <xsl:sequence select="ldh:tree-children-query($uri, (xs:anyURI('&sioc;has_parent'), xs:anyURI('&sioc;has_container')), ())"/>
+        <xsl:sequence select="ldh:doc-tree-children-query($uri, ())"/>
+    </xsl:function>
+
+    <!-- $path-uri: the document being opened to, so the child leading to it is in the page even when
+         the container holds more children than one fetch returns -->
+    <xsl:function name="ldh:doc-tree-children-query" as="xs:string">
+        <xsl:param name="uri" as="xs:anyURI"/>
+        <xsl:param name="path-uri" as="xs:anyURI?"/>
+
+        <xsl:sequence select="ldh:tree-children-query($uri, (xs:anyURI('&sioc;has_parent'), xs:anyURI('&sioc;has_container')), (), $path-uri)"/>
     </xsl:function>
 
     <!-- binds the drawer's tree to containment. client/tree.xsl dispatches on the disclosure button, so
@@ -503,7 +512,7 @@ ORDER BY DESC(?created)
                         <xsl:call-template name="ldh:TreeChildrenFetch">
                             <xsl:with-param name="container" select="$current-li/ul"/>
                             <xsl:with-param name="uri" select="$current-href"/>
-                            <xsl:with-param name="query" select="ldh:doc-tree-children-query($current-href)"/>
+                            <xsl:with-param name="query" select="ldh:doc-tree-children-query($current-href, $target-uri)"/>
                             <xsl:with-param name="then" select="ldh:doctree-descend-after-load(?, $current-li, $target-uri, $tree-container)"/>
                         </xsl:call-template>
 

src/main/webapp/static/com/atomgraph/linkeddatahub/xsl/client/tree.xsl

@@ -34,6 +34,12 @@ exclude-result-prefixes="#all"
         load-bearing rather than cosmetic.
     -->
 
+    <!-- How many children one fetch returns. A tree is a navigation aid, not a listing: a container
+         with more children than this shows the first page of them (plus the one being opened to), and
+         the alternative is not "all of them" but the response-size failure that unbounded DESCRIBEs
+         produced over large containers. Raise it in an app stylesheet if its containers warrant it. -->
+    <xsl:param name="ldh:tree-page-size" select="1000" as="xs:integer"/>
+
     <!-- The children of a node, as a query this module generates rather than one the domain writes.
          A tree is defined by the relation it follows, so that relation is the parameter: properties
          asserted on the child pointing at its parent, and - since RDF lets either end carry the link -
@@ -50,32 +56,58 @@ exclude-result-prefixes="#all"
         <xsl:param name="parent-properties" as="xs:anyURI*"/> <!-- asserted on the child: ?child P $this -->
         <xsl:param name="child-properties" as="xs:anyURI*"/> <!-- asserted on the parent: $this P ?child -->
 
+        <xsl:sequence select="ldh:tree-children-query($uri, $parent-properties, $child-properties, ())"/>
+    </xsl:function>
+
+    <!-- $path-uri is the document the tree is opening to, if any: the child leading to it is included
+         whatever the page holds, so a descent through a container larger than the page still finds its
+         next step (ldh:doctree-descend picks it by prefix, and would otherwise stop silently). -->
+    <xsl:function name="ldh:tree-children-query" as="xs:string">
+        <xsl:param name="uri" as="xs:anyURI"/>
+        <xsl:param name="parent-properties" as="xs:anyURI*"/>
+        <xsl:param name="child-properties" as="xs:anyURI*"/>
+        <xsl:param name="path-uri" as="xs:anyURI?"/>
+
         <xsl:variable name="branches" as="xs:string*" select="
             (for $property in $parent-properties return '{ ?child &lt;' || $property || '&gt; &lt;' || $uri || '&gt; }'),
             (for $property in $child-properties return '{ &lt;' || $uri || '&gt; &lt;' || $property || '&gt; ?child }')"/>
         <xsl:if test="empty($branches)">
             <xsl:message terminate="yes">ldh:tree-children-query requires at least one parent or child property</xsl:message>
         </xsl:if>
 
+        <xsl:variable name="links" select="string-join($branches, ' UNION ')" as="xs:string"/>
+        <!-- the relation the tree follows, echoed back on each child: it is what a domain's
+             ldh:TreeNode rule reads to decide whether a node opens (the document tree keys on
+             sioc:has_parent, which only a container has). Generic because the relation is the
+             parameter - whatever a tree is built on is what its nodes are judged by. -->
+        <xsl:variable name="tree-properties" select="distinct-values(($parent-properties, $child-properties))" as="xs:anyURI*"/>
+
         <!-- the link and the child's own description are in DIFFERENT graphs whenever the link is
              asserted on the parent, because each document is its own graph: a scheme's
              skos:hasTopConcept lives in the scheme's graph while the concept's rdf:type lives in the
              concept's. Scoping both to one GRAPH silently drops every child linked from above -
              measured against a fixture where it returned one top concept of two. -->
-        <!-- A DESCRIBE, written out whole rather than a SELECT for something else to wrap, and handed
-             to the endpoint as the string it already is. It used to go through SPARQLBuilder twice -
-             parsed from a string here, re-serialised in the fetch - and that round-trip MERGED the two
+        <!-- Written out whole rather than a SELECT for something else to wrap, and handed to the
+             endpoint as the string it already is. It used to go through SPARQLBuilder twice - parsed
+             from a string here, re-serialised in the fetch - and that round-trip MERGED the two
              sibling GRAPH blocks into one keeping only the last graph variable, putting the type
              requirement back inside the link's graph and silently dropping every child linked from the
-             parent side. Measured: the query left here correctly scoped and arrived at the endpoint as
-             GRAPH ?childGraph { {..} UNION {..} ?child a ?Type }, returning one top concept of two.
-             Wrapping the second block in a group did not survive either. Nothing needed the parse -
-             this query is generated, not authored or edited - so the scoping the comment above
-             describes is now the scoping that gets sent. -->
+             parent side. Nothing needed the parse - this query is generated, not authored or edited. -->
+        <!-- A CONSTRUCT of exactly what a node renders - its type for the icon, its labels, and the
+             tree relation for the disclosure - not a DESCRIBE of everything the child happens to say.
+             A DESCRIBE returns whole documents: measured over a container of 28k children it was
+             20.5 MB against 6.6 MB here, and over one of 65k it exceeded the platform's response limit
+             outright, so the tree answered 502 on every page under it. The page bound is what makes
+             that independent of container size; the children are sorted by ac:label() as they are
+             rendered, so ORDER BY only has to make the page itself deterministic. -->
         <xsl:sequence select="
-            'DESCRIBE ?child WHERE { GRAPH ?linkGraph { ' ||
-            string-join($branches, ' UNION ') ||
-            ' } GRAPH ?childGraph { ?child a ?Type } }'"/>
+            'CONSTRUCT { ?child a ?Type . ?child ?labelProp ?label . ?child ?treeProp ?treeValue }&#10;' ||
+            'WHERE { { SELECT DISTINCT ?child WHERE { GRAPH ?linkGraph { ' || $links || ' } } ORDER BY ?child LIMIT ' || $ldh:tree-page-size || ' }' ||
+            (if (exists($path-uri)) then ' UNION { GRAPH ?pathGraph { ' || $links || ' } FILTER (strstarts(&quot;' || $path-uri || '&quot;, str(?child))) }' else '') ||
+            '&#10;GRAPH ?childGraph { ?child a ?Type' ||
+            ' OPTIONAL { ?child ?labelProp ?label FILTER (?labelProp IN (' || string-join(for $property in $ldh:label-properties return '&lt;' || $property || '&gt;', ', ') || ')) }' ||
+            (if (exists($tree-properties)) then ' OPTIONAL { ?child ?treeProp ?treeValue FILTER (?treeProp IN (' || string-join(for $property in $tree-properties return '&lt;' || $property || '&gt;', ', ') || ')) }' else '') ||
+            ' } }'"/>
     </xsl:function>
 
     <!-- EVENT HANDLERS -->

ui-tests/lib/console.mjs

@@ -20,11 +20,11 @@ const seen = [endUserBase, adminBase].map(base => ({
 
 // Noise that is not this build's fault. Excluded by pattern and listed here with its
 // reason, rather than by loosening the assertion for everything.
-const preexisting = [
-    // The unlimited DESCRIBE over a large container 502s on the dev stack, and reproduces
-    // on builds predating this suite. Tracked separately; see commit 3305741ac.
-    { pattern: /sparql\?query=DESCRIBE[\s\S]*has_parent/i, reason: 'pre-existing 502 on unlimited DESCRIBE' },
-];
+// Empty, and worth keeping so: it held one entry, for the 502 the unlimited DESCRIBE of a
+// large container produced. That query is now a bounded CONSTRUCT, so the 502 is gone and the
+// suppression with it - an allowance that outlives its defect stops being an allowance and
+// becomes a blind spot over exactly the request the fix was about.
+const preexisting = [];
 
 const allowed = (text, declared) =>
     [...preexisting, ...declared].some(({ pattern }) => pattern.test(text));

ui-tests/specs/concept-tree.spec.mjs

@@ -26,14 +26,20 @@ const rowsFor = (page, name) =>
 const disclosureOf = (page, name) => rowsFor(page, name).locator('> div.tree-row > button');
 
 // Every concept-tree fetch, counted: the hops up are a SELECT for ?parent, the levels down
-// are the shared DESCRIBE of a node's children.
+// are the shared CONSTRUCT of a node's children.
+//
+// The down pattern matches the projection, not the verb: the verb stopped being distinctive
+// when the children query became a CONSTRUCT, and CONSTRUCT alone would also count whatever
+// else the page constructs. Matching what only this query projects is also what keeps the
+// counter honest - while it matched nothing, `the DOM is the cache` compared 0 against 0 and
+// passed without asserting anything.
 function countQueries(page) {
     const counts = { up: 0, down: 0 };
     page.on('response', response => {
         const url = decodeURIComponent(response.url());
         if (!url.includes('/sparql?')) return;
         if (/SELECT DISTINCT \?parent WHERE/.test(url)) counts.up++;
-        else if (/DESCRIBE \?child/.test(url)) counts.down++;
+        else if (/CONSTRUCT \{ \?child a \?Type/.test(url)) counts.down++;
     });
     return counts;
 }

ui-tests/specs/document-tree.spec.mjs

@@ -46,6 +46,20 @@ async function slowChildren(page, ms = 1500) {
         });
 }
 
+// Empties the children page on the wire, so a fixture container behaves like one holding more
+// children than a single fetch returns - without needing thousands of documents to build one.
+//
+// LIMIT 0 rather than a small number because the bound applies to the inner subselect alone:
+// with it at zero the page contributes nothing and every row that still appears got there
+// through the branch that pins the document being opened to. That makes the spec below a
+// statement about that branch rather than about the page size, and it fails the way the
+// missing branch would - a tree that stops before it reaches you.
+async function emptyChildrenPage(page) {
+    await page.route(
+        url => url.href.includes('/sparql?') && decodeURIComponent(url.href).includes('has_parent'),
+        route => route.continue({ url: route.request().url().replace(/LIMIT%20\d+/i, 'LIMIT%200') }));
+}
+
 test.describe('document tree', () => {
     test('opens the path down to the document being read and marks it', async ({ page }) => {
         await goto(page, itemUri(1));
@@ -57,6 +71,45 @@ test.describe('document tree', () => {
             .toHaveAttribute('aria-current', 'page');
     });
 
+    test('asks for a bounded page of children, and only what a node renders', async ({ page }) => {
+        const queries = [];
+        page.on('request', request => {
+            const url = decodeURIComponent(request.url());
+            if (url.includes('/sparql?') && url.includes('has_parent')) queries.push(url);
+        });
+
+        await goto(page, itemUri(1));
+        await openDrawer(page);
+        await expect(rowsFor(page, itemUri(1))).toHaveClass(/is-active/);
+
+        // The query this replaced was an unbounded DESCRIBE, which returns whole child
+        // documents: 20.5 MB over a container of 28k, and past the response limit over one of
+        // 65k, so the tree answered 502 on every page beneath it. Both halves matter - a
+        // CONSTRUCT that lost its bound would still grow without limit, and a bounded DESCRIBE
+        // would still carry every triple of every child it returned.
+        expect(queries.length, 'the descent should have fetched children').toBeGreaterThan(0);
+        for (const query of queries) {
+            expect(query, 'the children query must stay bounded').toMatch(/LIMIT \d+/);
+            expect(query, 'a DESCRIBE returns whole child documents').not.toMatch(/DESCRIBE/);
+        }
+    });
+
+    test('opens the path even when it falls outside the page of children', async ({ page }) => {
+        await emptyChildrenPage(page);
+
+        await goto(page, itemUri(1));
+        await openDrawer(page);
+
+        // ldh:doctree-descend picks its next step out of the rendered children and falls
+        // through silently when it is absent - so bounding the page without pinning the path
+        // would leave the reader at the top of a hierarchy with no sign of where they are,
+        // and nothing in the console to say so.
+        await expect(disclosureOf(page, fixtures.container)).toHaveAttribute('aria-expanded', 'true');
+        await expect(rowsFor(page, itemUri(1))).toHaveClass(/is-active/);
+        await expect(rowsFor(page, itemUri(1)).locator('> div.tree-row > a'))
+            .toHaveAttribute('aria-current', 'page');
+    });
+
     test('indents each level it opens', async ({ page }) => {
         await goto(page, itemUri(1));
         await openDrawer(page);

ui-tests/specs/tree-children-failure.spec.mjs

@@ -32,7 +32,12 @@ import { goto } from '../lib/settle.mjs';
 import { fixtures, itemTitle, itemUri } from '../lib/fixtures.mjs';
 
 // The query the tree asks for one node's children, as ldh:TreeChildrenFetch sends it.
-const CHILDREN_QUERY = /\/sparql\?query=DESCRIBE[\s\S]*has_parent/i;
+//
+// It routes on the wire form, so it has to track the query's shape: this matched DESCRIBE
+// until the children query became a bounded CONSTRUCT, and a route pattern that matches
+// nothing fails open - the 403 below is never injected and every assertion here passes
+// against a tree that was never refused anything.
+const CHILDREN_QUERY = /\/sparql\?query=CONSTRUCT[\s\S]*has_parent/i;
 
 // What the server really answers when the endpoint is refused: an http:Response whose subject is a
 // blank node. Served verbatim so the client meets the body it would meet in production, not an