Skip to content

Commit

Permalink
GH-1960 treat commit as no-op if no updates in the txn
Browse files Browse the repository at this point in the history
- check if txn string has content before sending update
- added regression test
- minor refactor: use StringBuilder instead of StringBuffer
  • Loading branch information
abrokenjester committed Mar 9, 2020
1 parent dbc5855 commit 802b4cb
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
Expand Up @@ -89,7 +89,7 @@ public class SPARQLConnection extends AbstractRepositoryConnection implements Ht

private final SPARQLProtocolSession client;

private StringBuffer sparqlTransaction;
private StringBuilder sparqlTransaction;

private Object transactionLock = new Object();

Expand Down Expand Up @@ -381,8 +381,9 @@ public GraphQuery prepareGraphQuery(QueryLanguage ql, String query, String base)
@Override
public TupleQuery prepareTupleQuery(QueryLanguage ql, String query, String base)
throws RepositoryException, MalformedQueryException {
if (SPARQL.equals(ql))
if (SPARQL.equals(ql)) {
return new SPARQLTupleQuery(client, base, query);
}
throw new UnsupportedQueryLanguageException("Unsupported query language " + ql);
}

Expand All @@ -391,11 +392,14 @@ public void commit() throws RepositoryException {
synchronized (transactionLock) {
if (isActive()) {
synchronized (transactionLock) {
SPARQLUpdate transaction = new SPARQLUpdate(client, null, sparqlTransaction.toString());
try {
transaction.execute();
} catch (UpdateExecutionException e) {
throw new RepositoryException("error executing transaction", e);
// treat commit as a no-op if transaction string is empty
if (sparqlTransaction.length() > 0) {
SPARQLUpdate transaction = new SPARQLUpdate(client, null, sparqlTransaction.toString());
try {
transaction.execute();
} catch (UpdateExecutionException e) {
throw new RepositoryException("error executing transaction", e);
}
}

sparqlTransaction = null;
Expand Down Expand Up @@ -424,7 +428,7 @@ public void begin() throws RepositoryException {
synchronized (transactionLock) {
if (!isActive()) {
synchronized (transactionLock) {
sparqlTransaction = new StringBuffer();
sparqlTransaction = new StringBuilder();
}
} else {
throw new RepositoryException("active transaction already exists");
Expand Down
Expand Up @@ -7,7 +7,11 @@
*******************************************************************************/
package org.eclipse.rdf4j.repository.sparql;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

Expand Down Expand Up @@ -35,4 +39,13 @@ public void setParserConfigPassesToProtocolSession() throws Exception {
verify(client, times(1)).setParserConfig(config);
}

@Test
public void commitOnEmptyTxnDoesNothing() throws Exception {
subject.begin();
subject.commit();

// verify both method signatures for sendUpdate never get called.
verify(client, never()).sendUpdate(any(), any(), any(), any(), anyBoolean(), anyInt(), any());
verify(client, never()).sendUpdate(any(), any(), any(), any(), anyBoolean(), any());
}
}

0 comments on commit 802b4cb

Please sign in to comment.