diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java index 328f7dd824..ccbda5d471 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java @@ -1106,9 +1106,11 @@ String getTableName(TreeName treeName) { * forbids (#873) - never name each other's trees. The id goes in escaped, for the reason {@link * #escapedBackendId} states: a name that does not survive being read back is a table of this * backend that its own clear cannot recognize. + *

+ * Built once and remembered; see {@link OwnNames}. */ TreeName getCatalogTree() { - return new TreeName(CATALOG_BASE_DN, escapedBackendId()); + return ownNames().catalogTree; } /** @@ -2536,6 +2538,8 @@ ClearLeftovers leftoverTables(Connection con, TableScope scope) { leftovers.unreadable.addAll(standing); return leftovers; } + // normalized once for the whole scan and not per table: see isOwnTree() + final Set ownBaseDNs=ownBaseDNs(); for (final String tableName : standing) { final TreeName stamp; try { @@ -2555,7 +2559,7 @@ ClearLeftovers leftoverTables(Connection con, TableScope scope) { } if (stamp==null) { leftovers.unattributed.add(tableName); - } else if (isOwnTree(stamp)) { + } else if (isOwnTree(stamp, ownBaseDNs)) { leftovers.ours.add(tableName+" ("+stamp+")"); } } @@ -2596,9 +2600,11 @@ private TreeName stampedTree(Connection con, Dialect dialect, String tableName) * here for the reason {@link #SHARED_COMPRESSED_SCHEMA_TREES} is: the prefix is built by a private * method of {@code PersistentCompressedSchema}, escapes and all. A table stamped with one of these * carries this backend's id in plain text, so a clear that finds one standing can say whose it is. + *

+ * Built once and remembered; see {@link OwnNames}. */ private String ownCompressedSchemaBaseDN() { - return SHARED_COMPRESSED_SCHEMA_BASE_DN+"_"+escapedBackendId(); + return ownNames().compressedSchemaBaseDN; } /** @@ -2610,9 +2616,65 @@ private String ownCompressedSchemaBaseDN() { * PersistentCompressedSchema} spells its own prefix with, percent first so that the escape of the * slash cannot be produced twice, and it leaves an id of the ordinary shape exactly as it is - * which is what keeps the table names of an installation unchanged. + *

+ * Escaped once and remembered; see {@link OwnNames}. */ private String escapedBackendId() { - return config.getBackendId().replace("%", "%25").replace("/", "%2F"); + return ownNames().escapedBackendId; + } + + /** + * The names this backend gives the trees that are its own rather than a base DN's: the escaped + * id of {@link #escapedBackendId}, the catalog tree of {@link #getCatalogTree} and the base DN of + * {@link #ownCompressedSchemaBaseDN}. Held in one object so that the three can never come from + * two different ids. + *

+ * Read from the configuration once rather than per call, for the reason {@link + * #poolConnectionString} is read once: {@code applyConfigurationChange()} replaces {@code config} + * whole, while the tables of this storage - the catalog table among them - stand under the id + * they were created with, and a name read again from a configuration that has moved would leave + * this storage naming a catalog nothing has ever written and its own tables attributed to + * nobody. The configuration framework holds backend-id read-only - "The backend ID may not be + * altered after the backend is created in the server" - so no change made through it renames a + * live backend, which is what makes reading the id once correct in the first place. + *

+ * That it also takes the escape and the allocation off every enrolment, off every clear and off + * every table of a clear's leftover scan (#930) is the smaller half of it: every one of those + * call sites is already paying a round trip to the database. + */ + private static final class OwnNames { + final String escapedBackendId; + final TreeName catalogTree; + final String compressedSchemaBaseDN; + + OwnNames(String backendId) { + escapedBackendId=backendId.replace("%", "%25").replace("/", "%2F"); + catalogTree=new TreeName(CATALOG_BASE_DN, escapedBackendId); + compressedSchemaBaseDN=SHARED_COMPRESSED_SCHEMA_BASE_DN+"_"+escapedBackendId; + } + } + + private volatile OwnNames ownNames; + + /** + * The names above, built at the first call that needs one, and not in the constructor: a storage + * is constructed by callers that go on to name no tree of it at all - the bounds of a statement + * are asked of one whose configuration carries no backend id whatsoever in the tests of {@code + * JDBCStatementBoundTestCase} - and a construction reading the id would fail there, where today + * nothing reads it. + *

+ * Two callers arriving at once may each build one, and the names of both are the same names. + * Every field of {@link OwnNames} is final, so a caller reading the reference reads the names + * whole and not half-built. + */ + private OwnNames ownNames() { + final OwnNames built=ownNames; + if (built!=null) { + return built; + } + final OwnNames names=new OwnNames(config.getBackendId()); + ownNames=names; + return names; } /** @@ -2623,23 +2685,33 @@ private String escapedBackendId() { * backend id (#873) and so belongs to this backend as plainly as any tree of a base DN it serves - * where the legacy pair, named from a literal, belongs to no backend in particular and is reported * by nobody. + *

+ * The base DNs are handed in rather than read here: they are the same for every table of one + * scan, and normalizing a DN builds its string from every RDN of it ({@code + * DN.toNormalizedUrlSafeString} memoizes nothing), which is a cost the caller pays once instead + * of once per table. */ - private boolean isOwnTree(TreeName treeName) { - if (getCatalogTree().equals(treeName) || ownCompressedSchemaBaseDN().equals(treeName.getBaseDN())) { - return true; - } + private boolean isOwnTree(TreeName treeName, Set ownBaseDNs) { + return getCatalogTree().equals(treeName) + || ownCompressedSchemaBaseDN().equals(treeName.getBaseDN()) + || ownBaseDNs.contains(treeName.getBaseDN()); + } + + /** + * The base DNs this backend serves, in the form the trees of an entry container are named after: + * every one of them is named from the normalized base DN, which is what {@code EntryContainer} + * builds its tree names from. + */ + private Set ownBaseDNs() { final SortedSet baseDNs=config.getBaseDN(); if (baseDNs==null) { - return false; + return Collections.emptySet(); } + final Set normalized=new HashSet<>(); for (final DN baseDN : baseDNs) { - // every tree of an entry container is named after the normalized form of its base DN, - // which is what EntryContainer builds its tree names from - if (treeName.getBaseDN().equals(baseDN.toNormalizedUrlSafeString())) { - return true; - } + normalized.add(baseDN.toNormalizedUrlSafeString()); } - return false; + return normalized; } /** diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogNameTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogNameTestCase.java new file mode 100644 index 0000000000..0ba1fe8b25 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogNameTestCase.java @@ -0,0 +1,66 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.backends.jdbc; + +import org.forgerock.opendj.server.config.server.JDBCBackendCfg; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.backends.pluggable.spi.TreeName; +import org.testng.annotations.Test; + +import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; + +/** + * The names a JDBC backend gives the trees that are its own rather than a base DN's: its catalog + * (#888) and its pair of compressed schema trees (#881). Both are built from the backend id, and + * the tables behind them are created under the id the storage was built with - so what this class + * pins is that the names do not move under a storage that has already created them. + *

+ * No database is needed: the names are read from the configuration and nothing else. + */ +@SuppressWarnings("javadoc") +public class CatalogNameTestCase extends DirectoryServerTestCase { + + private static JDBCStorage storageFor(String backendId) { + final JDBCBackendCfg cfg = mockCfg(JDBCBackendCfg.class); + when(cfg.getBackendId()).thenReturn(backendId); + return new JDBCStorage(cfg, null); + } + + /** + * The catalog is named after the backend id, and after the one this storage was built with: a + * configuration handed to {@code applyConfigurationChange()} replaces {@code config} whole, so a + * name read from it again would follow an id changed under a running backend - and the tables of + * this storage, the catalog table among them, stand under the id they were created with. The + * backend id is read-only in the configuration framework and no such change can be made through + * it, which is exactly why the storage may read it once; a rename reaching this method by any + * other route must not leave the storage naming a catalog nothing has ever written. + */ + @Test + public void testTheCatalogKeepsTheBackendIdTheStorageWasBuiltWith() { + final JDBCStorage storage = storageFor("pinnedBackend"); + final TreeName built = storage.getCatalogTree(); + assertEquals(built, new TreeName(JDBCStorage.CATALOG_BASE_DN, "pinnedBackend")); + + final JDBCBackendCfg renamed = mockCfg(JDBCBackendCfg.class); + when(renamed.getBackendId()).thenReturn("renamedBackend"); + storage.applyConfigurationChange(renamed); + + assertEquals(storage.getCatalogTree(), built, + "the catalog followed a backend id changed under the storage, naming a table nothing created"); + } +}