From a1c9c8c6ed9f1d26be79ded09be16bc8ce0f1428 Mon Sep 17 00:00:00 2001 From: Michael Simons Date: Wed, 4 Oct 2023 14:42:53 +0200 Subject: [PATCH] fix: Unify Bookmarkmanager creation and usage for good. --- src/main/asciidoc/faq/index.adoc | 15 +- .../data/neo4j/core/DefaultNeo4jClient.java | 45 +++--- .../core/DefaultReactiveNeo4jClient.java | 46 +++--- .../support/BookmarkManagerReference.java | 94 ++++++++++++ .../transaction/Neo4jTransactionManager.java | 12 +- .../ReactiveNeo4jTransactionManager.java | 12 +- .../Neo4jTransactionManagerTest.java | 3 +- .../ReactiveNeo4jTransactionManagerTest.java | 3 +- .../custom_queries/CustomQueriesIT.java | 1 - .../bookmarks/DatabaseInitializer.java | 46 ++++++ .../neo4j/integration/bookmarks/Person.java | 33 +++++ .../imperative/NoopBookmarkmanagerIT.java | 138 ++++++++++++++++++ .../ReactiveNoopBookmarkmanagerIT.java | 137 +++++++++++++++++ 13 files changed, 506 insertions(+), 79 deletions(-) create mode 100644 src/main/java/org/springframework/data/neo4j/core/support/BookmarkManagerReference.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/bookmarks/DatabaseInitializer.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/bookmarks/Person.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/bookmarks/imperative/NoopBookmarkmanagerIT.java create mode 100644 src/test/java/org/springframework/data/neo4j/integration/bookmarks/reactive/ReactiveNoopBookmarkmanagerIT.java diff --git a/src/main/asciidoc/faq/index.adoc b/src/main/asciidoc/faq/index.adoc index ccd4b2dd6..805d02416 100644 --- a/src/main/asciidoc/faq/index.adoc +++ b/src/main/asciidoc/faq/index.adoc @@ -734,7 +734,7 @@ WARNING: Use this bookmark manager at your own risk, it will effectively disable In a cluster this can be a sensible approach only and if only you can tolerate stale reads and are not in danger of overwriting old data. -You need to provide the following configuration in your system and make sure that SDN uses the transaction manager: +The following configuration creates a "noop" variant of the bookmark manager that will be picked up from relevant classes. [source,java,indent=0,tabsize=4] .BookmarksDisabledConfig.java @@ -742,25 +742,20 @@ You need to provide the following configuration in your system and make sure tha import org.neo4j.driver.Driver; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; -import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; -import org.springframework.transaction.PlatformTransactionManager; @Configuration public class BookmarksDisabledConfig { @Bean - public PlatformTransactionManager transactionManager( - Driver driver, DatabaseSelectionProvider databaseNameProvider) { + public Neo4jBookmarkManager neo4jBookmarkManager() { - Neo4jBookmarkManager bookmarkManager = Neo4jBookmarkManager.noop(); // <.> - return new Neo4jTransactionManager( - driver, databaseNameProvider, bookmarkManager); + return Neo4jBookmarkManager.noop(); } } ---- -<.> Get an instance of the Noop bookmark manager + +You can configure the pairs of `Neo4jTransactionManager/Neo4jClient` and `ReactiveNeo4jTransactionManager/ReactiveNeo4jClient` individually, but we recommend in doing so only when you already configuring them for specific database selection needs. [[faq.annotations.specific]] == Do I need to use Neo4j specific annotations? diff --git a/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java index 6d08d6306..15690f915 100644 --- a/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java @@ -16,13 +16,9 @@ package org.springframework.data.neo4j.core; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; -import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Function; @@ -39,12 +35,17 @@ import org.neo4j.driver.Session; import org.neo4j.driver.Value; import org.neo4j.driver.summary.ResultSummary; import org.neo4j.driver.types.TypeSystem; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.dao.DataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; import org.springframework.data.neo4j.core.convert.Neo4jConversions; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionUtils; import org.springframework.lang.Nullable; @@ -59,7 +60,7 @@ import org.springframework.util.StringUtils; * @author Michael J. Simons * @since 6.0 */ -final class DefaultNeo4jClient implements Neo4jClient { +final class DefaultNeo4jClient implements Neo4jClient, ApplicationContextAware { private final Driver driver; private @Nullable final DatabaseSelectionProvider databaseSelectionProvider; @@ -67,15 +68,15 @@ final class DefaultNeo4jClient implements Neo4jClient { private final ConversionService conversionService; private final Neo4jPersistenceExceptionTranslator persistenceExceptionTranslator = new Neo4jPersistenceExceptionTranslator(); - // Basically a local bookmark manager - private final Set bookmarks = new HashSet<>(); - private final ReentrantReadWriteLock bookmarksLock = new ReentrantReadWriteLock(); + // Local bookmark manager when using outside managed transactions + private final BookmarkManagerReference bookmarkManager; DefaultNeo4jClient(Builder builder) { this.driver = builder.driver; this.databaseSelectionProvider = builder.databaseSelectionProvider; this.userSelectionProvider = builder.userSelectionProvider; + this.bookmarkManager = new BookmarkManagerReference(Neo4jBookmarkManager::create, null); this.conversionService = new DefaultConversionService(); Optional.ofNullable(builder.neo4jConversions).orElseGet(Neo4jConversions::new).registerConvertersIn((ConverterRegistry) conversionService); @@ -85,29 +86,19 @@ final class DefaultNeo4jClient implements Neo4jClient { public QueryRunner getQueryRunner(DatabaseSelection databaseSelection, UserSelection impersonatedUser) { QueryRunner queryRunner = Neo4jTransactionManager.retrieveTransaction(driver, databaseSelection, impersonatedUser); - Collection lastBookmarks = Collections.emptySet(); + Collection lastBookmarks = bookmarkManager.resolve().getBookmarks(); + if (queryRunner == null) { - ReentrantReadWriteLock.ReadLock lock = bookmarksLock.readLock(); - try { - lock.lock(); - lastBookmarks = new HashSet<>(bookmarks); - queryRunner = driver.session(Neo4jTransactionUtils.sessionConfig(false, lastBookmarks, databaseSelection, impersonatedUser)); - } finally { - lock.unlock(); - } + queryRunner = driver.session(Neo4jTransactionUtils.sessionConfig(false, lastBookmarks, databaseSelection, impersonatedUser)); } - return new DelegatingQueryRunner(queryRunner, lastBookmarks, (usedBookmarks, newBookmarks) -> { + return new DelegatingQueryRunner(queryRunner, lastBookmarks, bookmarkManager.resolve()::updateBookmarks); + } - ReentrantReadWriteLock.WriteLock lock = bookmarksLock.writeLock(); - try { - lock.lock(); - bookmarks.removeAll(usedBookmarks); - bookmarks.addAll(newBookmarks); - } finally { - lock.unlock(); - } - }); + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + + this.bookmarkManager.setApplicationContext(applicationContext); } private static class DelegatingQueryRunner implements QueryRunner { diff --git a/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java b/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java index d1e94ec50..2a0ac340f 100644 --- a/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java +++ b/src/main/java/org/springframework/data/neo4j/core/DefaultReactiveNeo4jClient.java @@ -26,11 +26,16 @@ import org.neo4j.driver.reactivestreams.ReactiveSession; import org.neo4j.driver.summary.ResultSummary; import org.neo4j.driver.types.TypeSystem; import org.reactivestreams.Publisher; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.dao.DataAccessException; import org.springframework.data.neo4j.core.convert.Neo4jConversions; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; import org.springframework.data.neo4j.core.transaction.Neo4jTransactionUtils; import org.springframework.data.neo4j.core.transaction.ReactiveNeo4jTransactionManager; import org.springframework.lang.Nullable; @@ -43,12 +48,8 @@ import reactor.util.function.Tuple2; import reactor.util.function.Tuples; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; import java.util.Map; import java.util.Optional; -import java.util.Set; -import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Function; @@ -62,7 +63,7 @@ import java.util.function.Supplier; * @soundtrack Die Toten Hosen - Im Auftrag des Herrn * @since 6.0 */ -final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient { +final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient, ApplicationContextAware { private final Driver driver; private @Nullable final ReactiveDatabaseSelectionProvider databaseSelectionProvider; @@ -70,9 +71,8 @@ final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient { private final ConversionService conversionService; private final Neo4jPersistenceExceptionTranslator persistenceExceptionTranslator = new Neo4jPersistenceExceptionTranslator(); - // Basically a local bookmark manager - private final Set bookmarks = new HashSet<>(); - private final ReentrantReadWriteLock bookmarksLock = new ReentrantReadWriteLock(); + // Local bookmark manager when using outside managed transactions + private final BookmarkManagerReference bookmarkManager; DefaultReactiveNeo4jClient(Builder builder) { @@ -82,6 +82,7 @@ final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient { this.conversionService = new DefaultConversionService(); Optional.ofNullable(builder.neo4jConversions).orElseGet(Neo4jConversions::new).registerConvertersIn((ConverterRegistry) conversionService); + this.bookmarkManager = new BookmarkManagerReference(Neo4jBookmarkManager::create, null); } @Override @@ -91,27 +92,18 @@ final class DefaultReactiveNeo4jClient implements ReactiveNeo4jClient { .flatMap(targetDatabaseAndUser -> ReactiveNeo4jTransactionManager.retrieveReactiveTransaction(driver, targetDatabaseAndUser.getT1(), targetDatabaseAndUser.getT2()) .map(ReactiveQueryRunner.class::cast) - .zipWith(Mono.just(Collections.emptySet())) + .zipWith(Mono.just(bookmarkManager.resolve().getBookmarks())) .switchIfEmpty(Mono.fromSupplier(() -> { - ReentrantReadWriteLock.ReadLock lock = bookmarksLock.readLock(); - try { - lock.lock(); - Set lastBookmarks = new HashSet<>(bookmarks); - return Tuples.of(driver.session(ReactiveSession.class, Neo4jTransactionUtils.sessionConfig(false, lastBookmarks, targetDatabaseAndUser.getT1(), targetDatabaseAndUser.getT2())), lastBookmarks); - } finally { - lock.unlock(); - } + Collection lastBookmarks = bookmarkManager.resolve().getBookmarks(); + return Tuples.of(driver.session(ReactiveSession.class, Neo4jTransactionUtils.sessionConfig(false, lastBookmarks, targetDatabaseAndUser.getT1(), targetDatabaseAndUser.getT2())), lastBookmarks); }))) - .map(t -> new DelegatingQueryRunner(t.getT1(), t.getT2(), (usedBookmarks, newBookmarks) -> { - ReentrantReadWriteLock.WriteLock lock = bookmarksLock.writeLock(); - try { - lock.lock(); - bookmarks.removeAll(usedBookmarks); - bookmarks.addAll(newBookmarks); - } finally { - lock.unlock(); - } - })); + .map(t -> new DelegatingQueryRunner(t.getT1(), t.getT2(), bookmarkManager.resolve()::updateBookmarks)); + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + + bookmarkManager.setApplicationContext(applicationContext); } private static class DelegatingQueryRunner implements ReactiveQueryRunner { diff --git a/src/main/java/org/springframework/data/neo4j/core/support/BookmarkManagerReference.java b/src/main/java/org/springframework/data/neo4j/core/support/BookmarkManagerReference.java new file mode 100644 index 000000000..a1579feb8 --- /dev/null +++ b/src/main/java/org/springframework/data/neo4j/core/support/BookmarkManagerReference.java @@ -0,0 +1,94 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.core.support; + +import java.util.function.Supplier; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.lang.Nullable; + +/** + * Don't use outside SDN code. You have been warned. + * + * @author Michael J. Simons + */ +public final class BookmarkManagerReference implements ApplicationContextAware { + + private final Supplier defaultBookmarkManagerSupplier; + + private ObjectProvider neo4jBookmarkManagers = new ObjectProvider() { + @Override + public Neo4jBookmarkManager getObject(Object... args) throws BeansException { + throw new BeanCreationException("This provider can't create new beans"); + } + + @Override + public Neo4jBookmarkManager getIfAvailable() throws BeansException { + return null; + } + + @Override + public Neo4jBookmarkManager getIfUnique() throws BeansException { + return null; + } + + @Override + public Neo4jBookmarkManager getObject() throws BeansException { + throw new BeanCreationException("This provider can't create new beans"); + } + }; + + @Nullable + private volatile Neo4jBookmarkManager bookmarkManager; + + private ApplicationEventPublisher applicationEventPublisher; + + public BookmarkManagerReference(Supplier defaultBookmarkManagerSupplier, @Nullable Neo4jBookmarkManager bookmarkManager) { + this.defaultBookmarkManagerSupplier = defaultBookmarkManagerSupplier; + this.bookmarkManager = bookmarkManager; + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + + this.neo4jBookmarkManagers = applicationContext.getBeanProvider(Neo4jBookmarkManager.class); + this.applicationEventPublisher = applicationContext; + if (this.bookmarkManager != null) { + this.bookmarkManager.setApplicationEventPublisher(this.applicationEventPublisher); + } + } + + public Neo4jBookmarkManager resolve() { + Neo4jBookmarkManager result = this.bookmarkManager; + if (result == null) { + synchronized (this) { + result = this.bookmarkManager; + if (result == null) { + this.bookmarkManager = neo4jBookmarkManagers.getIfAvailable(this.defaultBookmarkManagerSupplier); + this.bookmarkManager.setApplicationEventPublisher(this.applicationEventPublisher); + result = this.bookmarkManager; + } + } + } + return result; + } +} diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java index f636e8a8e..bf212a647 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManager.java @@ -30,6 +30,7 @@ import org.springframework.data.neo4j.core.DatabaseSelection; import org.springframework.data.neo4j.core.DatabaseSelectionProvider; import org.springframework.data.neo4j.core.UserSelection; import org.springframework.data.neo4j.core.UserSelectionProvider; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; import org.springframework.lang.Nullable; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionException; @@ -136,7 +137,7 @@ public final class Neo4jTransactionManager extends AbstractPlatformTransactionMa */ private final UserSelectionProvider userSelectionProvider; - private final Neo4jBookmarkManager bookmarkManager; + private final BookmarkManagerReference bookmarkManager; /** * This will create a transaction manager for the default database. @@ -181,14 +182,13 @@ public final class Neo4jTransactionManager extends AbstractPlatformTransactionMa this.userSelectionProvider = builder.userSelectionProvider == null ? UserSelectionProvider.getDefaultSelectionProvider() : builder.userSelectionProvider; - this.bookmarkManager = - builder.bookmarkManager == null ? Neo4jBookmarkManager.create() : builder.bookmarkManager; + this.bookmarkManager = new BookmarkManagerReference(Neo4jBookmarkManager::create, builder.bookmarkManager); } @Override public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - this.bookmarkManager.setApplicationEventPublisher(applicationContext); + this.bookmarkManager.setApplicationContext(applicationContext); } /** @@ -298,7 +298,7 @@ public final class Neo4jTransactionManager extends AbstractPlatformTransactionMa try { // Prepare configuration data Neo4jTransactionContext context = new Neo4jTransactionContext( - databaseSelectionProvider.getDatabaseSelection(), userSelectionProvider.getUserSelection(), bookmarkManager.getBookmarks()); + databaseSelectionProvider.getDatabaseSelection(), userSelectionProvider.getUserSelection(), bookmarkManager.resolve().getBookmarks()); // Configure and open session together with a native transaction Session session = this.driver.session( @@ -341,7 +341,7 @@ public final class Neo4jTransactionManager extends AbstractPlatformTransactionMa Neo4jTransactionObject transactionObject = extractNeo4jTransaction(status); Neo4jTransactionHolder transactionHolder = transactionObject.getRequiredResourceHolder(); Collection newBookmarks = transactionHolder.commit(); - this.bookmarkManager.updateBookmarks(transactionHolder.getBookmarks(), newBookmarks); + this.bookmarkManager.resolve().updateBookmarks(transactionHolder.getBookmarks(), newBookmarks); } @Override diff --git a/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java b/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java index e13ba5bf7..2f132b9a4 100644 --- a/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java +++ b/src/main/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManager.java @@ -30,6 +30,7 @@ import org.springframework.data.neo4j.core.DatabaseSelection; import org.springframework.data.neo4j.core.ReactiveDatabaseSelectionProvider; import org.springframework.data.neo4j.core.ReactiveUserSelectionProvider; import org.springframework.data.neo4j.core.UserSelection; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; import org.springframework.lang.Nullable; import org.springframework.transaction.NoTransactionException; import org.springframework.transaction.TransactionDefinition; @@ -133,7 +134,7 @@ public final class ReactiveNeo4jTransactionManager extends AbstractReactiveTrans */ private final ReactiveUserSelectionProvider userSelectionProvider; - private final Neo4jBookmarkManager bookmarkManager; + private final BookmarkManagerReference bookmarkManager; /** * This will create a transaction manager for the default database. @@ -178,14 +179,13 @@ public final class ReactiveNeo4jTransactionManager extends AbstractReactiveTrans this.userSelectionProvider = builder.userSelectionProvider == null ? ReactiveUserSelectionProvider.getDefaultSelectionProvider() : builder.userSelectionProvider; - this.bookmarkManager = - builder.bookmarkManager == null ? Neo4jBookmarkManager.create() : builder.bookmarkManager; + this.bookmarkManager = new BookmarkManagerReference(Neo4jBookmarkManager::create, builder.bookmarkManager); } @Override public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { - this.bookmarkManager.setApplicationEventPublisher(applicationContext); + this.bookmarkManager.setApplicationContext(applicationContext); } /** @@ -292,7 +292,7 @@ public final class ReactiveNeo4jTransactionManager extends AbstractReactiveTrans userSelectionProvider .getUserSelection() .switchIfEmpty(Mono.just(UserSelection.connectedUser())), - (databaseSelection, userSelection) -> new Neo4jTransactionContext(databaseSelection, userSelection, bookmarkManager.getBookmarks())) + (databaseSelection, userSelection) -> new Neo4jTransactionContext(databaseSelection, userSelection, bookmarkManager.resolve().getBookmarks())) .map(context -> Tuples.of(context, this.driver.session(ReactiveSession.class, Neo4jTransactionUtils.sessionConfig(readOnly, context.getBookmarks(), context.getDatabaseSelection(), context.getUserSelection())))) .flatMap(contextAndSession -> Mono.fromDirect(contextAndSession.getT2().beginTransaction(transactionConfig)).single() .map(nativeTransaction -> new ReactiveNeo4jTransactionHolder(contextAndSession.getT1(), @@ -325,7 +325,7 @@ public final class ReactiveNeo4jTransactionManager extends AbstractReactiveTrans ReactiveNeo4jTransactionHolder holder = extractNeo4jTransaction(genericReactiveTransaction) .getRequiredResourceHolder(); return holder.commit() - .doOnNext(bookmark -> bookmarkManager.updateBookmarks(holder.getBookmarks(), bookmark)) + .doOnNext(bookmark -> bookmarkManager.resolve().updateBookmarks(holder.getBookmarks(), bookmark)) .then(); } diff --git a/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManagerTest.java b/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManagerTest.java index ef6521b8c..d5dfb9cec 100644 --- a/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManagerTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/transaction/Neo4jTransactionManagerTest.java @@ -52,6 +52,7 @@ import org.neo4j.driver.types.TypeSystem; import org.springframework.data.neo4j.core.DatabaseSelection; import org.springframework.data.neo4j.core.UserSelection; import org.springframework.data.neo4j.core.Neo4jClient; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.jta.JtaTransactionManager; @@ -152,7 +153,7 @@ class Neo4jTransactionManagerTest { throws NoSuchFieldException, IllegalAccessException { Field bookmarkManager = Neo4jTransactionManager.class.getDeclaredField("bookmarkManager"); bookmarkManager.setAccessible(true); - bookmarkManager.set(txManager, value); + bookmarkManager.set(txManager, new BookmarkManagerReference(Neo4jBookmarkManager::create, value)); } @Nested diff --git a/src/test/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManagerTest.java b/src/test/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManagerTest.java index f2c15d18a..d16e32c4d 100644 --- a/src/test/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManagerTest.java +++ b/src/test/java/org/springframework/data/neo4j/core/transaction/ReactiveNeo4jTransactionManagerTest.java @@ -46,6 +46,7 @@ import org.neo4j.driver.reactivestreams.ReactiveSession; import org.neo4j.driver.reactivestreams.ReactiveTransaction; import org.springframework.data.neo4j.core.DatabaseSelection; import org.springframework.data.neo4j.core.UserSelection; +import org.springframework.data.neo4j.core.support.BookmarkManagerReference; import org.springframework.data.r2dbc.connectionfactory.R2dbcTransactionManager; import org.springframework.transaction.reactive.TransactionSynchronizationManager; import org.springframework.transaction.reactive.TransactionalOperator; @@ -169,7 +170,7 @@ class ReactiveNeo4jTransactionManagerTest { throws NoSuchFieldException, IllegalAccessException { Field bookmarkManager = ReactiveNeo4jTransactionManager.class.getDeclaredField("bookmarkManager"); bookmarkManager.setAccessible(true); - bookmarkManager.set(txManager, value); + bookmarkManager.set(txManager, new BookmarkManagerReference(Neo4jBookmarkManager::create, value)); } } diff --git a/src/test/java/org/springframework/data/neo4j/documentation/repositories/custom_queries/CustomQueriesIT.java b/src/test/java/org/springframework/data/neo4j/documentation/repositories/custom_queries/CustomQueriesIT.java index c629b8826..3ebee0d69 100644 --- a/src/test/java/org/springframework/data/neo4j/documentation/repositories/custom_queries/CustomQueriesIT.java +++ b/src/test/java/org/springframework/data/neo4j/documentation/repositories/custom_queries/CustomQueriesIT.java @@ -80,7 +80,6 @@ class CustomQueriesIT { static void setupData(@Autowired Driver driver, @Autowired BookmarkCapture bookmarkCapture) throws IOException { try (Session session = driver.session(bookmarkCapture.createSessionConfig())) { - session.run("MATCH (n) DETACH DELETE n").consume(); session.run("MATCH (n) DETACH DELETE n").consume(); CypherUtils.loadCypherFromResource("/data/movies.cypher", session); bookmarkCapture.seedWith(session.lastBookmarks()); diff --git a/src/test/java/org/springframework/data/neo4j/integration/bookmarks/DatabaseInitializer.java b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/DatabaseInitializer.java new file mode 100644 index 000000000..169d2f869 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/DatabaseInitializer.java @@ -0,0 +1,46 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.bookmarks; + +import java.io.IOException; +import java.io.UncheckedIOException; + +import org.neo4j.driver.Driver; +import org.neo4j.driver.Session; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.data.neo4j.integration.movies.shared.CypherUtils; + +/** + * @author Michael J. Simons + */ +public final class DatabaseInitializer implements InitializingBean { + + private final Driver driver; + + public DatabaseInitializer(Driver driver) { + this.driver = driver; + } + + @Override + public void afterPropertiesSet() { + try (Session session = driver.session()) { + session.run("MATCH (n) DETACH DELETE n").consume(); + CypherUtils.loadCypherFromResource("/data/movies.cypher", session); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/bookmarks/Person.java b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/Person.java new file mode 100644 index 000000000..2e69c6092 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/Person.java @@ -0,0 +1,33 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.bookmarks; + +import org.springframework.data.neo4j.core.schema.GeneratedValue; +import org.springframework.data.neo4j.core.schema.Id; +import org.springframework.data.neo4j.core.schema.Node; + +/** + * @author Michael J. Simons + */ +@Node +public class Person { + + @Id + @GeneratedValue + private Long id; + + private String name; +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/bookmarks/imperative/NoopBookmarkmanagerIT.java b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/imperative/NoopBookmarkmanagerIT.java new file mode 100644 index 000000000..36e467695 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/imperative/NoopBookmarkmanagerIT.java @@ -0,0 +1,138 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.bookmarks.imperative; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import org.neo4j.driver.Driver; +import org.neo4j.driver.SessionConfig; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.data.neo4j.integration.bookmarks.DatabaseInitializer; +import org.springframework.data.neo4j.integration.bookmarks.Person; +import org.springframework.data.neo4j.repository.Neo4jRepository; +import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories; +import org.springframework.data.neo4j.repository.query.Query; +import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jImperativeTestConfiguration; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.scheduling.annotation.Async; +import org.springframework.scheduling.annotation.EnableAsync; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +/** + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +public class NoopBookmarkmanagerIT { + + protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport; + + @Configuration + @EnableNeo4jRepositories(considerNestedRepositories = true) + @EnableTransactionManagement + @ComponentScan + @EnableAsync + static class Config extends Neo4jImperativeTestConfiguration { + + @Bean + DatabaseInitializer databaseInitializer(Driver driver) { + return new DatabaseInitializer(driver); + } + + @Bean + public Driver driver() { + var driver = neo4jConnectionSupport.getDriver(); + return Mockito.spy(driver); + } + + @Bean + public Neo4jBookmarkManager bookmarkManager() { + return Neo4jBookmarkManager.noop(); + } + + @Override + public boolean isCypher5Compatible() { + return neo4jConnectionSupport.isCypher5SyntaxCompatible(); + } + } + + @Test + void mustNotUseBookmarks(@Autowired PersonService personService, @Autowired Driver driver) throws ExecutionException, InterruptedException { + + var movies = personService.getMoviesByActorNameLike("Bill"); + assertThat(movies).hasSize(5); + var sessionConfigCaptor = ArgumentCaptor.forClass(SessionConfig.class); + verify(driver, times(5)).session(any(), sessionConfigCaptor.capture()); + assertThat(sessionConfigCaptor.getAllValues()) + .allMatch(cfg -> { + var bookmarks = new ArrayList<>(); + if (cfg.bookmarks() != null) { + cfg.bookmarks().forEach(bookmarks::add); + } + return bookmarks.isEmpty(); + }); + } + + interface PersonRepository extends Neo4jRepository { + + @Async + @Query("MATCH (p:Person) WHERE p.name =~ (('.*' + $name) + '.*') RETURN p.name") + CompletableFuture> findMatchingNames(String name); + + @Query("MATCH (m:Movie)<-[:ACTED_IN]-(p:Person) WHERE p.name= $name return m.title") + CompletableFuture> getPersonMovies(String name); + } + + @Service + static class PersonService { + private final PersonRepository personRepository; + + PersonService(PersonRepository personRepository) { + this.personRepository = personRepository; + } + + public List getMoviesByActorNameLike(String namePattern) throws ExecutionException, InterruptedException { + + CompletableFuture> completableFutureCompletableFuture = personRepository.findMatchingNames(namePattern) + .thenCompose(names -> { + List result = Collections.synchronizedList(new ArrayList()); + var futures = names.stream().map(personRepository::getPersonMovies) + .map(cf -> cf.thenAccept(result::addAll)) + .toArray(CompletableFuture[]::new); + return CompletableFuture.allOf(futures) + .thenApply(__ -> result); + }); + return completableFutureCompletableFuture.get(); + } + } +} diff --git a/src/test/java/org/springframework/data/neo4j/integration/bookmarks/reactive/ReactiveNoopBookmarkmanagerIT.java b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/reactive/ReactiveNoopBookmarkmanagerIT.java new file mode 100644 index 000000000..8a5c280f9 --- /dev/null +++ b/src/test/java/org/springframework/data/neo4j/integration/bookmarks/reactive/ReactiveNoopBookmarkmanagerIT.java @@ -0,0 +1,137 @@ +/* + * Copyright 2011-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.neo4j.integration.bookmarks.reactive; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import org.neo4j.driver.Driver; +import org.neo4j.driver.SessionConfig; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager; +import org.springframework.data.neo4j.integration.bookmarks.DatabaseInitializer; +import org.springframework.data.neo4j.integration.bookmarks.Person; +import org.springframework.data.neo4j.repository.ReactiveNeo4jRepository; +import org.springframework.data.neo4j.repository.config.EnableReactiveNeo4jRepositories; +import org.springframework.data.neo4j.repository.query.Query; +import org.springframework.data.neo4j.test.Neo4jExtension; +import org.springframework.data.neo4j.test.Neo4jIntegrationTest; +import org.springframework.data.neo4j.test.Neo4jReactiveTestConfiguration; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.EnableTransactionManagement; + +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +/** + * @author Michael J. Simons + */ +@Neo4jIntegrationTest +@Tag(Neo4jExtension.NEEDS_REACTIVE_SUPPORT) +public class ReactiveNoopBookmarkmanagerIT { + + protected static Neo4jExtension.Neo4jConnectionSupport neo4jConnectionSupport; + + @Configuration + @EnableReactiveNeo4jRepositories(considerNestedRepositories = true) + @EnableTransactionManagement + @ComponentScan + static class Config extends Neo4jReactiveTestConfiguration { + + @Bean + DatabaseInitializer databaseInitializer(Driver driver) { + return new DatabaseInitializer(driver); + } + + @Bean + public Driver driver() { + var driver = neo4jConnectionSupport.getDriver(); + return Mockito.spy(driver); + } + + @Bean + public Neo4jBookmarkManager bookmarkManager() { + return Neo4jBookmarkManager.noop(); + } + + @Override + public boolean isCypher5Compatible() { + return neo4jConnectionSupport.isCypher5SyntaxCompatible(); + } + } + + @Test + void mustNotUseBookmarks(@Autowired PersonService personService, @Autowired Driver driver) { + + AtomicReference> result = new AtomicReference<>(); + personService.getMoviesByActorNameLike("Bill") + .as(StepVerifier::create) + .consumeNextWith(result::set) + .verifyComplete(); + + assertThat(result) + .hasValueSatisfying(movies -> assertThat(movies).hasSize(5)); + + var sessionConfigCaptor = ArgumentCaptor.forClass(SessionConfig.class); + verify(driver, times(5)).session(any(), sessionConfigCaptor.capture()); + assertThat(sessionConfigCaptor.getAllValues()) + .allMatch(cfg -> { + var bookmarks = new ArrayList<>(); + if (cfg.bookmarks() != null) { + cfg.bookmarks().forEach(bookmarks::add); + } + return bookmarks.isEmpty(); + }); + } + + interface PersonRepository extends ReactiveNeo4jRepository { + + @Query("MATCH (p:Person) WHERE p.name =~ (('.*' + $name) + '.*') RETURN p.name") + Flux findMatchingNames(String name); + + @Query("MATCH (m:Movie)<-[:ACTED_IN]-(p:Person) WHERE p.name= $name return m.title") + Flux getPersonMovies(String name); + } + + @Service + static class PersonService { + private final PersonRepository personRepository; + + PersonService(PersonRepository personRepository) { + this.personRepository = personRepository; + } + + public Mono> getMoviesByActorNameLike(String namePattern) { + return personRepository.findMatchingNames(namePattern) + .flatMap(personRepository::getPersonMovies, 2) + .collectList(); + } + } +}