Skip to content

Commit 906d2d6

Browse files
adamretteradamretter
committed
[bugfix] Make sure to delete fs folders when removing Collections
1 parent d8456ca commit 906d2d6

File tree

2 files changed

+165
-2
lines changed

2 files changed

+165
-2
lines changed

‎src/org/exist/storage/NativeBroker.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1827,6 +1827,16 @@ private boolean _removeCollection(final Txn transaction, @EnsureLocked(mode=Lock
18271827
// 7) remove the documents nodes and binary documents of the Collection from dom.dbx
18281828
removeCollectionsDocumentNodes(transaction, collection);
18291829

1830+
// if there was a fs folder for holding binaries for the collection remove the (now empty folder) from disk
1831+
final Path fsBinCollection = getCollectionBinaryFileFsPath(collectionUri);
1832+
if (Files.exists(fsBinCollection)) {
1833+
if (!FileUtils.list(fsBinCollection).isEmpty()) {
1834+
LOG.error("Unable to remove non-empty fs folder for Collection binaries: " + fsBinCollection.toAbsolutePath().toString());
1835+
} else {
1836+
FileUtils.delete(fsBinCollection);
1837+
}
1838+
}
1839+
18301840
colTrigger.afterDeleteCollection(this, transaction, collectionUri);
18311841

18321842
return true;
@@ -1885,13 +1895,13 @@ public Object start() {
18851895
}
18861896
}.run();
18871897

1888-
docTrigger.afterDeleteDocument(this, transaction, doc.getURI());
1889-
18901898
// if it is a binary document remove the content from disk
18911899
if (doc instanceof BinaryDocument) {
18921900
removeCollectionBinary(transaction, (BinaryDocument)doc, streamableDigest);
18931901
}
18941902

1903+
docTrigger.afterDeleteDocument(this, transaction, doc.getURI());
1904+
18951905
//Make doc's id available again
18961906
collectionsDb.freeResourceId(doc.getDocId());
18971907
}
@@ -2412,6 +2422,10 @@ private void storeBinaryResource(final Txn transaction,
24122422
final Path binFile = getCollectionFile(getFsDir(), blob.getURI(), true);
24132423
final boolean exists = Files.exists(binFile);
24142424

2425+
if (exists && Files.isDirectory(binFile)) {
2426+
throw new IOException("Cannot overwrite binary fs Collection '" + blob.getURI().getRawCollectionPath() + "' with Document: " + blob.getURI().lastSegment().toString());
2427+
}
2428+
24152429
if(fsJournalDir.isPresent()) {
24162430
final StreamableDigest streamableDigest = BINARY_RESOURCE_DIGEST_TYPE.newStreamableDigest();
24172431

‎test/src/org/exist/storage/BinaryDocumentTest.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
* eXist Open Source Native XML Database
3+
* Copyright (C) 2001-2018 The eXist Project
4+
* http://exist-db.org
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public License
8+
* as published by the Free Software Foundation; either version 2
9+
* of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public
17+
* License along with this library; if not, write to the Free Software
18+
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
19+
*/
20+
21+
package org.exist.storage;
22+
23+
import org.exist.EXistException;
24+
import org.exist.collections.Collection;
25+
import org.exist.collections.triggers.TriggerException;
26+
import org.exist.security.PermissionDeniedException;
27+
import org.exist.storage.txn.Txn;
28+
import org.exist.test.ExistEmbeddedServer;
29+
import org.exist.util.LockException;
30+
import org.exist.util.io.FastByteArrayInputStream;
31+
import org.exist.xmldb.XmldbURI;
32+
import org.junit.ClassRule;
33+
import org.junit.Test;
34+
35+
import java.io.IOException;
36+
import java.io.InputStream;
37+
import java.nio.file.Files;
38+
import java.nio.file.Path;
39+
import java.util.Optional;
40+
41+
import static java.nio.charset.StandardCharsets.UTF_8;
42+
import static org.junit.Assert.assertEquals;
43+
import static org.junit.Assert.assertTrue;
44+
import static org.junit.Assert.fail;
45+
46+
public class BinaryDocumentTest {
47+
48+
@ClassRule
49+
public static final ExistEmbeddedServer existEmbeddedServer = new ExistEmbeddedServer(true, true);
50+
51+
@Test
52+
public void removeCollection() throws PermissionDeniedException, IOException, TriggerException, LockException, EXistException {
53+
final XmldbURI testCollectionUri = XmldbURI.create("/db/remove-collection-test");
54+
final XmldbURI thingUri = testCollectionUri.append("thing");
55+
56+
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
57+
try (final DBBroker broker = pool.get(Optional.of(pool.getSecurityManager().getSystemSubject()));
58+
final Txn transaction = pool.getTransactionManager().beginTransaction()) {
59+
60+
// create a collection
61+
final Collection thingCollection = broker.getOrCreateCollection(transaction, thingUri);
62+
broker.saveCollection(transaction, thingCollection);
63+
64+
// add a binary document to the collection
65+
final byte[] binaryData1 = "binary-file1".getBytes(UTF_8);
66+
try (final InputStream is = new FastByteArrayInputStream(binaryData1)) {
67+
thingCollection.addBinaryResource(transaction, broker, XmldbURI.create("file1.bin"), is, "application/octet-stream", binaryData1.length);
68+
}
69+
70+
// remove the collection
71+
assertTrue(broker.removeCollection(transaction, thingCollection));
72+
73+
// try and store a binary doc with the same name as the thing collection (should succeed)
74+
final Collection testCollection = broker.getCollection(testCollectionUri);
75+
final byte[] binaryData2 = "binary-file2".getBytes(UTF_8);
76+
try (final InputStream is = new FastByteArrayInputStream(binaryData2)) {
77+
testCollection.addBinaryResource(transaction, broker, XmldbURI.create("thing"), is, "application/octet-stream", binaryData2.length);
78+
}
79+
}
80+
}
81+
82+
@Test
83+
public void overwriteCollectionFsDir() throws EXistException, PermissionDeniedException, IOException, TriggerException, LockException {
84+
final XmldbURI testCollectionUri = XmldbURI.create("/db/overwrite-collection-fs-dir-test");
85+
final XmldbURI thingUri = testCollectionUri.append("thing");
86+
87+
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
88+
try (final DBBroker broker = pool.get(Optional.of(pool.getSecurityManager().getSystemSubject()));
89+
final Txn transaction = pool.getTransactionManager().beginTransaction()) {
90+
91+
// create a folder in the storage which will represent an orphaned collection folder
92+
final Collection testCollection = broker.getOrCreateCollection(transaction, testCollectionUri);
93+
broker.saveCollection(transaction, testCollection);
94+
final Path fsThingCollection = existEmbeddedServer.getTemporaryStorage().get()
95+
.resolve("fs")
96+
.resolve(thingUri.getRawCollectionPath().substring(1));
97+
Files.createDirectories(fsThingCollection);
98+
99+
// attempt to create a binary document with the same uri as the orphaned thingCollection (should fail)
100+
final byte[] binaryData = "binary-file".getBytes(UTF_8);
101+
try (final InputStream is = new FastByteArrayInputStream(binaryData)) {
102+
103+
try {
104+
testCollection.addBinaryResource(transaction, broker, thingUri.lastSegment(), is, "application/octet-stream", binaryData.length);
105+
fail("Should not have been able to overwrite Collection with Binary Document");
106+
107+
} catch (final IOException e) {
108+
assertEquals(
109+
"Cannot overwrite binary fs Collection '" + thingUri.getRawCollectionPath() + "' with Document: " + thingUri.lastSegment().toString(),
110+
e.getMessage()
111+
);
112+
}
113+
}
114+
}
115+
}
116+
117+
@Test
118+
public void overwriteCollection() throws EXistException, PermissionDeniedException, IOException, TriggerException, LockException {
119+
final XmldbURI testCollectionUri = XmldbURI.create("/db/overwrite-collection-test");
120+
final XmldbURI thingUri = testCollectionUri.append("thing");
121+
122+
final BrokerPool pool = existEmbeddedServer.getBrokerPool();
123+
try (final DBBroker broker = pool.get(Optional.of(pool.getSecurityManager().getSystemSubject()));
124+
final Txn transaction = pool.getTransactionManager().beginTransaction()) {
125+
126+
// create a collection
127+
final Collection thingCollection = broker.getOrCreateCollection(transaction, thingUri);
128+
broker.saveCollection(transaction, thingCollection);
129+
130+
// attempt to create a binary document with the same uri as the thingCollection (should fail)
131+
final Collection testCollection = broker.getCollection(testCollectionUri);
132+
133+
final byte[] binaryData = "binary-file".getBytes(UTF_8);
134+
try (final InputStream is = new FastByteArrayInputStream(binaryData)) {
135+
136+
try {
137+
testCollection.addBinaryResource(transaction, broker, thingUri.lastSegment(), is, "application/octet-stream", binaryData.length);
138+
fail("Should not have been able to overwrite Collection with Binary Document");
139+
140+
} catch (final EXistException e) {
141+
assertEquals(
142+
"The collection '" + testCollectionUri.getRawCollectionPath() + "' already has a sub-collection named '" + thingUri.lastSegment().toString() + "', you cannot create a Document with the same name as an existing collection.",
143+
e.getMessage()
144+
);
145+
}
146+
}
147+
}
148+
}
149+
}

0 commit comments

Comments
 (0)