Skip to content

Commit f6a91d4

Browse files
committed
GH-5123 catch all throwables and revert changes
1 parent 954d5b2 commit f6a91d4

4 files changed

Lines changed: 139 additions & 11 deletions

File tree

core/sail/base/src/main/java/org/eclipse/rdf4j/sail/base/SailSourceBranch.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.eclipse.rdf4j.model.Model;
2828
import org.eclipse.rdf4j.model.ModelFactory;
2929
import org.eclipse.rdf4j.model.Resource;
30+
import org.eclipse.rdf4j.model.Statement;
3031
import org.eclipse.rdf4j.model.Value;
3132
import org.eclipse.rdf4j.model.impl.DynamicModelFactory;
3233
import org.eclipse.rdf4j.sail.SailException;
@@ -70,7 +71,7 @@ class SailSourceBranch implements SailSource {
7071

7172
/**
7273
* The {@link Model} instances that should be used to store {@link SailSink#approve(Resource, IRI, Value, Resource)}
73-
* and {@link SailSink#deprecate(Resource, IRI, Value, Resource)} statements.
74+
* and {@link SailSink#deprecate(Statement)} statements.
7475
*/
7576
private final ModelFactory modelFactory;
7677

@@ -306,7 +307,7 @@ public void flush() throws SailException {
306307
prepared = null;
307308
}
308309
}
309-
} catch (SailException e) {
310+
} catch (Throwable e) {
310311
// clear changes if flush fails
311312
changes.clear();
312313
prepared = null;

core/sail/nativerdf/src/main/java/org/eclipse/rdf4j/sail/nativerdf/NativeSailStore.java

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.eclipse.rdf4j.model.Value;
3838
import org.eclipse.rdf4j.model.ValueFactory;
3939
import org.eclipse.rdf4j.query.algebra.evaluation.impl.EvaluationStatistics;
40+
import org.eclipse.rdf4j.sail.Sail;
4041
import org.eclipse.rdf4j.sail.SailException;
4142
import org.eclipse.rdf4j.sail.base.BackingSailSource;
4243
import org.eclipse.rdf4j.sail.base.Changeset;
@@ -389,7 +390,11 @@ public synchronized void flush() throws SailException {
389390
throw new SailException(e);
390391
} catch (RuntimeException e) {
391392
logger.error("Encountered an unexpected problem while trying to commit", e);
392-
throw e;
393+
if (e instanceof SailException) {
394+
throw e;
395+
}
396+
// Ensure upstream handles this as a SailException so branch flush clears pending changes
397+
throw new SailException(e);
393398
} finally {
394399
sinkStoreAccessLock.unlock();
395400
}
@@ -447,7 +452,10 @@ public void clear(Resource... contexts) throws SailException {
447452
public void approve(Resource subj, IRI pred, Value obj, Resource ctx) throws SailException {
448453
try {
449454
addStatement(subj, pred, obj, explicit, ctx);
450-
} catch (IllegalArgumentException e) {
455+
} catch (RuntimeException e) {
456+
if (e instanceof SailException) {
457+
throw e;
458+
}
451459
// Ensure upstream handles this as a SailException so branch flush clears pending changes
452460
throw new SailException(e);
453461
}
@@ -482,8 +490,10 @@ public void approveAll(Set<Statement> approved, Set<Resource> approvedContexts)
482490
}
483491
} catch (IOException e) {
484492
throw new SailException(e);
485-
} catch (IllegalArgumentException e) {
486-
// Wrap invalid value types (e.g., RDF* Triple value) so upstream can rollback/clear changes
493+
} catch (RuntimeException e) {
494+
if (e instanceof SailException) {
495+
throw e;
496+
}
487497
logger.error("Encountered an unexpected problem while trying to add a statement", e);
488498
throw new SailException(e);
489499
} finally {
@@ -545,8 +555,10 @@ private boolean addStatement(Resource subj, IRI pred, Value obj, boolean explici
545555
}
546556
} catch (IOException e) {
547557
throw new SailException(e);
548-
} catch (IllegalArgumentException e) {
549-
// Wrap invalid value types (e.g., RDF* Triple value) so upstream can rollback/clear changes
558+
} catch (RuntimeException e) {
559+
if (e instanceof SailException) {
560+
throw e;
561+
}
550562
logger.error("Encountered an unexpected problem while trying to add a statement", e);
551563
throw new SailException(e);
552564
} finally {
@@ -621,7 +633,11 @@ private long removeStatements(Resource subj, IRI pred, Value obj, boolean explic
621633
throw new SailException(e);
622634
} catch (RuntimeException e) {
623635
logger.error("Encountered an unexpected problem while trying to remove statements", e);
624-
throw e;
636+
if (e instanceof SailException) {
637+
throw e;
638+
}
639+
// Ensure upstream handles this as a SailException so branch flush clears pending changes
640+
throw new SailException(e);
625641
} finally {
626642
sinkStoreAccessLock.unlock();
627643
}

core/sail/nativerdf/src/main/java/org/eclipse/rdf4j/sail/nativerdf/NativeStoreConnection.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@ protected void rollbackInternal() throws SailException {
104104

105105
@Override
106106
protected void addStatementInternal(Resource subj, IRI pred, Value obj, Resource... contexts) throws SailException {
107-
// mark that statements were added so reads can flush pending updates pre-emptively
108-
setStatementsAdded();
109107
// assume the triple is not yet present in the triple store
110108
sailChangedEvent.setStatementsAdded(true);
111109

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/*
2+
* Copyright (c) 2025 Eclipse RDF4J contributors.
3+
*
4+
* All rights reserved. This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Distribution License v1.0
6+
* which accompanies this distribution, and is available at
7+
* http://www.eclipse.org/org/documents/edl-v10.php.
8+
*
9+
* SPDX-License-Identifier: BSD-3-Clause
10+
*/
11+
package org.eclipse.rdf4j.sail.nativerdf;
12+
13+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
14+
15+
import java.io.File;
16+
import java.lang.reflect.Field;
17+
import java.nio.file.Files;
18+
import java.util.Map;
19+
20+
import org.eclipse.rdf4j.model.IRI;
21+
import org.eclipse.rdf4j.model.Resource;
22+
import org.eclipse.rdf4j.model.ValueFactory;
23+
import org.eclipse.rdf4j.sail.SailException;
24+
import org.eclipse.rdf4j.sail.base.SailSink;
25+
import org.junit.jupiter.api.AfterEach;
26+
import org.junit.jupiter.api.BeforeEach;
27+
import org.junit.jupiter.api.Test;
28+
29+
/**
30+
* Verifies that NativeSailStore wraps unexpected RuntimeExceptions in SailException so upstream callers can reliably
31+
* handle failures (e.g., branch flush clearing pending changes).
32+
*/
33+
public class NativeSailStoreRuntimeWrappingIT {
34+
35+
private File dataDir;
36+
private NativeSailStore store;
37+
38+
@BeforeEach
39+
public void setUp() throws Exception {
40+
dataDir = Files.createTempDirectory("nativestore-wrap-test").toFile();
41+
store = new NativeSailStore(dataDir, "spoc");
42+
}
43+
44+
@AfterEach
45+
public void tearDown() throws Exception {
46+
if (store != null) {
47+
store.close();
48+
}
49+
}
50+
51+
@Test
52+
public void testFlushWrapsRuntimeIntoSailException() throws Exception {
53+
// Replace TripleStore with a stub that throws at commit()
54+
replaceTripleStore(new ThrowOnCommitTripleStore(dataDir, "spoc"));
55+
56+
// Add a statement to ensure a triplestore transaction is started
57+
SailSink sink = store.getExplicitSailSource().sink(null);
58+
ValueFactory vf = store.getValueFactory();
59+
IRI s = vf.createIRI("urn:s");
60+
IRI p = vf.createIRI("urn:p");
61+
IRI o = vf.createIRI("urn:o");
62+
sink.approve(s, p, o, (Resource) null);
63+
64+
// Now flush, expecting a SailException (wrapping the runtime)
65+
assertThatThrownBy(() -> sink.flush())
66+
.isInstanceOf(SailException.class);
67+
}
68+
69+
@Test
70+
public void testRemoveStatementsWrapsRuntimeIntoSailException() throws Exception {
71+
// Replace TripleStore with a stub that throws at removeTriplesByContext()
72+
replaceTripleStore(new ThrowOnRemoveTripleStore(dataDir, "spoc"));
73+
74+
SailSink sink = store.getExplicitSailSource().sink(null);
75+
// Expect SailException when attempting to remove (deprecateByQuery delegates)
76+
assertThatThrownBy(() -> sink.deprecateByQuery(null, null, null, new Resource[] { null }))
77+
.isInstanceOf(SailException.class);
78+
}
79+
80+
private void replaceTripleStore(TripleStore newTripleStore) throws Exception {
81+
Field f = NativeSailStore.class.getDeclaredField("tripleStore");
82+
f.setAccessible(true);
83+
f.set(store, newTripleStore);
84+
}
85+
86+
private static class ThrowOnCommitTripleStore extends TripleStore {
87+
public ThrowOnCommitTripleStore(File dir, String indexSpecStr) throws Exception {
88+
super(dir, indexSpecStr, false);
89+
}
90+
91+
@Override
92+
public void commit() {
93+
throw new RuntimeException("simulated failure during commit");
94+
}
95+
}
96+
97+
private static class ThrowOnRemoveTripleStore extends TripleStore {
98+
public ThrowOnRemoveTripleStore(File dir, String indexSpecStr) throws Exception {
99+
super(dir, indexSpecStr, false);
100+
}
101+
102+
@Override
103+
public Map<Integer, Long> removeTriplesByContext(int subjID, int predID, int objID, int contextId,
104+
boolean explicit) {
105+
throw new RuntimeException("simulated failure during removeTriplesByContext");
106+
}
107+
108+
@Override
109+
public void startTransaction() {
110+
// no-op; we're only interested in remove path throwing
111+
}
112+
}
113+
}

0 commit comments

Comments
 (0)