diff --git a/jstests/libs/namespace_utils.js b/jstests/libs/namespace_utils.js index 93b53d30be4..d04d93ee235 100644 --- a/jstests/libs/namespace_utils.js +++ b/jstests/libs/namespace_utils.js @@ -11,3 +11,19 @@ export function getCollectionNameFromFullNamespace(ns) { export function getDBNameAndCollNameFromFullNamespace(ns) { return ns.split(/\.(.+)/); } + +/** + * Returns whether the collection name is valid. See NamespaceString::validCollectionName(). + */ +export function isValidCollectionName(collName) { + if (typeof collName !== "string" || collName.length === 0) { + return false; + } + if (collName.includes("\0")) { + return false; + } + if (collName === "oplog.$main") { + return true; + } + return !collName.startsWith(".") && !collName.includes("$"); +} diff --git a/jstests/libs/override_methods/collection_write_path/set_recordids_replicated.js b/jstests/libs/override_methods/collection_write_path/set_recordids_replicated.js index 611f20709b2..1921b26aa64 100644 --- a/jstests/libs/override_methods/collection_write_path/set_recordids_replicated.js +++ b/jstests/libs/override_methods/collection_write_path/set_recordids_replicated.js @@ -12,9 +12,11 @@ */ import {OverrideHelpers} from "jstests/libs/override_methods/override_helpers.js"; +import {isValidCollectionName} from "jstests/libs/namespace_utils.js"; const commandsToOverride = new Set(["create", "insert", "update", "createIndexes"]); -const createdCollections = new Set(); +// The set of collections already seen by this override and thus ignored. +const collectionsKnownToExist = new Set(); function hasError(res) { return res.ok !== 1 || res.writeErrors || (res.hasOwnProperty("nErrors") && res.nErrors != 0); @@ -24,17 +26,22 @@ function runCommandWithRecordIdsReplicated(conn, dbName, commandName, commandObj const collName = commandObj[commandName]; const ns = dbName + "." + collName; if (commandName === "drop") { - createdCollections.delete(ns); + collectionsKnownToExist.delete(ns); return func.apply(conn, makeFuncArgs(commandObj)); } if ( !commandsToOverride.has(commandName) || - createdCollections.has(ns) || + collectionsKnownToExist.has(ns) || typeof commandObj !== "object" || commandObj === null ) { return func.apply(conn, makeFuncArgs(commandObj)); } + if (!isValidCollectionName(collName)) { + // Avoid issuing listCollections for invalid namespaces (e.g., embedded nulls) so we don't + // mask the command's own InvalidNamespace error with our override logic. + return func.apply(conn, makeFuncArgs(commandObj)); + } if (commandName === "create") { if (commandObj.hasOwnProperty("clusteredIndex")) { return func.apply(conn, makeFuncArgs(commandObj)); @@ -51,7 +58,7 @@ function runCommandWithRecordIdsReplicated(conn, dbName, commandName, commandObj jsTestLog("create error: " + tojsononeline(res)); } if (!hasError(res) || res.code === ErrorCodes.NamespaceExists) { - createdCollections.add(ns); + collectionsKnownToExist.add(ns); } return res; } else { @@ -59,7 +66,7 @@ function runCommandWithRecordIdsReplicated(conn, dbName, commandName, commandObj const clustered = "clustered"; const clusteredTrue = commandObj["indexes"].some((obj) => clustered in obj && obj.unique === true); if (clusteredTrue) { - createdCollections.add(ns); + collectionsKnownToExist.add(ns); return func.apply(conn, makeFuncArgs(commandObj)); } } @@ -70,20 +77,48 @@ function runCommandWithRecordIdsReplicated(conn, dbName, commandName, commandObj return func.apply(conn, makeFuncArgs(commandObj)); } } + // If the collection already existed, don't clean up even if the inner command fails. + const collExists = conn.getDB(dbName).getCollectionInfos({name: collName}).length > 0; + if (collExists) { + collectionsKnownToExist.add(ns); + return func.apply(conn, makeFuncArgs(commandObj)); + } const createObj = {create: collName, recordIdsReplicated: true}; ["lsid", "$clusterTime", "writeConcern", "collectionUUID"].forEach((option) => { if (commandObj.hasOwnProperty(option)) { createObj[option] = commandObj[option]; } }); - const res = func.apply(conn, makeFuncArgs(createObj)); - if (hasError(res)) { - jsTestLog("create error: " + tojsononeline(res)); + + const createRes = func.apply(conn, makeFuncArgs(createObj)); + let createdByOverride = false; + if (hasError(createRes)) { + jsTest.log.info( + "Error while creating collection for set_recordids_replicated.js override: " + tojsononeline(createRes), + ); + } else { + createdByOverride = true; } - if (!hasError(res) || res.code === ErrorCodes.NamespaceExists) { - createdCollections.add(ns); + if (!hasError(createRes) || createRes.code === ErrorCodes.NamespaceExists) { + collectionsKnownToExist.add(ns); } - return func.apply(conn, makeFuncArgs(commandObj)); + + const wrappedCmdRes = func.apply(conn, makeFuncArgs(commandObj)); + if (createdByOverride && hasError(wrappedCmdRes) && commandName === "createIndexes") { + jsTest.log.info( + "Cleaning up collection created for set_recordids_replicated.js override after createIndexes error: " + + tojsononeline(wrappedCmdRes), + ); + const dropRes = func.apply(conn, makeFuncArgs({drop: collName})); + if (hasError(dropRes)) { + jsTest.log.info( + "Error while cleaning up collection for set_recordids_replicated.js override: " + + tojsononeline(dropRes), + ); + } + collectionsKnownToExist.delete(ns); + } + return wrappedCmdRes; } } diff --git a/jstests/noPassthrough/index_builds/index_create_illegal_options.js b/jstests/noPassthrough/index_builds/index_create_illegal_options.js index ae393ede78c..a236ef079af 100644 --- a/jstests/noPassthrough/index_builds/index_create_illegal_options.js +++ b/jstests/noPassthrough/index_builds/index_create_illegal_options.js @@ -2,7 +2,6 @@ * Ensures that a createIndexes command request fails when creating an index with illegal options. */ -import {FeatureFlagUtil} from "jstests/libs/feature_flag_util.js"; import {after, afterEach, before, describe, it} from "jstests/libs/mochalite.js"; describe("Specifying index type in the createIndex command", function () { @@ -37,14 +36,11 @@ describe("Specifying index type in the createIndex command", function () { this.db[this.testCollName].createIndex({"foo": indexType}), expectedErrorCodes, ); - // TODO(SERVER-114308): Primary-driven index builds eagerly create the collection, this assertion will fail on those build variants. - if (!FeatureFlagUtil.isPresentAndEnabled(this.db, "PrimaryDrivenIndexBuilds")) { - assert.doesNotContain( - this.db.getCollectionNames(), - [this.testCollName], - `The ${this.testCollName} collection should not be implicitly created upon failing to create the index.`, - ); - } + assert.doesNotContain( + this.db.getCollectionNames(), + [this.testCollName], + `The ${this.testCollName} collection should not be implicitly created upon failing to create the index.`, + ); }); });