diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9a9a8..d171d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ All notable changes are documented here, following [Keep a Changelog](https://ke ### Fixed - Reads of a table that has no tag assignment no longer query `collaboration_record_tag`. Core selects the Tags field for every query or get that names no fields, so each of them (including per-record gets and the stored-record read before an update) ran one assignment statement whether or not the table had ever been tagged: 600 gets of an untagged table ran 600 statements, now none. Each node remembers which tables have no assignment (`TaggedTables`, core `Memoization` registered with `ClusterCaches`) for `CollaborationQBitConfig.withTagLookupCacheSeconds` (default 30, 0 disables); writing an assignment clears it on the node and through the ClusterBus. Tables with assignments are read as before, one lookup per 500 parents. No API or schema change. See "What the Tags field costs a read" in the README. +- Browsing one of the qbit's child tables (comments, tasks, record tags, attachments and the other parent-referencing tables) reads each distinct parent once per result page to authorize its rows, instead of once per row: 61 tasks on two parents ran 61 parent reads, now 2. The check itself and its outcome are unchanged. - The retained upload journal's PostgreSQL 17 qualification accepts Amazon Aurora PostgreSQL, which reports `full_page_writes=off` and does not let a deployment change it. `off` is accepted only when the settings query detects Aurora (`to_regprocedure('aurora_version()') IS NOT NULL`); stock PostgreSQL with `full_page_writes=off` is still refused, and every other requirement is unchanged on Aurora. See `docs/RETAINED-UPLOAD-JOURNAL.md`. - Bounded STRING custom fields carry QQQ's native `ValueTooLongBehavior.ERROR`, so a write longer than `maxLength` is rejected with a field error and the stored value is preserved, instead of passing through unchecked (the QQQ default `PASS_THROUGH`). - Persisted help content is reloaded after each committed runtime metadata refresh, so sections and fields created by publishing a custom-field definition receive their stored `helpContent` rows without a restart. diff --git a/README.md b/README.md index b39767f..ba46795 100644 --- a/README.md +++ b/README.md @@ -409,7 +409,7 @@ size, pending suggestions, and the five most-used approved tags. `collaborationTags` is a virtual STRING field labeled Tags with possible-value source `collaboration_tag`. `TagCriteriaExpander` supports IN, NOT_IN, EQUALS, NOT_EQUALS, IS_BLANK, IS_NOT_BLANK and CONTAINS_ALL using core `QSubQuery` membership filters. Ordinary quick search matches labels; `#name` matches normalized names. Numeric criteria are tag IDs; string criteria are names. The old ID materializer is removed. Core owns SQL execution or bounded cross-backend materialization; its bound applies to inner rows, not final parent matches. -The runtime provider adds this field and its search entry where TAGS is enabled. It is queryable but not selectable: the overlay has no virtual-field post-query hook and existing host customizers must be preserved. +The runtime provider adds this field and its search entry where TAGS is enabled. It is queryable and selectable (`TagValueProvider` fills it after host customizers have run). ### What the Tags field costs a read diff --git a/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java b/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java index bee456c..2b607ad 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/attachments/AttachmentPreviewCustomizer.java @@ -55,6 +55,7 @@ public class AttachmentPreviewCustomizer implements TableCustomizerInterface List filter(List records, boolean isGet, boolean generateUrls, AttachmentsService service) throws QException { List visible = new ArrayList<>(); + ParentRecordAccess.ResultSetParents parents = new ParentRecordAccess.ResultSetParents(); for(QRecord record : records) { try @@ -63,7 +64,7 @@ public class AttachmentPreviewCustomizer implements TableCustomizerInterface { throw (new QNotFoundException("Not found")); } - ParentRecordAccess.requireVisibleParent(record.getValueString("tableName"), record.getValueString("recordId")); + parents.requireVisible(record.getValueString("tableName"), record.getValueString("recordId")); } catch(QNotFoundException expected) { diff --git a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java index 91546a7..35703f5 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRecordAccess.java @@ -81,6 +81,42 @@ public final class ParentRecordAccess } } + /******************************************************************************* + ** One result set's parent checks: rows that share a parent cost one lookup. + ** The session and transaction do not change within the call that creates it, + ** so never keep an instance beyond that call. + *******************************************************************************/ + public static final class ResultSetParents + { + private final java.util.Map, Boolean> visible = new java.util.HashMap<>(); + + /******************************************************************************* + ** Same outcome as ParentRecordAccess.requireVisibleParent for every row. + *******************************************************************************/ + public void requireVisible(String tableName, String recordId) throws QException + { + java.util.List parent = java.util.Arrays.asList(tableName, recordId); + Boolean known = visible.get(parent); + if(known == null) + { + try + { + requireVisibleParent(tableName, recordId); + known = true; + } + catch(QNotFoundException expected) + { + known = false; + } + visible.put(parent, known); + } + if(!known) + { + throw (new QNotFoundException("Not found")); + } + } + } + /******************************************************************************* ** Core record capability checks include parent WRITE locks for implied grants. *******************************************************************************/ diff --git a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java index b453dec..7e3ffca 100644 --- a/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java +++ b/src/main/java/com/kingsrook/qbits/collaboration/security/ParentRowCustomizer.java @@ -43,6 +43,7 @@ public class ParentRowCustomizer implements TableCustomizerInterface public List postQuery(QueryOrGetInputInterface input, List records) throws QException { List visible = new ArrayList<>(); + ParentRecordAccess.ResultSetParents parents = new ParentRecordAccess.ResultSetParents(); for(QRecord record : records) { try @@ -51,7 +52,7 @@ public class ParentRowCustomizer implements TableCustomizerInterface { throw (new QNotFoundException("Not found")); } - ParentRecordAccess.requireVisibleParent(record.getValueString("tableName"), record.getValueString("recordId")); + parents.requireVisible(record.getValueString("tableName"), record.getValueString("recordId")); QRecord row = new QRecord(record); if(row.getValue("deletedAt") != null) { diff --git a/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java index 5819d4d..f44bfce 100644 --- a/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java +++ b/src/test/java/com/kingsrook/qbits/collaboration/TagLookupStatementBudgetTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; import com.kingsrook.qbits.collaboration.model.CollaborationTables; +import com.kingsrook.qbits.collaboration.permissions.CollaborationPermissions; import com.kingsrook.qbits.collaboration.tags.TagEntry; import com.kingsrook.qbits.collaboration.tags.TaggedTables; import com.kingsrook.qbits.collaboration.tags.TagsService; @@ -251,6 +252,27 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture + /******************************************************************************* + ** Browsing one of this qbit's child tables authorizes each distinct parent + ** once, not once per row (61 rows on two parents ran 61 parent reads before). + *******************************************************************************/ + @Test + void childRowsSharingAParentCostOneParentLookup() throws Exception + { + List tasks = new ArrayList<>(); + for(int id = 1; id <= 61; id++) + { + tasks.add(new QRecord().withValue("id", id).withValue("tableName", "document").withValue("recordId", id == 61 ? "2" : "1") + .withValue("title", "Task " + id).withValue("assigneeUserId", "alice").withValue("createdByUserId", "alice") + .withValue("status", "OPEN").withValue("parentDeleted", false)); + } + MemoryRecordStore.getInstance().insert(new InsertInput(CollaborationTables.TASK).withRecords(tasks), true); + login("alice", CollaborationPermissions.VIEW); + assertThat(backendQueries(() -> assertThat(new QueryAction().execute(new QueryInput(CollaborationTables.TASK)).getRecords()).hasSize(61)).get("document")).isEqualTo(2L); + } + + + /******************************************************************************* ** A static cache that is not registered would go stale on the other nodes. *******************************************************************************/ @@ -311,6 +333,18 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture ** Backend queries against this qbit's tables while the work runs, by table. *******************************************************************************/ private Map collaborationQueries(UnsafeVoidVoidMethod work) throws Exception + { + Map result = backendQueries(work); + result.keySet().removeIf(table -> !table.startsWith(CollaborationQBitConfig.TABLE_NAME_PREFIX)); + return (result); + } + + + + /******************************************************************************* + ** Every backend query while the work runs, by table. + *******************************************************************************/ + private Map backendQueries(UnsafeVoidVoidMethod work) throws Exception { MemoryRecordStore.resetStatistics(); MemoryRecordStore.setCollectStatistics(true); @@ -321,11 +355,7 @@ class TagLookupStatementBudgetTest extends PersonalServiceFixture List inputs = MemoryRecordStore.getActionInputs().get(QueryInput.class); for(AbstractActionInput input : inputs == null ? List.of() : inputs) { - String table = ((QueryInput) input).getTableName(); - if(table.startsWith(CollaborationQBitConfig.TABLE_NAME_PREFIX)) - { - result.merge(table, 1L, Long::sum); - } + result.merge(((QueryInput) input).getTableName(), 1L, Long::sum); } return (result); }