Skip to content

Commit

Permalink
GH-839 - Check for a read only query hint.
Browse files Browse the repository at this point in the history
Allow custom queries to indicate with `/*+ OGM READ_ONLY */` that they are read only. This will allow procedure calls to not flush the session and fix #839.
  • Loading branch information
michael-simons committed Sep 10, 2020
1 parent 41702d5 commit 6385a97
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
*/
public class ExecuteQueriesDelegate extends SessionDelegate {

private static final String OGM_READ_ONLY_HINT = "/*+ OGM READ_ONLY */";
private static final Pattern WRITE_CYPHER_KEYWORDS = Pattern.compile("\\b(CREATE|MERGE|SET|DELETE|REMOVE|DROP|CALL)\\b",
Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE);
private static final Set<Class<?>> VOID_TYPES = new HashSet<>(Arrays.asList(Void.class, void.class));
Expand Down Expand Up @@ -159,7 +160,7 @@ private <T> Iterable<T> executeAndMap(Class<T> type, String cypher, Map<String,

// While an update query may not return objects, it has enough changes
// to modify all entities in the context, so we must flush it either way.
if (mayBeReadWrite(cypher)) {
if (mayBeReadWrite(cypher) && !cypher.contains(OGM_READ_ONLY_HINT)) {
session.clear();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import ch.qos.logback.classic.LoggerContext;

import java.io.IOException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -39,6 +40,8 @@
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mockito;
import org.neo4j.ogm.context.MappingContext;
import org.neo4j.ogm.domain.cineasts.annotated.Actor;
import org.neo4j.ogm.domain.cineasts.annotated.ExtendedUser;
import org.neo4j.ogm.domain.cineasts.annotated.Movie;
Expand Down Expand Up @@ -149,6 +152,31 @@ && isBoltDriver()) { // 4.1+ will fail on any attempt to write in a read-only tr
}
}

@Test
public void shouldBeAbleToIndicateSafeCall() throws NoSuchFieldException, IllegalAccessException {

// Don' think too long about that bloody mess…
MappingContext spyOnMappingContext = Mockito.spy(((Neo4jSession) session).context());
Field mappingContextField = Neo4jSession.class.getDeclaredField("mappingContext");
mappingContextField.setAccessible(true);
mappingContextField.set(session, spyOnMappingContext);

Iterable<String> functionNames = session
.query(String.class, "CALL dbms.functions() yield name", Collections.emptyMap());
assertThat(functionNames).isNotEmpty();
functionNames = session
.query(String.class, "CALL dbms.functions() yield name /*+ OGM READ_ONLY */", Collections.emptyMap());
assertThat(functionNames).isNotEmpty();
functionNames = session
.query(String.class, "CALL dbms.functions() yield name \n/*+ OGM READ_ONLY */", Collections.emptyMap());
assertThat(functionNames).isNotEmpty();
functionNames = session
.query(String.class, "CALL /*+ OGM READ_ONLY */ dbms.functions() yield name", Collections.emptyMap());
assertThat(functionNames).isNotEmpty();

Mockito.verify(spyOnMappingContext, Mockito.atMost(1)).clear();
}

@Test // DATAGRAPH-697
public void modifyingQueryShouldReturnStatistics() {
session.save(new Actor("Jeff"));
Expand Down

0 comments on commit 6385a97

Please sign in to comment.