From 774272578900c27258f497fd01da347de49e430f Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Tue, 22 Sep 2026 09:09:49 +0000
Subject: [PATCH] [#930] Build the names a JDBC backend gives its own trees once, from the id it was created with (#1007)
---
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 19 +++
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogNameTestCase.java | 103 ++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 120 ++++++++++++++++++-----
3 files changed, 212 insertions(+), 30 deletions(-)
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 fad5fa3..caacc46 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
@@ -1205,11 +1205,13 @@
* The catalog is per backend and named after the backend id alone: a process that has opened
* nothing can still find its table, and backends sharing one database URL - which nothing
* 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.
+ * OwnNames#escapedBackendId} states: a name that does not survive being read back is a table of
+ * this backend that its own clear cannot recognize.
+ * <p>
+ * Built once and remembered; see {@link OwnNames}.
*/
TreeName getCatalogTree() {
- return new TreeName(CATALOG_BASE_DN, escapedBackendId());
+ return ownNames().catalogTree;
}
/**
@@ -3287,6 +3289,8 @@
leftovers.unreadable.addAll(standing);
return leftovers;
}
+ // normalized once for the whole scan and not per table: see isOwnTree()
+ final Set<String> ownBaseDNs=ownBaseDNs();
for (final String tableName : standing) {
final TreeName stamp;
try {
@@ -3306,7 +3310,7 @@
}
if (stamp==null) {
leftovers.unattributed.add(tableName);
- } else if (isOwnTree(stamp)) {
+ } else if (isOwnTree(stamp, ownBaseDNs)) {
leftovers.ours.add(tableName+" ("+stamp+")");
}
}
@@ -3347,23 +3351,75 @@
* 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.
+ * <p>
+ * Built once and remembered; see {@link OwnNames}.
*/
- private String ownCompressedSchemaBaseDN() {
- return SHARED_COMPRESSED_SCHEMA_BASE_DN+"_"+escapedBackendId();
+ String ownCompressedSchemaBaseDN() {
+ return ownNames().compressedSchemaBaseDN;
}
/**
- * The backend id as one component of a tree name. A tree name is {@code /<base DN>/<id>} and is
- * read back by splitting on its slashes ({@code TreeName.valueOf}), so an id carrying one of them
- * would name a tree that parses into another tree than it was built from - and a table is stamped
- * with that name (#866), so a clear reading the stamp of a table of this backend's own would then
- * fail to recognize it and pass it over in silence. The escape is the one {@code
- * 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.
+ * 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.
+ * <p>
+ * 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.
+ * <p>
+ * 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 String escapedBackendId() {
- return config.getBackendId().replace("%", "%25").replace("/", "%2F");
+ private static final class OwnNames {
+ /**
+ * The backend id as one component of a tree name. A tree name is {@code /<base DN>/<id>} and is
+ * read back by splitting on its slashes ({@code TreeName.valueOf}), so an id carrying one of
+ * them would name a tree that parses into another tree than it was built from - and a table is
+ * stamped with that name (#866), so a clear reading the stamp of a table of this backend's own
+ * would then fail to recognize it and pass it over in silence. The escape is the one {@code
+ * 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.
+ */
+ 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.
+ * <p>
+ * 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;
}
/**
@@ -3374,23 +3430,33 @@
* 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.
+ * <p>
+ * 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<String> 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<String> ownBaseDNs() {
final SortedSet<DN> baseDNs=config.getBaseDN();
if (baseDNs==null) {
- return false;
+ return Collections.emptySet();
}
+ final Set<String> 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 0000000..c34720d
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/CatalogNameTestCase.java
@@ -0,0 +1,103 @@
+/*
+ * 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.
+ * <p>
+ * 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");
+ }
+
+ /**
+ * The second name built from the id, the base DN of this backend's own pair of compressed schema
+ * trees (#881), is pinned for the reason the catalog is: the pair stands under the id the storage
+ * created it with, and a name read again from a configuration that has moved would leave a clear
+ * reporting this backend's own pair as a table attributable to nobody.
+ */
+ @Test
+ public void testTheCompressedSchemaNameKeepsTheBackendIdTheStorageWasBuiltWith() {
+ final JDBCStorage storage = storageFor("pinnedBackend");
+ final String built = storage.ownCompressedSchemaBaseDN();
+ assertEquals(built, JDBCStorage.SHARED_COMPRESSED_SCHEMA_BASE_DN + "_pinnedBackend");
+
+ final JDBCBackendCfg renamed = mockCfg(JDBCBackendCfg.class);
+ when(renamed.getBackendId()).thenReturn("renamedBackend");
+ storage.applyConfigurationChange(renamed);
+
+ assertEquals(storage.ownCompressedSchemaBaseDN(), built,
+ "the compressed schema pair followed a backend id changed under the storage");
+ }
+
+ /**
+ * Both names carry the id escaped, and escaped the way {@code PersistentCompressedSchema} spells
+ * the prefix of the same pair: the percent first, so that the escape of a slash cannot itself be
+ * escaped a second time. A tree name is {@code /<base DN>/<id>} and is read back by splitting on
+ * its slashes, so an id spelled into one unescaped names a tree that parses into another tree
+ * than it was built from - and the clear reading the stamp of such a table would pass a table of
+ * this backend's own over in silence. An id of that shape is possible: {@code backend-id} is a
+ * plain string in the configuration definition, constrained to nothing.
+ */
+ @Test
+ public void testBothNamesEscapeTheSlashAndThePercentOfTheBackendId() {
+ final JDBCStorage storage = storageFor("a/b%c");
+ assertEquals(storage.getCatalogTree(), new TreeName(JDBCStorage.CATALOG_BASE_DN, "a%2Fb%25c"));
+ assertEquals(storage.ownCompressedSchemaBaseDN(),
+ JDBCStorage.SHARED_COMPRESSED_SCHEMA_BASE_DN + "_a%2Fb%25c");
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
index e60e01d..49f26c1 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
@@ -2909,9 +2909,10 @@
/**
* What a clear leaves standing it reports, and it reports it as what it is: a table stamped with a
- * tree of a base DN this backend serves is its own and can be removed by hand, while a table of a
- * backend sharing this database (#873) is that backend's business and no part of this outcome.
- * Told apart by the stamp of #866 and by nothing else - a table name is a bare hash.
+ * tree of a base DN this backend serves, or with one of the trees this backend names after its own
+ * id, is its own and can be removed by hand, while a table of a backend sharing this database
+ * (#873) is that backend's business and no part of this outcome. Told apart by the stamp of #866
+ * and by nothing else - a table name is a bare hash.
*/
@Test
public void testAClearReportsTheTablesItCanAttributeToThisBackend() throws Exception {
@@ -2922,6 +2923,9 @@
final DN neighbourBaseDN = DN.valueOf("dc=clear-report-neighbour,dc=com");
final TreeName neighbourTree = new TreeName(neighbourBaseDN.toNormalizedUrlSafeString(), "id2entry");
final JDBCStorage storage = new JDBCStorage(createBackendCfg(getBackendId() + "_reported", baseDN), null);
+ // one of the pair this backend names after its own id (#881) rather than after a base DN: such
+ // a table is this backend's own by a term of the decision that no base DN of it can answer for
+ final TreeName ownSchema = new TreeName(storage.ownCompressedSchemaBaseDN(), "compressed_attributes");
final JDBCStorage neighbour =
new JDBCStorage(createBackendCfg(getBackendId() + "_reportedNeighbour", neighbourBaseDN), null);
try {
@@ -2930,6 +2934,7 @@
@Override
public void run(WriteableTransaction txn) throws Exception {
txn.openTree(owned, true);
+ txn.openTree(ownSchema, true);
}
});
neighbour.open(AccessMode.READ_WRITE);
@@ -2959,6 +2964,8 @@
"a table of a base DN this backend serves was not reported as its own: " + leftovers.ours);
assertFalse(leftovers.unattributed.toString().toLowerCase().contains(storage.getTableName(owned).toLowerCase()),
"a table this backend can name was reported as attributable to nobody: " + leftovers.unattributed);
+ assertTrue(leftovers.ours.toString().toLowerCase().contains(storage.getTableName(ownSchema).toLowerCase()),
+ "a table of this backend's own compressed schema pair was not reported as its own: " + leftovers.ours);
assertTrue(leftovers.unreadable.isEmpty(),
"the stamp of a table this database does give up was reported as unreadable: " + leftovers.unreadable);
}
@@ -2969,6 +2976,7 @@
// purpose is dropped here by hand, as the report says such a table has to be
clearQuietly(storage);
dropTableIfExists(storage.getTableName(owned));
+ dropTableIfExists(storage.getTableName(ownSchema));
}
}
@@ -2995,6 +3003,11 @@
assertNotNull(theirs, "the database would not say which tables the neighbour is holding");
assertTrue(theirs.ours.toString().toLowerCase().contains(other.getTableName(otherTree).toLowerCase()),
"the table left unreported is one the scan does not reach at all: " + theirs.ours);
+ // and the catalog standing beside it is that backend's own: the clear of the case above
+ // dropped its own catalog by hand, so this is where a catalog is there to be reported
+ assertTrue(theirs.ours.toString().toLowerCase()
+ .contains(other.getTableName(other.getCatalogTree()).toLowerCase()),
+ "a backend's own standing catalog was not reported as its own: " + theirs.ours);
}
}
--
Gitblit v1.10.0