From 523cb45b416a6a28a8a5c00d0db778fa75da6524 Mon Sep 17 00:00:00 2001 From: WingedGuardian Date: Sun, 8 Nov 2026 00:39:40 -0500 Subject: [PATCH] ALTER DB: never damage an existing target database (#2800) An existing target is now checked for being pinned like the source, and is renamed aside before the source is moved in; it is deleted only after the move succeeded, and restored if the move fails. Co-Authored-By: Claude Opus 5.5 --- .../main/java/org/basex/core/cmd/AlterDB.java | 34 +++++- .../test/java/org/basex/core/AlterDBTest.java | 102 ++++++++++++++++++ 2 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 basex-core/src/test/java/org/basex/core/AlterDBTest.java diff --git a/basex-core/src/main/java/org/basex/core/cmd/AlterDB.java b/basex-core/src/main/java/org/basex/core/cmd/AlterDB.java index 8ecd1c0cca..c26ed69e1b 100644 --- a/basex-core/src/main/java/org/basex/core/cmd/AlterDB.java +++ b/basex-core/src/main/java/org/basex/core/cmd/AlterDB.java @@ -48,6 +48,9 @@ protected boolean run() { if(!closed) closed = close(context, src); // check if source database is still opened if(context.pinned(src)) return error(DB_PINNED_X, src); + // an existing target database will be replaced: close it, check if it is still opened + close(context, trg); + if(context.pinned(trg)) return error(DB_PINNED_X, trg); // try to alter database return alter(src, trg, soptions) && (!closed || new Open(trg).run(context)) ? @@ -60,7 +63,8 @@ public void addLocks() { } /** - * Renames the specified database. + * Renames the specified database. An existing target database is replaced. + * If renaming fails, both databases are left unchanged. * @param source name of the existing database * @param target new database name * @param sopts static options @@ -69,16 +73,38 @@ public void addLocks() { public static synchronized boolean alter(final String source, final String target, final StaticOptions sopts) { - // drop target database (unless it is the source, e.g. if only the case differs) final IOFile src = sopts.dbPath(source), trg = sopts.dbPath(target); - if(!src.sameFile(trg)) DropDB.drop(target, sopts); + // move an existing target database aside (unless it is the source, e.g. if only the case + // differs). On Windows, this fails if a file of the target is opened by another process. + IOFile old = null; + if(trg.exists() && !src.sameFile(trg)) { + for(int c = 0; old == null || old.exists(); c++) { + old = new IOFile(sopts.dbPath(), '.' + target + '.' + c); + } + try { + trg.moveTo(old, false); + } catch(final IOException ex) { + Util.debug(ex); + return false; + } + } try { src.moveTo(trg, false); - return true; } catch(final IOException ex) { Util.debug(ex); + // restore the target database + if(old != null) { + try { + old.moveTo(trg, false); + } catch(final IOException ex2) { + Util.debug(ex2); + } + } return false; } + // delete the replaced database + if(old != null && !old.delete()) Util.debugln("Could not delete %", old); + return true; } @Override diff --git a/basex-core/src/test/java/org/basex/core/AlterDBTest.java b/basex-core/src/test/java/org/basex/core/AlterDBTest.java new file mode 100644 index 0000000000..acc5da48d8 --- /dev/null +++ b/basex-core/src/test/java/org/basex/core/AlterDBTest.java @@ -0,0 +1,102 @@ +package org.basex.core; + +import static org.basex.query.QueryError.*; +import static org.basex.query.func.Function.*; +import static org.junit.jupiter.api.Assertions.*; + +import java.io.*; + +import org.basex.*; +import org.basex.api.client.*; +import org.basex.core.cmd.*; +import org.basex.data.*; +import org.basex.io.*; +import org.basex.util.*; +import org.junit.jupiter.api.*; +import org.junit.jupiter.api.Test; + +/** + * Tests for renaming a database to the name of an existing database. + * + * @author BaseX Team, BSD License + * @author WingedGuardian + */ +public final class AlterDBTest extends SandboxTest { + /** Name of the target database. */ + private static final String NAME2 = NAME + '2'; + + /** Creates the source and the target database. */ + @BeforeEach public void init() { + execute(new CreateDB(NAME, "")); + execute(new CreateDB(NAME2, "")); + execute(new Close()); + } + + /** Drops the test databases. */ + @AfterEach public void finish() { + execute(new DropDB(NAME)); + execute(new DropDB(NAME2)); + } + + /** An existing target database is replaced, and no temporary files are left behind. */ + @Test public void replace() { + execute(new AlterDB(NAME, NAME2)); + assertFalse(context.soptions.dbExists(NAME)); + query(_DB_GET.args(NAME2) + "/*/name()", "a"); + for(final IOFile file : context.soptions.dbPath().children()) { + assertFalse(file.name().startsWith('.' + NAME2), file.name()); + } + } + + /** + * A target database that is opened by another client is not replaced. + * @throws IOException I/O exception + */ + @Test public void targetOpened() throws IOException { + try(LocalSession session = new LocalSession(context)) { + session.execute(new Open(NAME2)); + final BaseXException ex = assertThrows(BaseXException.class, + () -> new AlterDB(NAME, NAME2).execute(context)); + assertTrue(ex.getMessage().contains(NAME2), ex.getMessage()); + error(_DB_ALTER.args(NAME, NAME2), DB_LOCK1_X); + } + query(_DB_GET.args(NAME) + "/*/name()", "a"); + query(_DB_GET.args(NAME2) + "/*/name()", "b"); + } + + /** + * A target database with a file that is opened by another process is left intact. + * Windows only: on other systems, open files do not prevent renaming or deleting a directory. + * @throws IOException I/O exception + */ + @Test public void targetFileOpened() throws IOException { + Assumptions.assumeTrue(Prop.WIN, "Windows only"); + final IOFile file = new IOFile(context.soptions.dbPath(NAME2), + DataText.DATAINF + IO.BASEXSUFFIX); + try(RandomAccessFile raf = new RandomAccessFile(file.file(), "r")) { + assertThrows(BaseXException.class, () -> new AlterDB(NAME, NAME2).execute(context)); + error(_DB_ALTER.args(NAME, NAME2), UPDBERROR_X_X); + } + query(_DB_GET.args(NAME) + "/*/name()", "a"); + query(_DB_GET.args(NAME2) + "/*/name()", "b"); + // the operation succeeds after the file has been closed + execute(new AlterDB(NAME, NAME2)); + query(_DB_GET.args(NAME2) + "/*/name()", "a"); + } + + /** + * A target database is restored if the source database cannot be renamed. + * Windows only: on other systems, open files do not prevent renaming a directory. + * @throws IOException I/O exception + */ + @Test public void sourceFileOpened() throws IOException { + Assumptions.assumeTrue(Prop.WIN, "Windows only"); + final IOFile file = new IOFile(context.soptions.dbPath(NAME), + DataText.DATAINF + IO.BASEXSUFFIX); + try(RandomAccessFile raf = new RandomAccessFile(file.file(), "r")) { + assertThrows(BaseXException.class, () -> new AlterDB(NAME, NAME2).execute(context)); + } + query(_DB_GET.args(NAME) + "/*/name()", "a"); + query(_DB_GET.args(NAME2) + "/*/name()", "b"); + } +}