diff --git a/.github/workflows/stale-pr-issue.yml b/.github/workflows/stale-pr-issue.yml new file mode 100644 index 0000000..d945402 --- /dev/null +++ b/.github/workflows/stale-pr-issue.yml @@ -0,0 +1,48 @@ +# Mark inactive issues and PRs as stale +# GitHub action based on https://github.com/actions/stale + +name: 'Close stale issues and PRs' +on: + schedule: + # Execute every day + - cron: '0 0 * * *' + +permissions: + actions: write + issues: write + pull-requests: write + +jobs: + close-pending: + runs-on: ubuntu-latest + steps: + - uses: actions/stale@v9 + with: + # GLOBAL ------------------------------------------------------------ + # Exempt any PRs or issues already added to a milestone + exempt-all-milestones: true + # Days until issues or pull requests are labelled as stale + days-before-stale: 60 + + # ISSUES ------------------------------------------------------------ + # Issues will be closed after 90 days of inactive (60 to mark as stale + 30 to close) + days-before-issue-close: 30 + stale-issue-message: > + Hello, this issue has been inactive for 60 days, so we're marking it as stale. + If you would like to continue this discussion, please comment within the next 30 days or we'll close the issue. + close-issue-message: > + Hello, as this issue has been inactive for 90 days, we're closing the issue. + If you would like to resume the discussion, please create a new issue. + exempt-issue-labels: keep-open + + # PULL REQUESTS ----------------------------------------------------- + # PRs will be closed after 90 days of inactive (60 to mark as stale + 30 to close) + days-before-pr-close: 30 + stale-pr-message: > + Hello, this pull request has been inactive for 60 days, so we're marking it as stale. + If you would like to continue working on this pull request, please make an update within the next 30 days, or we'll close the pull request. + close-pr-message: > + Hello, as this pull request has been inactive for 90 days, we're closing this pull request. + We always welcome contributions, and if you would like to continue, please open a new pull request. + exempt-pr-labels: keep-open + \ No newline at end of file diff --git a/README.md b/README.md index fbfd620..9f47b15 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,8 @@ # java-dataloader [![Build](https://github.com/graphql-java/java-dataloader/actions/workflows/master.yml/badge.svg)](https://github.com/graphql-java/java-dataloader/actions/workflows/master.yml) -[![Latest Release](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/java-dataloader/badge.svg)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/java-dataloader/) +[![Latest Release](https://img.shields.io/maven-central/v/com.graphql-java/java-dataloader?versionPrefix=4.)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/graphql-java/) +[![Latest Snapshot](https://img.shields.io/maven-central/v/com.graphql-java/java-dataloader?label=maven-central%20snapshot&versionPrefix=0)](https://maven-badges.herokuapp.com/maven-central/com.graphql-java/graphql-java/) [![Apache licensed](https://img.shields.io/hexpm/l/plug.svg?maxAge=2592000)](https://github.com/graphql-java/java-dataloader/blob/master/LICENSE) This small and simple utility library is a pure Java 11 port of [Facebook DataLoader](https://github.com/facebook/dataloader). @@ -67,7 +68,7 @@ repositories { } dependencies { - compile 'com.graphql-java:java-dataloader: 3.3.0' + compile 'com.graphql-java:java-dataloader: 4.0.0' } ``` @@ -750,6 +751,65 @@ When ticker mode is **true** the `ScheduledDataLoaderRegistry` algorithm is as f * If it returns **true**, then `dataLoader.dispatch()` is called **and** a task is scheduled to re-evaluate this specific dataloader in the near future * The re-evaluation tasks are run periodically according to the `registry.getScheduleDuration()` +## Instrumenting the data loader code + +A `DataLoader` can have a `DataLoaderInstrumentation` associated with it. This callback interface is intended to provide +insight into working of the `DataLoader` such as how long it takes to run or to allow for logging of key events. + +You set the `DataLoaderInstrumentation` into the `DataLoaderOptions` at build time. + +```java + + + DataLoaderInstrumentation timingInstrumentation = new DataLoaderInstrumentation() { + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + long then = System.currentTimeMillis(); + return DataLoaderInstrumentationHelper.whenCompleted((result, err) -> { + long ms = System.currentTimeMillis() - then; + System.out.println(format("dispatch time: %d ms", ms)); + }); + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + long then = System.currentTimeMillis(); + return DataLoaderInstrumentationHelper.whenCompleted((result, err) -> { + long ms = System.currentTimeMillis() - then; + System.out.println(format("batch loader time: %d ms", ms)); + }); + } + }; + DataLoaderOptions options = DataLoaderOptions.newOptions().setInstrumentation(timingInstrumentation); + DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader, options); + +``` + +The example shows how long the overall `DataLoader` dispatch takes or how long the batch loader takes to run. + +### Instrumenting the DataLoaderRegistry + +You can also associate a `DataLoaderInstrumentation` with a `DataLoaderRegistry`. Every `DataLoader` registered will be changed so that the registry +`DataLoaderInstrumentation` is associated with it. This allows you to set just the one `DataLoaderInstrumentation` in place and it applies to all +data loaders. + +```java + DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader); + DataLoader teamsDataLoader = DataLoaderFactory.newDataLoader(teamsBatchLoader); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(timingInstrumentation) + .register("users", userDataLoader) + .register("teams", teamsDataLoader) + .build(); + + DataLoader changedUsersDataLoader = registry.getDataLoader("users"); +``` + +The `timingInstrumentation` here will be associated with the `DataLoader` under the key `users` and the key `teams`. Note that since +DataLoader is immutable, a new changed object is created so you must use the registry to get the `DataLoader`. + + ## Other information sources - [Facebook DataLoader Github repo](https://github.com/facebook/dataloader) diff --git a/build.gradle b/build.gradle index 70d3918..7d98bc3 100644 --- a/build.gradle +++ b/build.gradle @@ -65,8 +65,8 @@ jar { } dependencies { - api "org.slf4j:slf4j-api:$slf4j_version" api "org.reactivestreams:reactive-streams:$reactive_streams_version" + api "org.jspecify:jspecify:1.0.0" } task sourcesJar(type: Jar) { @@ -99,7 +99,6 @@ testing { implementation 'org.junit.jupiter:junit-jupiter-api' implementation 'org.junit.jupiter:junit-jupiter-params' implementation 'org.junit.jupiter:junit-jupiter-engine' - implementation "org.slf4j:slf4j-simple:$slf4j_version" implementation "org.awaitility:awaitility:$awaitility_version" implementation "org.hamcrest:hamcrest:$hamcrest_version" implementation "io.projectreactor:reactor-core:$reactor_core_version" @@ -177,6 +176,7 @@ nexusPublishing { } signing { + required { !project.hasProperty('publishToMavenLocal') } def signingKey = System.env.MAVEN_CENTRAL_PGP_KEY useInMemoryPgpKeys(signingKey, "") sign publishing.publications @@ -199,4 +199,4 @@ tasks.named("dependencyUpdates").configure { rejectVersionIf { isNonStable(it.candidate.version) } -} \ No newline at end of file +} diff --git a/gradle.properties b/gradle.properties index 22d5603..428b6e2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -20,7 +20,6 @@ projectDescription = Port of Facebook Dataloader for Java # Dependency versions. junit_version=5.11.3 hamcrest_version=2.2 -slf4j_version=1.7.30 awaitility_version=2.0.0 reactor_core_version=3.6.6 caffeine_version=3.1.8 diff --git a/src/main/java/org/dataloader/BatchLoader.java b/src/main/java/org/dataloader/BatchLoader.java index c1916e3..2b0c3c5 100644 --- a/src/main/java/org/dataloader/BatchLoader.java +++ b/src/main/java/org/dataloader/BatchLoader.java @@ -17,6 +17,8 @@ package org.dataloader; import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.NullMarked; import java.util.List; import java.util.concurrent.CompletionStage; @@ -74,6 +76,7 @@ */ @FunctionalInterface @PublicSpi +@NullMarked public interface BatchLoader { /** diff --git a/src/main/java/org/dataloader/BatchLoaderContextProvider.java b/src/main/java/org/dataloader/BatchLoaderContextProvider.java index d1eb1fe..702fd66 100644 --- a/src/main/java/org/dataloader/BatchLoaderContextProvider.java +++ b/src/main/java/org/dataloader/BatchLoaderContextProvider.java @@ -1,6 +1,7 @@ package org.dataloader; import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; /** * A BatchLoaderContextProvider is used by the {@link org.dataloader.DataLoader} code to @@ -8,9 +9,10 @@ * case is for propagating user security credentials or database connection parameters for example. */ @PublicSpi +@NullMarked public interface BatchLoaderContextProvider { /** * @return a context object that may be needed in batch load calls */ Object getContext(); -} \ No newline at end of file +} diff --git a/src/main/java/org/dataloader/BatchLoaderEnvironment.java b/src/main/java/org/dataloader/BatchLoaderEnvironment.java index 6039a4a..6b84e70 100644 --- a/src/main/java/org/dataloader/BatchLoaderEnvironment.java +++ b/src/main/java/org/dataloader/BatchLoaderEnvironment.java @@ -2,6 +2,8 @@ import org.dataloader.annotations.PublicApi; import org.dataloader.impl.Assertions; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.ArrayList; import java.util.Collections; @@ -14,6 +16,7 @@ * of the calling users for example or database parameters that allow the data layer call to succeed. */ @PublicApi +@NullMarked public class BatchLoaderEnvironment { private final Object context; @@ -34,7 +37,7 @@ private BatchLoaderEnvironment(Object context, List keyContextsList, Map * @return a context object or null if there isn't one */ @SuppressWarnings("unchecked") - public T getContext() { + public @Nullable T getContext() { return (T) context; } diff --git a/src/main/java/org/dataloader/BatchLoaderEnvironmentProvider.java b/src/main/java/org/dataloader/BatchLoaderEnvironmentProvider.java index fd60a14..dae7c92 100644 --- a/src/main/java/org/dataloader/BatchLoaderEnvironmentProvider.java +++ b/src/main/java/org/dataloader/BatchLoaderEnvironmentProvider.java @@ -1,6 +1,7 @@ package org.dataloader; import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; /** * A BatchLoaderEnvironmentProvider is used by the {@link org.dataloader.DataLoader} code to @@ -9,9 +10,10 @@ * case is for propagating user security credentials or database connection parameters. */ @PublicSpi +@NullMarked public interface BatchLoaderEnvironmentProvider { /** * @return a {@link org.dataloader.BatchLoaderEnvironment} that may be needed in batch calls */ BatchLoaderEnvironment get(); -} \ No newline at end of file +} diff --git a/src/main/java/org/dataloader/BatchLoaderWithContext.java b/src/main/java/org/dataloader/BatchLoaderWithContext.java index fbe66b0..eba26e4 100644 --- a/src/main/java/org/dataloader/BatchLoaderWithContext.java +++ b/src/main/java/org/dataloader/BatchLoaderWithContext.java @@ -1,6 +1,7 @@ package org.dataloader; import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; import java.util.List; import java.util.concurrent.CompletionStage; @@ -14,6 +15,7 @@ * use this interface. */ @PublicSpi +@NullMarked public interface BatchLoaderWithContext { /** * Called to batch load the provided keys and return a promise to a list of values. This default diff --git a/src/main/java/org/dataloader/BatchPublisher.java b/src/main/java/org/dataloader/BatchPublisher.java index c499226..943becf 100644 --- a/src/main/java/org/dataloader/BatchPublisher.java +++ b/src/main/java/org/dataloader/BatchPublisher.java @@ -1,5 +1,8 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import org.reactivestreams.Subscriber; import java.util.List; @@ -18,6 +21,8 @@ * @param type parameter indicating the type of values returned * @see BatchLoader for the non-reactive version */ +@NullMarked +@PublicSpi public interface BatchPublisher { /** * Called to batch the provided keys into a stream of values. You must provide diff --git a/src/main/java/org/dataloader/BatchPublisherWithContext.java b/src/main/java/org/dataloader/BatchPublisherWithContext.java index 4eadfe9..9ee010b 100644 --- a/src/main/java/org/dataloader/BatchPublisherWithContext.java +++ b/src/main/java/org/dataloader/BatchPublisherWithContext.java @@ -1,5 +1,7 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; import org.reactivestreams.Subscriber; import java.util.List; @@ -12,6 +14,8 @@ * See {@link BatchPublisher} for more details on the design invariants that you must implement in order to * use this interface. */ +@NullMarked +@PublicSpi public interface BatchPublisherWithContext { /** * Called to batch the provided keys into a stream of values. You must provide diff --git a/src/main/java/org/dataloader/CacheKey.java b/src/main/java/org/dataloader/CacheKey.java index 88b5f97..c5641b1 100644 --- a/src/main/java/org/dataloader/CacheKey.java +++ b/src/main/java/org/dataloader/CacheKey.java @@ -16,6 +16,9 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; + /** * Function that is invoked on input keys of type {@code K} to derive keys that are required by the {@link CacheMap} * implementation. @@ -25,6 +28,8 @@ * @author Arnold Schrijver */ @FunctionalInterface +@NullMarked +@PublicSpi public interface CacheKey { /** diff --git a/src/main/java/org/dataloader/CacheMap.java b/src/main/java/org/dataloader/CacheMap.java index 1a4a455..54b1b49 100644 --- a/src/main/java/org/dataloader/CacheMap.java +++ b/src/main/java/org/dataloader/CacheMap.java @@ -18,6 +18,8 @@ import org.dataloader.annotations.PublicSpi; import org.dataloader.impl.DefaultCacheMap; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.Collection; import java.util.concurrent.CompletableFuture; @@ -39,6 +41,7 @@ * @author Brad Baker */ @PublicSpi +@NullMarked public interface CacheMap { /** @@ -71,7 +74,7 @@ static CacheMap simpleMap() { * * @return the cached value, or {@code null} if not found (depends on cache implementation) */ - CompletableFuture get(K key); + @Nullable CompletableFuture get(K key); /** * Gets a collection of CompletableFutures from the cache map. diff --git a/src/main/java/org/dataloader/DataLoader.java b/src/main/java/org/dataloader/DataLoader.java index dc4b726..d03e5ac 100644 --- a/src/main/java/org/dataloader/DataLoader.java +++ b/src/main/java/org/dataloader/DataLoader.java @@ -21,13 +21,22 @@ import org.dataloader.impl.CompletableFutureKit; import org.dataloader.stats.Statistics; import org.dataloader.stats.StatisticsCollector; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.function.BiConsumer; +import java.util.function.Consumer; import static org.dataloader.impl.Assertions.nonNull; @@ -54,17 +63,19 @@ * * @param type parameter indicating the type of the data load keys * @param type parameter indicating the type of the data that is returned - * * @author Arnold Schrijver * @author Brad Baker */ @PublicApi +@NullMarked public class DataLoader { private final DataLoaderHelper helper; private final StatisticsCollector stats; private final CacheMap futureCache; private final ValueCache valueCache; + private final DataLoaderOptions options; + private final Object batchLoadFunction; /** * Creates new DataLoader with the specified batch loader function and default options @@ -73,9 +84,7 @@ public class DataLoader { * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -90,13 +99,11 @@ public static DataLoader newDataLoader(BatchLoader batchLoadF * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newDataLoader(BatchLoader batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newDataLoader(BatchLoader batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -114,9 +121,7 @@ public static DataLoader newDataLoader(BatchLoader batchLoadF * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -133,14 +138,12 @@ public static DataLoader newDataLoaderWithTry(BatchLoader * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see DataLoaderFactory#newDataLoaderWithTry(BatchLoader) * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newDataLoaderWithTry(BatchLoader> batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newDataLoaderWithTry(BatchLoader> batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -151,9 +154,7 @@ public static DataLoader newDataLoaderWithTry(BatchLoader * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -168,13 +169,11 @@ public static DataLoader newDataLoader(BatchLoaderWithContext * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newDataLoader(BatchLoaderWithContext batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newDataLoader(BatchLoaderWithContext batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -192,9 +191,7 @@ public static DataLoader newDataLoader(BatchLoaderWithContext * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -211,14 +208,12 @@ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContex * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see DataLoaderFactory#newDataLoaderWithTry(BatchLoader) * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newDataLoaderWithTry(BatchLoaderWithContext> batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newDataLoaderWithTry(BatchLoaderWithContext> batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -229,9 +224,7 @@ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContex * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -246,13 +239,11 @@ public static DataLoader newMappedDataLoader(MappedBatchLoader the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newMappedDataLoader(MappedBatchLoader batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newMappedDataLoader(MappedBatchLoader batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -271,9 +262,7 @@ public static DataLoader newMappedDataLoader(MappedBatchLoader the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -290,14 +279,12 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see DataLoaderFactory#newDataLoaderWithTry(BatchLoader) * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoader> batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoader> batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -308,9 +295,7 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -325,13 +310,11 @@ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithC * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newMappedDataLoader(MappedBatchLoaderWithContext batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newMappedDataLoader(MappedBatchLoaderWithContext batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -349,9 +332,7 @@ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithC * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -368,14 +349,12 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see DataLoaderFactory#newDataLoaderWithTry(BatchLoader) * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoaderWithContext> batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoaderWithContext> batchLoadFunction, @Nullable DataLoaderOptions options) { return DataLoaderFactory.mkDataLoader(batchLoadFunction, options); } @@ -383,7 +362,6 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * Creates a new data loader with the provided batch load function, and default options. * * @param batchLoadFunction the batch load function to use - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated @@ -396,26 +374,27 @@ public DataLoader(BatchLoader batchLoadFunction) { * * @param batchLoadFunction the batch load function to use * @param options the batch load options - * * @deprecated use {@link DataLoaderFactory} instead */ @Deprecated - public DataLoader(BatchLoader batchLoadFunction, DataLoaderOptions options) { + public DataLoader(BatchLoader batchLoadFunction, @Nullable DataLoaderOptions options) { this((Object) batchLoadFunction, options); } @VisibleForTesting - DataLoader(Object batchLoadFunction, DataLoaderOptions options) { + DataLoader(Object batchLoadFunction, @Nullable DataLoaderOptions options) { this(batchLoadFunction, options, Clock.systemUTC()); } @VisibleForTesting - DataLoader(Object batchLoadFunction, DataLoaderOptions options, Clock clock) { + DataLoader(Object batchLoadFunction, @Nullable DataLoaderOptions options, Clock clock) { DataLoaderOptions loaderOptions = options == null ? new DataLoaderOptions() : options; this.futureCache = determineFutureCache(loaderOptions); this.valueCache = determineValueCache(loaderOptions); // order of keys matter in data loader this.stats = nonNull(loaderOptions.getStatisticsCollector()); + this.batchLoadFunction = nonNull(batchLoadFunction); + this.options = loaderOptions; this.helper = new DataLoaderHelper<>(this, batchLoadFunction, loaderOptions, this.futureCache, this.valueCache, this.stats, clock); } @@ -431,6 +410,32 @@ private ValueCache determineValueCache(DataLoaderOptions loaderOptions) { return (ValueCache) loaderOptions.valueCache().orElseGet(ValueCache::defaultValueCache); } + /** + * @return the options used to build this {@link DataLoader} + */ + public DataLoaderOptions getOptions() { + return options; + } + + /** + * @return the batch load interface used to build this {@link DataLoader} + */ + public Object getBatchLoadFunction() { + return batchLoadFunction; + } + + /** + * This allows you to change the current {@link DataLoader} and turn it into a new one + * + * @param builderConsumer the {@link DataLoaderFactory.Builder} consumer for changing the {@link DataLoader} + * @return a newly built {@link DataLoader} instance + */ + public DataLoader transform(Consumer> builderConsumer) { + DataLoaderFactory.Builder builder = DataLoaderFactory.builder(this); + builderConsumer.accept(builder); + return builder.build(); + } + /** * This returns the last instant the data loader was dispatched. When the data loader is created this value is set to now. * @@ -457,7 +462,6 @@ public Duration getTimeSinceDispatch() { * and returned from cache). * * @param key the key to load - * * @return the future of the value */ public CompletableFuture load(K key) { @@ -475,7 +479,6 @@ public CompletableFuture load(K key) { * NOTE : This will NOT cause a data load to happen. You must call {@link #load(Object)} for that to happen. * * @param key the key to check - * * @return an Optional to the future of the value */ public Optional> getIfPresent(K key) { @@ -494,7 +497,6 @@ public Optional> getIfPresent(K key) { * NOTE : This will NOT cause a data load to happen. You must call {@link #load(Object)} for that to happen. * * @param key the key to check - * * @return an Optional to the future of the value */ public Optional> getIfCompleted(K key) { @@ -514,11 +516,10 @@ public Optional> getIfCompleted(K key) { * * @param key the key to load * @param keyContext a context object that is specific to this key - * * @return the future of the value */ - public CompletableFuture load(K key, Object keyContext) { - return helper.load(key, keyContext); + public CompletableFuture load(@NonNull K key, @Nullable Object keyContext) { + return helper.load(nonNull(key), keyContext); } /** @@ -530,7 +531,6 @@ public CompletableFuture load(K key, Object keyContext) { * and returned from cache). * * @param keys the list of keys to load - * * @return the composite future of the list of values */ public CompletableFuture> loadMany(List keys) { @@ -550,7 +550,6 @@ public CompletableFuture> loadMany(List keys) { * * @param keys the list of keys to load * @param keyContexts the list of key calling context objects - * * @return the composite future of the list of values */ public CompletableFuture> loadMany(List keys, List keyContexts) { @@ -583,7 +582,6 @@ public CompletableFuture> loadMany(List keys, List keyContext * {@link org.dataloader.MappedBatchLoaderWithContext} to help retrieve data. * * @param keysAndContexts the map of keys to their respective contexts - * * @return the composite future of the map of keys and values */ public CompletableFuture> loadMany(Map keysAndContexts) { @@ -656,7 +654,6 @@ public int dispatchDepth() { * on the next load request. * * @param key the key to remove - * * @return the data loader for fluent coding */ public DataLoader clear(K key) { @@ -670,7 +667,6 @@ public DataLoader clear(K key) { * * @param key the key to remove * @param handler a handler that will be called after the async remote clear completes - * * @return the data loader for fluent coding */ public DataLoader clear(K key, BiConsumer handler) { @@ -696,7 +692,6 @@ public DataLoader clearAll() { * Clears the entire cache map of the loader, and of the cached value store. * * @param handler a handler that will be called after the async remote clear all completes - * * @return the data loader for fluent coding */ public DataLoader clearAll(BiConsumer handler) { @@ -714,7 +709,6 @@ public DataLoader clearAll(BiConsumer handler) { * * @param key the key * @param value the value - * * @return the data loader for fluent coding */ public DataLoader prime(K key, V value) { @@ -726,7 +720,6 @@ public DataLoader prime(K key, V value) { * * @param key the key * @param error the exception to prime instead of a value - * * @return the data loader for fluent coding */ public DataLoader prime(K key, Exception error) { @@ -740,7 +733,6 @@ public DataLoader prime(K key, Exception error) { * * @param key the key * @param value the value - * * @return the data loader for fluent coding */ public DataLoader prime(K key, CompletableFuture value) { @@ -760,7 +752,6 @@ public DataLoader prime(K key, CompletableFuture value) { * If no cache key function is present in {@link DataLoaderOptions}, then the returned value equals the input key. * * @param key the input key - * * @return the cache key after the input is transformed with the cache key function */ public Object getCacheKey(K key) { @@ -779,6 +770,7 @@ public Statistics getStatistics() { /** * Gets the cacheMap associated with this data loader passed in via {@link DataLoaderOptions#cacheMap()} + * * @return the cacheMap of this data loader */ public CacheMap getCacheMap() { @@ -788,6 +780,7 @@ public CacheMap getCacheMap() { /** * Gets the valueCache associated with this data loader passed in via {@link DataLoaderOptions#valueCache()} + * * @return the valueCache of this data loader */ public ValueCache getValueCache() { diff --git a/src/main/java/org/dataloader/DataLoaderFactory.java b/src/main/java/org/dataloader/DataLoaderFactory.java index db14f2e..ef1a287 100644 --- a/src/main/java/org/dataloader/DataLoaderFactory.java +++ b/src/main/java/org/dataloader/DataLoaderFactory.java @@ -1,6 +1,7 @@ package org.dataloader; import org.dataloader.annotations.PublicApi; +import org.jspecify.annotations.Nullable; /** * A factory class to create {@link DataLoader}s @@ -16,7 +17,6 @@ public class DataLoaderFactory { * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoader(BatchLoader batchLoadFunction) { @@ -30,7 +30,6 @@ public static DataLoader newDataLoader(BatchLoader batchLoadF * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoader(BatchLoader batchLoadFunction, DataLoaderOptions options) { @@ -51,7 +50,6 @@ public static DataLoader newDataLoader(BatchLoader batchLoadF * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoaderWithTry(BatchLoader> batchLoadFunction) { @@ -67,9 +65,7 @@ public static DataLoader newDataLoaderWithTry(BatchLoader * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newDataLoaderWithTry(BatchLoader> batchLoadFunction, DataLoaderOptions options) { @@ -83,7 +79,6 @@ public static DataLoader newDataLoaderWithTry(BatchLoader * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoader(BatchLoaderWithContext batchLoadFunction) { @@ -97,7 +92,6 @@ public static DataLoader newDataLoader(BatchLoaderWithContext * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoader(BatchLoaderWithContext batchLoadFunction, DataLoaderOptions options) { @@ -118,7 +112,6 @@ public static DataLoader newDataLoader(BatchLoaderWithContext * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContext> batchLoadFunction) { @@ -134,9 +127,7 @@ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContex * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContext> batchLoadFunction, DataLoaderOptions options) { @@ -150,7 +141,6 @@ public static DataLoader newDataLoaderWithTry(BatchLoaderWithContex * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedDataLoader(MappedBatchLoader batchLoadFunction) { @@ -164,10 +154,9 @@ public static DataLoader newMappedDataLoader(MappedBatchLoader the key type * @param the value type - * * @return a new DataLoader */ - public static DataLoader newMappedDataLoader(MappedBatchLoader batchLoadFunction, DataLoaderOptions options) { + public static DataLoader newMappedDataLoader(MappedBatchLoader batchLoadFunction, @Nullable DataLoaderOptions options) { return mkDataLoader(batchLoadFunction, options); } @@ -186,7 +175,6 @@ public static DataLoader newMappedDataLoader(MappedBatchLoader the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoader> batchLoadFunction) { @@ -202,9 +190,7 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoader> batchLoadFunction, DataLoaderOptions options) { @@ -218,7 +204,6 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithContext batchLoadFunction) { @@ -232,7 +217,6 @@ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithC * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithContext batchLoadFunction, DataLoaderOptions options) { @@ -253,7 +237,6 @@ public static DataLoader newMappedDataLoader(MappedBatchLoaderWithC * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoaderWithContext> batchLoadFunction) { @@ -269,9 +252,7 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoaderWithContext> batchLoadFunction, DataLoaderOptions options) { @@ -285,7 +266,6 @@ public static DataLoader newMappedDataLoaderWithTry(MappedBatchLoad * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoader(BatchPublisher batchLoadFunction) { @@ -299,7 +279,6 @@ public static DataLoader newPublisherDataLoader(BatchPublisher the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoader(BatchPublisher batchLoadFunction, DataLoaderOptions options) { @@ -320,7 +299,6 @@ public static DataLoader newPublisherDataLoader(BatchPublisher the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoaderWithTry(BatchPublisher> batchLoadFunction) { @@ -336,9 +314,7 @@ public static DataLoader newPublisherDataLoaderWithTry(BatchPublish * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newPublisherDataLoaderWithTry(BatchPublisher> batchLoadFunction, DataLoaderOptions options) { @@ -352,7 +328,6 @@ public static DataLoader newPublisherDataLoaderWithTry(BatchPublish * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoader(BatchPublisherWithContext batchLoadFunction) { @@ -366,7 +341,6 @@ public static DataLoader newPublisherDataLoader(BatchPublisherWithC * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoader(BatchPublisherWithContext batchLoadFunction, DataLoaderOptions options) { @@ -387,7 +361,6 @@ public static DataLoader newPublisherDataLoader(BatchPublisherWithC * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newPublisherDataLoaderWithTry(BatchPublisherWithContext> batchLoadFunction) { @@ -403,9 +376,7 @@ public static DataLoader newPublisherDataLoaderWithTry(BatchPublish * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newPublisherDataLoaderWithTry(BatchPublisher) */ public static DataLoader newPublisherDataLoaderWithTry(BatchPublisherWithContext> batchLoadFunction, DataLoaderOptions options) { @@ -419,7 +390,6 @@ public static DataLoader newPublisherDataLoaderWithTry(BatchPublish * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoader(MappedBatchPublisher batchLoadFunction) { @@ -433,7 +403,6 @@ public static DataLoader newMappedPublisherDataLoader(MappedBatchPu * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoader(MappedBatchPublisher batchLoadFunction, DataLoaderOptions options) { @@ -454,7 +423,6 @@ public static DataLoader newMappedPublisherDataLoader(MappedBatchPu * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoaderWithTry(MappedBatchPublisher> batchLoadFunction) { @@ -470,9 +438,7 @@ public static DataLoader newMappedPublisherDataLoaderWithTry(Mapped * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newDataLoaderWithTry(BatchLoader) */ public static DataLoader newMappedPublisherDataLoaderWithTry(MappedBatchPublisher> batchLoadFunction, DataLoaderOptions options) { @@ -486,7 +452,6 @@ public static DataLoader newMappedPublisherDataLoaderWithTry(Mapped * @param batchLoadFunction the batch load function to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoader(MappedBatchPublisherWithContext batchLoadFunction) { @@ -500,7 +465,6 @@ public static DataLoader newMappedPublisherDataLoader(MappedBatchPu * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoader(MappedBatchPublisherWithContext batchLoadFunction, DataLoaderOptions options) { @@ -521,7 +485,6 @@ public static DataLoader newMappedPublisherDataLoader(MappedBatchPu * @param batchLoadFunction the batch load function to use that uses {@link org.dataloader.Try} objects * @param the key type * @param the value type - * * @return a new DataLoader */ public static DataLoader newMappedPublisherDataLoaderWithTry(MappedBatchPublisherWithContext> batchLoadFunction) { @@ -537,9 +500,7 @@ public static DataLoader newMappedPublisherDataLoaderWithTry(Mapped * @param options the options to use * @param the key type * @param the value type - * * @return a new DataLoader - * * @see #newMappedPublisherDataLoaderWithTry(MappedBatchPublisher) */ public static DataLoader newMappedPublisherDataLoaderWithTry(MappedBatchPublisherWithContext> batchLoadFunction, DataLoaderOptions options) { @@ -549,4 +510,61 @@ public static DataLoader newMappedPublisherDataLoaderWithTry(Mapped static DataLoader mkDataLoader(Object batchLoadFunction, DataLoaderOptions options) { return new DataLoader<>(batchLoadFunction, options); } + + /** + * Return a new {@link Builder} of a data loader. + * + * @param the key type + * @param the value type + * @return a new {@link Builder} of a data loader + */ + public static Builder builder() { + return new Builder<>(); + } + + /** + * Return a new {@link Builder} of a data loader using the specified one as a template. + * + * @param the key type + * @param the value type + * @param dataLoader the {@link DataLoader} to copy values from into the builder + * @return a new {@link Builder} of a data loader + */ + public static Builder builder(DataLoader dataLoader) { + return new Builder<>(dataLoader); + } + + /** + * A builder of {@link DataLoader}s + * + * @param the key type + * @param the value type + */ + public static class Builder { + Object batchLoadFunction; + DataLoaderOptions options = DataLoaderOptions.newOptions(); + + Builder() { + } + + Builder(DataLoader dataLoader) { + this.batchLoadFunction = dataLoader.getBatchLoadFunction(); + this.options = dataLoader.getOptions(); + } + + public Builder batchLoadFunction(Object batchLoadFunction) { + this.batchLoadFunction = batchLoadFunction; + return this; + } + + public Builder options(DataLoaderOptions options) { + this.options = options; + return this; + } + + public DataLoader build() { + return mkDataLoader(batchLoadFunction, options); + } + } } + diff --git a/src/main/java/org/dataloader/DataLoaderHelper.java b/src/main/java/org/dataloader/DataLoaderHelper.java index 9cd38d6..7858780 100644 --- a/src/main/java/org/dataloader/DataLoaderHelper.java +++ b/src/main/java/org/dataloader/DataLoaderHelper.java @@ -3,6 +3,8 @@ import org.dataloader.annotations.GuardedBy; import org.dataloader.annotations.Internal; import org.dataloader.impl.CompletableFutureKit; +import org.dataloader.instrumentation.DataLoaderInstrumentation; +import org.dataloader.instrumentation.DataLoaderInstrumentationContext; import org.dataloader.reactive.ReactiveSupport; import org.dataloader.scheduler.BatchLoaderScheduler; import org.dataloader.stats.StatisticsCollector; @@ -34,6 +36,7 @@ import static java.util.stream.Collectors.toList; import static org.dataloader.impl.Assertions.assertState; import static org.dataloader.impl.Assertions.nonNull; +import static org.dataloader.instrumentation.DataLoaderInstrumentationHelper.ctxOrNoopCtx; /** * This helps break up the large DataLoader class functionality, and it contains the logic to dispatch the @@ -145,12 +148,16 @@ CompletableFuture load(K key, Object loadContext) { boolean cachingEnabled = loaderOptions.cachingEnabled(); stats.incrementLoadCount(new IncrementLoadCountStatisticsContext<>(key, loadContext)); - + DataLoaderInstrumentationContext ctx = ctxOrNoopCtx(instrumentation().beginLoad(dataLoader, key,loadContext)); + CompletableFuture cf; if (cachingEnabled) { - return loadFromCache(key, loadContext, batchingEnabled); + cf = loadFromCache(key, loadContext, batchingEnabled); } else { - return queueOrInvokeLoader(key, loadContext, batchingEnabled, false); + cf = queueOrInvokeLoader(key, loadContext, batchingEnabled, false); } + ctx.onDispatched(); + cf.whenComplete(ctx::onCompleted); + return cf; } } @@ -167,6 +174,8 @@ Object getCacheKeyWithContext(K key, Object context) { } DispatchResult dispatch() { + DataLoaderInstrumentationContext> instrCtx = ctxOrNoopCtx(instrumentation().beginDispatch(dataLoader)); + boolean batchingEnabled = loaderOptions.batchingEnabled(); final List keys; final List callContexts; @@ -175,7 +184,8 @@ DispatchResult dispatch() { int queueSize = loaderQueue.size(); if (queueSize == 0) { lastDispatchTime.set(now()); - return emptyDispatchResult(); + instrCtx.onDispatched(); + return endDispatchCtx(instrCtx, emptyDispatchResult()); } // we copy the pre-loaded set of futures ready for dispatch @@ -192,7 +202,8 @@ DispatchResult dispatch() { lastDispatchTime.set(now()); } if (!batchingEnabled) { - return emptyDispatchResult(); + instrCtx.onDispatched(); + return endDispatchCtx(instrCtx, emptyDispatchResult()); } final int totalEntriesHandled = keys.size(); // @@ -213,7 +224,15 @@ DispatchResult dispatch() { } else { futureList = dispatchQueueBatch(keys, callContexts, queuedFutures); } - return new DispatchResult<>(futureList, totalEntriesHandled); + instrCtx.onDispatched(); + return endDispatchCtx(instrCtx, new DispatchResult<>(futureList, totalEntriesHandled)); + } + + private DispatchResult endDispatchCtx(DataLoaderInstrumentationContext> instrCtx, DispatchResult dispatchResult) { + // once the CF completes, we can tell the instrumentation + dispatchResult.getPromisedResults() + .whenComplete((result, throwable) -> instrCtx.onCompleted(dispatchResult, throwable)); + return dispatchResult; } private CompletableFuture> sliceIntoBatchesOfBatches(List keys, List> queuedFutures, List callContexts, int maxBatchSize) { @@ -390,6 +409,9 @@ CompletableFuture> invokeLoader(List keys, List keyContexts, missedKeyIndexes.add(i); missedKeys.add(keys.get(i)); missedKeyContexts.add(keyContexts.get(i)); + missedQueuedFutures.add(queuedFutures.get(i)); + } else { + queuedFutures.get(i).complete(cacheGet.get()); } } } @@ -424,11 +446,14 @@ CompletableFuture> invokeLoader(List keys, List keyContexts, } CompletableFuture> invokeLoader(List keys, List keyContexts, List> queuedFutures) { + Object context = loaderOptions.getBatchLoaderContextProvider().getContext(); + BatchLoaderEnvironment environment = BatchLoaderEnvironment.newBatchLoaderEnvironment() + .context(context).keyContexts(keys, keyContexts).build(); + + DataLoaderInstrumentationContext> instrCtx = ctxOrNoopCtx(instrumentation().beginBatchLoader(dataLoader, keys, environment)); + CompletableFuture> batchLoad; try { - Object context = loaderOptions.getBatchLoaderContextProvider().getContext(); - BatchLoaderEnvironment environment = BatchLoaderEnvironment.newBatchLoaderEnvironment() - .context(context).keyContexts(keys, keyContexts).build(); if (isMapLoader()) { batchLoad = invokeMapBatchLoader(keys, environment); } else if (isPublisher()) { @@ -438,12 +463,16 @@ CompletableFuture> invokeLoader(List keys, List keyContexts, } else { batchLoad = invokeListBatchLoader(keys, environment); } + instrCtx.onDispatched(); } catch (Exception e) { + instrCtx.onDispatched(); batchLoad = CompletableFutureKit.failedFuture(e); } + batchLoad.whenComplete(instrCtx::onCompleted); return batchLoad; } + @SuppressWarnings("unchecked") private CompletableFuture> invokeListBatchLoader(List keys, BatchLoaderEnvironment environment) { CompletionStage> loadResult; @@ -572,6 +601,10 @@ private boolean isMappedPublisher() { return batchLoadFunction instanceof MappedBatchPublisher; } + private DataLoaderInstrumentation instrumentation() { + return loaderOptions.getInstrumentation(); + } + int dispatchDepth() { synchronized (dataLoader) { return loaderQueue.size(); diff --git a/src/main/java/org/dataloader/DataLoaderOptions.java b/src/main/java/org/dataloader/DataLoaderOptions.java index b96e785..8667943 100644 --- a/src/main/java/org/dataloader/DataLoaderOptions.java +++ b/src/main/java/org/dataloader/DataLoaderOptions.java @@ -17,18 +17,22 @@ package org.dataloader; import org.dataloader.annotations.PublicApi; -import org.dataloader.impl.Assertions; +import org.dataloader.instrumentation.DataLoaderInstrumentation; +import org.dataloader.instrumentation.DataLoaderInstrumentationHelper; import org.dataloader.scheduler.BatchLoaderScheduler; import org.dataloader.stats.NoOpStatisticsCollector; import org.dataloader.stats.StatisticsCollector; +import java.util.Objects; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Supplier; import static org.dataloader.impl.Assertions.nonNull; /** - * Configuration options for {@link DataLoader} instances. + * Configuration options for {@link DataLoader} instances. This is an immutable class so each time + * you change a value it returns a new object. * * @author Arnold Schrijver */ @@ -36,18 +40,21 @@ public class DataLoaderOptions { private static final BatchLoaderContextProvider NULL_PROVIDER = () -> null; - - private boolean batchingEnabled; - private boolean cachingEnabled; - private boolean cachingExceptionsEnabled; - private CacheKey cacheKeyFunction; - private CacheMap cacheMap; - private ValueCache valueCache; - private int maxBatchSize; - private Supplier statisticsCollector; - private BatchLoaderContextProvider environmentProvider; - private ValueCacheOptions valueCacheOptions; - private BatchLoaderScheduler batchLoaderScheduler; + private static final Supplier NOOP_COLLECTOR = NoOpStatisticsCollector::new; + private static final ValueCacheOptions DEFAULT_VALUE_CACHE_OPTIONS = ValueCacheOptions.newOptions(); + + private final boolean batchingEnabled; + private final boolean cachingEnabled; + private final boolean cachingExceptionsEnabled; + private final CacheKey cacheKeyFunction; + private final CacheMap cacheMap; + private final ValueCache valueCache; + private final int maxBatchSize; + private final Supplier statisticsCollector; + private final BatchLoaderContextProvider environmentProvider; + private final ValueCacheOptions valueCacheOptions; + private final BatchLoaderScheduler batchLoaderScheduler; + private final DataLoaderInstrumentation instrumentation; /** * Creates a new data loader options with default settings. @@ -56,11 +63,30 @@ public DataLoaderOptions() { batchingEnabled = true; cachingEnabled = true; cachingExceptionsEnabled = true; + cacheKeyFunction = null; + cacheMap = null; + valueCache = null; maxBatchSize = -1; - statisticsCollector = NoOpStatisticsCollector::new; + statisticsCollector = NOOP_COLLECTOR; environmentProvider = NULL_PROVIDER; - valueCacheOptions = ValueCacheOptions.newOptions(); + valueCacheOptions = DEFAULT_VALUE_CACHE_OPTIONS; batchLoaderScheduler = null; + instrumentation = DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION; + } + + private DataLoaderOptions(Builder builder) { + this.batchingEnabled = builder.batchingEnabled; + this.cachingEnabled = builder.cachingEnabled; + this.cachingExceptionsEnabled = builder.cachingExceptionsEnabled; + this.cacheKeyFunction = builder.cacheKeyFunction; + this.cacheMap = builder.cacheMap; + this.valueCache = builder.valueCache; + this.maxBatchSize = builder.maxBatchSize; + this.statisticsCollector = builder.statisticsCollector; + this.environmentProvider = builder.environmentProvider; + this.valueCacheOptions = builder.valueCacheOptions; + this.batchLoaderScheduler = builder.batchLoaderScheduler; + this.instrumentation = builder.instrumentation; } /** @@ -80,7 +106,8 @@ public DataLoaderOptions(DataLoaderOptions other) { this.statisticsCollector = other.statisticsCollector; this.environmentProvider = other.environmentProvider; this.valueCacheOptions = other.valueCacheOptions; - batchLoaderScheduler = other.batchLoaderScheduler; + this.batchLoaderScheduler = other.batchLoaderScheduler; + this.instrumentation = other.instrumentation; } /** @@ -90,6 +117,51 @@ public static DataLoaderOptions newOptions() { return new DataLoaderOptions(); } + /** + * @return a new default data loader options {@link Builder} that you can then customize + */ + public static DataLoaderOptions.Builder newOptionsBuilder() { + return new DataLoaderOptions.Builder(); + } + + /** + * @param otherOptions the options to copy + * @return a new default data loader options {@link Builder} from the specified one that you can then customize + */ + public static DataLoaderOptions.Builder newDataLoaderOptions(DataLoaderOptions otherOptions) { + return new DataLoaderOptions.Builder(otherOptions); + } + + /** + * Will transform the current options in to a builder ands allow you to build a new set of options + * + * @param builderConsumer the consumer of a builder that has this objects starting values + * @return a new {@link DataLoaderOptions} object + */ + public DataLoaderOptions transform(Consumer builderConsumer) { + Builder builder = newDataLoaderOptions(this); + builderConsumer.accept(builder); + return builder.build(); + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + DataLoaderOptions that = (DataLoaderOptions) o; + return batchingEnabled == that.batchingEnabled + && cachingEnabled == that.cachingEnabled + && cachingExceptionsEnabled == that.cachingExceptionsEnabled + && maxBatchSize == that.maxBatchSize + && Objects.equals(cacheKeyFunction, that.cacheKeyFunction) && + Objects.equals(cacheMap, that.cacheMap) && + Objects.equals(valueCache, that.valueCache) && + Objects.equals(statisticsCollector, that.statisticsCollector) && + Objects.equals(environmentProvider, that.environmentProvider) && + Objects.equals(valueCacheOptions, that.valueCacheOptions) && + Objects.equals(batchLoaderScheduler, that.batchLoaderScheduler); + } + + /** * Option that determines whether to use batching (the default), or not. * @@ -103,12 +175,10 @@ public boolean batchingEnabled() { * Sets the option that determines whether batch loading is enabled. * * @param batchingEnabled {@code true} to enable batch loading, {@code false} otherwise - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setBatchingEnabled(boolean batchingEnabled) { - this.batchingEnabled = batchingEnabled; - return this; + return builder().setBatchingEnabled(batchingEnabled).build(); } /** @@ -124,17 +194,15 @@ public boolean cachingEnabled() { * Sets the option that determines whether caching is enabled. * * @param cachingEnabled {@code true} to enable caching, {@code false} otherwise - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setCachingEnabled(boolean cachingEnabled) { - this.cachingEnabled = cachingEnabled; - return this; + return builder().setCachingEnabled(cachingEnabled).build(); } /** * Option that determines whether to cache exceptional values (the default), or not. - * + *

* For short-lived caches (that is request caches) it makes sense to cache exceptions since * it's likely the key is still poisoned. However, if you have long-lived caches, then it may make * sense to set this to false since the downstream system may have recovered from its failure @@ -150,12 +218,10 @@ public boolean cachingExceptionsEnabled() { * Sets the option that determines whether exceptional values are cache enabled. * * @param cachingExceptionsEnabled {@code true} to enable caching exceptional values, {@code false} otherwise - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setCachingExceptionsEnabled(boolean cachingExceptionsEnabled) { - this.cachingExceptionsEnabled = cachingExceptionsEnabled; - return this; + return builder().setCachingExceptionsEnabled(cachingExceptionsEnabled).build(); } /** @@ -173,12 +239,10 @@ public Optional cacheKeyFunction() { * Sets the function to use for creating the cache key, if caching is enabled. * * @param cacheKeyFunction the cache key function to use - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setCacheKeyFunction(CacheKey cacheKeyFunction) { - this.cacheKeyFunction = cacheKeyFunction; - return this; + return builder().setCacheKeyFunction(cacheKeyFunction).build(); } /** @@ -196,12 +260,10 @@ public DataLoaderOptions setCacheKeyFunction(CacheKey cacheKeyFunction) { * Sets the cache map implementation to use for caching, if caching is enabled. * * @param cacheMap the cache map instance - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setCacheMap(CacheMap cacheMap) { - this.cacheMap = cacheMap; - return this; + return builder().setCacheMap(cacheMap).build(); } /** @@ -219,12 +281,10 @@ public int maxBatchSize() { * before they are split into multiple class * * @param maxBatchSize the maximum batch size - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setMaxBatchSize(int maxBatchSize) { - this.maxBatchSize = maxBatchSize; - return this; + return builder().setMaxBatchSize(maxBatchSize).build(); } /** @@ -240,12 +300,10 @@ public StatisticsCollector getStatisticsCollector() { * a common value * * @param statisticsCollector the statistics collector to use - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setStatisticsCollector(Supplier statisticsCollector) { - this.statisticsCollector = nonNull(statisticsCollector); - return this; + return builder().setStatisticsCollector(nonNull(statisticsCollector)).build(); } /** @@ -259,12 +317,10 @@ public BatchLoaderContextProvider getBatchLoaderContextProvider() { * Sets the batch loader environment provider that will be used to give context to batch load functions * * @param contextProvider the batch loader context provider - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setBatchLoaderContextProvider(BatchLoaderContextProvider contextProvider) { - this.environmentProvider = nonNull(contextProvider); - return this; + return builder().setBatchLoaderContextProvider(nonNull(contextProvider)).build(); } /** @@ -282,12 +338,10 @@ public DataLoaderOptions setBatchLoaderContextProvider(BatchLoaderContextProvide * Sets the value cache implementation to use for caching values, if caching is enabled. * * @param valueCache the value cache instance - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setValueCache(ValueCache valueCache) { - this.valueCache = valueCache; - return this; + return builder().setValueCache(valueCache).build(); } /** @@ -301,12 +355,10 @@ public ValueCacheOptions getValueCacheOptions() { * Sets the {@link ValueCacheOptions} that control how the {@link ValueCache} will be used * * @param valueCacheOptions the value cache options - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setValueCacheOptions(ValueCacheOptions valueCacheOptions) { - this.valueCacheOptions = Assertions.nonNull(valueCacheOptions); - return this; + return builder().setValueCacheOptions(nonNull(valueCacheOptions)).build(); } /** @@ -321,11 +373,129 @@ public BatchLoaderScheduler getBatchLoaderScheduler() { * to some future time. * * @param batchLoaderScheduler the scheduler - * - * @return the data loader options for fluent coding + * @return a new data loader options instance for fluent coding */ public DataLoaderOptions setBatchLoaderScheduler(BatchLoaderScheduler batchLoaderScheduler) { - this.batchLoaderScheduler = batchLoaderScheduler; - return this; + return builder().setBatchLoaderScheduler(batchLoaderScheduler).build(); + } + + /** + * @return the {@link DataLoaderInstrumentation} to use + */ + public DataLoaderInstrumentation getInstrumentation() { + return instrumentation; + } + + /** + * Sets in a new {@link DataLoaderInstrumentation} + * + * @param instrumentation the new {@link DataLoaderInstrumentation} + * @return a new data loader options instance for fluent coding + */ + public DataLoaderOptions setInstrumentation(DataLoaderInstrumentation instrumentation) { + return builder().setInstrumentation(instrumentation).build(); + } + + private Builder builder() { + return new Builder(this); + } + + public static class Builder { + private boolean batchingEnabled; + private boolean cachingEnabled; + private boolean cachingExceptionsEnabled; + private CacheKey cacheKeyFunction; + private CacheMap cacheMap; + private ValueCache valueCache; + private int maxBatchSize; + private Supplier statisticsCollector; + private BatchLoaderContextProvider environmentProvider; + private ValueCacheOptions valueCacheOptions; + private BatchLoaderScheduler batchLoaderScheduler; + private DataLoaderInstrumentation instrumentation; + + public Builder() { + this(new DataLoaderOptions()); // use the defaults of the DataLoaderOptions for this builder + } + + Builder(DataLoaderOptions other) { + this.batchingEnabled = other.batchingEnabled; + this.cachingEnabled = other.cachingEnabled; + this.cachingExceptionsEnabled = other.cachingExceptionsEnabled; + this.cacheKeyFunction = other.cacheKeyFunction; + this.cacheMap = other.cacheMap; + this.valueCache = other.valueCache; + this.maxBatchSize = other.maxBatchSize; + this.statisticsCollector = other.statisticsCollector; + this.environmentProvider = other.environmentProvider; + this.valueCacheOptions = other.valueCacheOptions; + this.batchLoaderScheduler = other.batchLoaderScheduler; + this.instrumentation = other.instrumentation; + } + + public Builder setBatchingEnabled(boolean batchingEnabled) { + this.batchingEnabled = batchingEnabled; + return this; + } + + public Builder setCachingEnabled(boolean cachingEnabled) { + this.cachingEnabled = cachingEnabled; + return this; + } + + public Builder setCachingExceptionsEnabled(boolean cachingExceptionsEnabled) { + this.cachingExceptionsEnabled = cachingExceptionsEnabled; + return this; + } + + public Builder setCacheKeyFunction(CacheKey cacheKeyFunction) { + this.cacheKeyFunction = cacheKeyFunction; + return this; + } + + public Builder setCacheMap(CacheMap cacheMap) { + this.cacheMap = cacheMap; + return this; + } + + public Builder setValueCache(ValueCache valueCache) { + this.valueCache = valueCache; + return this; + } + + public Builder setMaxBatchSize(int maxBatchSize) { + this.maxBatchSize = maxBatchSize; + return this; + } + + public Builder setStatisticsCollector(Supplier statisticsCollector) { + this.statisticsCollector = statisticsCollector; + return this; + } + + public Builder setBatchLoaderContextProvider(BatchLoaderContextProvider environmentProvider) { + this.environmentProvider = environmentProvider; + return this; + } + + public Builder setValueCacheOptions(ValueCacheOptions valueCacheOptions) { + this.valueCacheOptions = valueCacheOptions; + return this; + } + + public Builder setBatchLoaderScheduler(BatchLoaderScheduler batchLoaderScheduler) { + this.batchLoaderScheduler = batchLoaderScheduler; + return this; + } + + public Builder setInstrumentation(DataLoaderInstrumentation instrumentation) { + this.instrumentation = nonNull(instrumentation); + return this; + } + + public DataLoaderOptions build() { + return new DataLoaderOptions(this); + } + } } diff --git a/src/main/java/org/dataloader/DataLoaderRegistry.java b/src/main/java/org/dataloader/DataLoaderRegistry.java index 5a3f90f..06c93c4 100644 --- a/src/main/java/org/dataloader/DataLoaderRegistry.java +++ b/src/main/java/org/dataloader/DataLoaderRegistry.java @@ -1,6 +1,9 @@ package org.dataloader; import org.dataloader.annotations.PublicApi; +import org.dataloader.instrumentation.ChainedDataLoaderInstrumentation; +import org.dataloader.instrumentation.DataLoaderInstrumentation; +import org.dataloader.instrumentation.DataLoaderInstrumentationHelper; import org.dataloader.stats.Statistics; import java.util.ArrayList; @@ -16,30 +19,108 @@ /** * This allows data loaders to be registered together into a single place, so * they can be dispatched as one. It also allows you to retrieve data loaders by - * name from a central place + * name from a central place. + *

+ * Notes on {@link DataLoaderInstrumentation} : A {@link DataLoaderRegistry} can have an instrumentation + * associated with it. As each {@link DataLoader} is added to the registry, the {@link DataLoaderInstrumentation} + * of the registry is applied to that {@link DataLoader}. + *

+ * The {@link DataLoader} is changed and hence the object in the registry is not the + * same one as was originally registered. So you MUST get access to the {@link DataLoader} via {@link DataLoaderRegistry#getDataLoader(String)} methods + * and not use the original {@link DataLoader} object. + *

+ * If the {@link DataLoader} has no {@link DataLoaderInstrumentation} then the registry one is added to it. If it does have one already + * then a {@link ChainedDataLoaderInstrumentation} is created with the registry {@link DataLoaderInstrumentation} in it first and then any other + * {@link DataLoaderInstrumentation}s added after that. If the registry {@link DataLoaderInstrumentation} instance and {@link DataLoader} {@link DataLoaderInstrumentation} instance + * are the same object, then nothing is changed, since the same instrumentation code is being run. */ @PublicApi public class DataLoaderRegistry { - protected final Map> dataLoaders = new ConcurrentHashMap<>(); + protected final Map> dataLoaders; + protected final DataLoaderInstrumentation instrumentation; + public DataLoaderRegistry() { + this(new ConcurrentHashMap<>(), null); } private DataLoaderRegistry(Builder builder) { - this.dataLoaders.putAll(builder.dataLoaders); + this(builder.dataLoaders, builder.instrumentation); + } + + protected DataLoaderRegistry(Map> dataLoaders, DataLoaderInstrumentation instrumentation) { + this.dataLoaders = instrumentDLs(dataLoaders, instrumentation); + this.instrumentation = instrumentation; + } + + private Map> instrumentDLs(Map> incomingDataLoaders, DataLoaderInstrumentation registryInstrumentation) { + Map> dataLoaders = new ConcurrentHashMap<>(incomingDataLoaders); + if (registryInstrumentation != null) { + dataLoaders.replaceAll((k, existingDL) -> instrumentDL(registryInstrumentation, existingDL)); + } + return dataLoaders; + } + + /** + * Can be called to tweak a {@link DataLoader} so that it has the registry {@link DataLoaderInstrumentation} added as the first one. + * + * @param registryInstrumentation the common registry {@link DataLoaderInstrumentation} + * @param existingDL the existing data loader + * @return a new {@link DataLoader} or the same one if there is nothing to change + */ + private static DataLoader instrumentDL(DataLoaderInstrumentation registryInstrumentation, DataLoader existingDL) { + if (registryInstrumentation == null) { + return existingDL; + } + DataLoaderOptions options = existingDL.getOptions(); + DataLoaderInstrumentation existingInstrumentation = options.getInstrumentation(); + // if they have any instrumentations then add to it + if (existingInstrumentation != null) { + if (existingInstrumentation == registryInstrumentation) { + // nothing to change + return existingDL; + } + if (existingInstrumentation == DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION) { + // replace it with the registry one + return mkInstrumentedDataLoader(existingDL, options, registryInstrumentation); + } + if (existingInstrumentation instanceof ChainedDataLoaderInstrumentation) { + // avoids calling a chained inside a chained + DataLoaderInstrumentation newInstrumentation = ((ChainedDataLoaderInstrumentation) existingInstrumentation).prepend(registryInstrumentation); + return mkInstrumentedDataLoader(existingDL, options, newInstrumentation); + } else { + DataLoaderInstrumentation newInstrumentation = new ChainedDataLoaderInstrumentation().add(registryInstrumentation).add(existingInstrumentation); + return mkInstrumentedDataLoader(existingDL, options, newInstrumentation); + } + } else { + return mkInstrumentedDataLoader(existingDL, options, registryInstrumentation); + } + } + + private static DataLoader mkInstrumentedDataLoader(DataLoader existingDL, DataLoaderOptions options, DataLoaderInstrumentation newInstrumentation) { + return existingDL.transform(builder -> builder.options(setInInstrumentation(options, newInstrumentation))); } + private static DataLoaderOptions setInInstrumentation(DataLoaderOptions options, DataLoaderInstrumentation newInstrumentation) { + return options.transform(optionsBuilder -> optionsBuilder.setInstrumentation(newInstrumentation)); + } + + /** + * @return the {@link DataLoaderInstrumentation} associated with this registry which can be null + */ + public DataLoaderInstrumentation getInstrumentation() { + return instrumentation; + } /** * This will register a new dataloader * * @param key the key to put the data loader under * @param dataLoader the data loader to register - * * @return this registry */ public DataLoaderRegistry register(String key, DataLoader dataLoader) { - dataLoaders.put(key, dataLoader); + dataLoaders.put(key, instrumentDL(instrumentation, dataLoader)); return this; } @@ -54,13 +135,15 @@ public DataLoaderRegistry register(String key, DataLoader dataLoader) { * @param mappingFunction the function to compute a data loader * @param the type of keys * @param the type of values - * * @return a data loader */ @SuppressWarnings("unchecked") public DataLoader computeIfAbsent(final String key, final Function> mappingFunction) { - return (DataLoader) dataLoaders.computeIfAbsent(key, mappingFunction); + return (DataLoader) dataLoaders.computeIfAbsent(key, (k) -> { + DataLoader dl = mappingFunction.apply(k); + return instrumentDL(instrumentation, dl); + }); } /** @@ -68,7 +151,6 @@ public DataLoader computeIfAbsent(final String key, * and return a new combined registry * * @param registry the registry to combine into this registry - * * @return a new combined registry */ public DataLoaderRegistry combine(DataLoaderRegistry registry) { @@ -97,7 +179,6 @@ public DataLoaderRegistry combine(DataLoaderRegistry registry) { * This will unregister a new dataloader * * @param key the key of the data loader to unregister - * * @return this registry */ public DataLoaderRegistry unregister(String key) { @@ -111,7 +192,6 @@ public DataLoaderRegistry unregister(String key) { * @param key the key of the data loader * @param the type of keys * @param the type of values - * * @return a data loader or null if its not present */ @SuppressWarnings("unchecked") @@ -182,13 +262,13 @@ public static Builder newRegistry() { public static class Builder { private final Map> dataLoaders = new HashMap<>(); + private DataLoaderInstrumentation instrumentation; /** * This will register a new dataloader * * @param key the key to put the data loader under * @param dataLoader the data loader to register - * * @return this builder for a fluent pattern */ public Builder register(String key, DataLoader dataLoader) { @@ -201,7 +281,6 @@ public Builder register(String key, DataLoader dataLoader) { * from a previous {@link DataLoaderRegistry} * * @param otherRegistry the previous {@link DataLoaderRegistry} - * * @return this builder for a fluent pattern */ public Builder registerAll(DataLoaderRegistry otherRegistry) { @@ -209,6 +288,11 @@ public Builder registerAll(DataLoaderRegistry otherRegistry) { return this; } + public Builder instrumentation(DataLoaderInstrumentation instrumentation) { + this.instrumentation = instrumentation; + return this; + } + /** * @return the newly built {@link DataLoaderRegistry} */ diff --git a/src/main/java/org/dataloader/DelegatingDataLoader.java b/src/main/java/org/dataloader/DelegatingDataLoader.java new file mode 100644 index 0000000..c54a731 --- /dev/null +++ b/src/main/java/org/dataloader/DelegatingDataLoader.java @@ -0,0 +1,188 @@ +package org.dataloader; + +import org.dataloader.annotations.PublicApi; +import org.dataloader.stats.Statistics; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.function.BiConsumer; +import java.util.function.Consumer; + +/** + * This delegating {@link DataLoader} makes it easier to create wrappers of {@link DataLoader}s in case you want to change how + * values are returned for example. + *

+ * The most common way would be to make a new {@link DelegatingDataLoader} subclass that overloads the {@link DelegatingDataLoader#load(Object, Object)} + * method. + *

+ * For example the following allows you to change the returned value in some way : + * 

{@code
+ * DataLoader rawLoader = createDataLoader();
+ * DelegatingDataLoader delegatingDataLoader = new DelegatingDataLoader<>(rawLoader) {
+ *    public CompletableFuture load(@NonNull String key, @Nullable Object keyContext) {
+ *       CompletableFuture cf = super.load(key, keyContext);
+ *       return cf.thenApply(v -> "|" + v + "|");
+ *    }
+ *};
+ *}
+ * + * @param type parameter indicating the type of the data load keys + * @param type parameter indicating the type of the data that is returned + */ +@PublicApi +@NullMarked +public class DelegatingDataLoader extends DataLoader { + + protected final DataLoader delegate; + + /** + * This can be called to unwrap a given {@link DataLoader} such that if it's a {@link DelegatingDataLoader} the underlying + * {@link DataLoader} is returned otherwise it's just passed in data loader + * + * @param dataLoader the dataLoader to unwrap + * @param type parameter indicating the type of the data load keys + * @param type parameter indicating the type of the data that is returned + * @return the delegate dataLoader OR just this current one if it's not wrapped + */ + public static DataLoader unwrap(DataLoader dataLoader) { + if (dataLoader instanceof DelegatingDataLoader) { + return ((DelegatingDataLoader) dataLoader).getDelegate(); + } + return dataLoader; + } + + public DelegatingDataLoader(DataLoader delegate) { + super(delegate.getBatchLoadFunction(), delegate.getOptions()); + this.delegate = delegate; + } + + public DataLoader getDelegate() { + return delegate; + } + + /** + * The {@link DataLoader#load(Object)} and {@link DataLoader#loadMany(List)} type methods all call back + * to the {@link DataLoader#load(Object, Object)} and hence we don't override them. + * + * @param key the key to load + * @param keyContext a context object that is specific to this key + * @return the future of the value + */ + @Override + public CompletableFuture load(@NonNull K key, @Nullable Object keyContext) { + return delegate.load(key, keyContext); + } + + @Override + public DataLoader transform(Consumer> builderConsumer) { + return delegate.transform(builderConsumer); + } + + @Override + public Instant getLastDispatchTime() { + return delegate.getLastDispatchTime(); + } + + @Override + public Duration getTimeSinceDispatch() { + return delegate.getTimeSinceDispatch(); + } + + @Override + public Optional> getIfPresent(K key) { + return delegate.getIfPresent(key); + } + + @Override + public Optional> getIfCompleted(K key) { + return delegate.getIfCompleted(key); + } + + @Override + public CompletableFuture> dispatch() { + return delegate.dispatch(); + } + + @Override + public DispatchResult dispatchWithCounts() { + return delegate.dispatchWithCounts(); + } + + @Override + public List dispatchAndJoin() { + return delegate.dispatchAndJoin(); + } + + @Override + public int dispatchDepth() { + return delegate.dispatchDepth(); + } + + @Override + public Object getCacheKey(K key) { + return delegate.getCacheKey(key); + } + + @Override + public Statistics getStatistics() { + return delegate.getStatistics(); + } + + @Override + public CacheMap getCacheMap() { + return delegate.getCacheMap(); + } + + @Override + public ValueCache getValueCache() { + return delegate.getValueCache(); + } + + @Override + public DataLoader clear(K key) { + delegate.clear(key); + return this; + } + + @Override + public DataLoader clear(K key, BiConsumer handler) { + delegate.clear(key, handler); + return this; + } + + @Override + public DataLoader clearAll() { + delegate.clearAll(); + return this; + } + + @Override + public DataLoader clearAll(BiConsumer handler) { + delegate.clearAll(handler); + return this; + } + + @Override + public DataLoader prime(K key, V value) { + delegate.prime(key, value); + return this; + } + + @Override + public DataLoader prime(K key, Exception error) { + delegate.prime(key, error); + return this; + } + + @Override + public DataLoader prime(K key, CompletableFuture value) { + delegate.prime(key, value); + return this; + } +} diff --git a/src/main/java/org/dataloader/DispatchResult.java b/src/main/java/org/dataloader/DispatchResult.java index 97711da..7305c78 100644 --- a/src/main/java/org/dataloader/DispatchResult.java +++ b/src/main/java/org/dataloader/DispatchResult.java @@ -1,6 +1,7 @@ package org.dataloader; import org.dataloader.annotations.PublicApi; +import org.jspecify.annotations.NullMarked; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -12,6 +13,7 @@ * @param for two */ @PublicApi +@NullMarked public class DispatchResult { private final CompletableFuture> futureList; private final int keysCount; diff --git a/src/main/java/org/dataloader/MappedBatchLoader.java b/src/main/java/org/dataloader/MappedBatchLoader.java index 5a7a1a6..1ad4c79 100644 --- a/src/main/java/org/dataloader/MappedBatchLoader.java +++ b/src/main/java/org/dataloader/MappedBatchLoader.java @@ -16,6 +16,9 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; + import java.util.Map; import java.util.Set; import java.util.concurrent.CompletionStage; @@ -54,6 +57,8 @@ * @param type parameter indicating the type of values returned * */ +@PublicSpi +@NullMarked public interface MappedBatchLoader { /** diff --git a/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java b/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java index 7438d20..9559260 100644 --- a/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java +++ b/src/main/java/org/dataloader/MappedBatchLoaderWithContext.java @@ -16,6 +16,9 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; + import java.util.Map; import java.util.Set; import java.util.concurrent.CompletionStage; @@ -28,6 +31,8 @@ * See {@link MappedBatchLoader} for more details on the design invariants that you must implement in order to * use this interface. */ +@PublicSpi +@NullMarked public interface MappedBatchLoaderWithContext { /** * Called to batch load the provided keys and return a promise to a map of values. diff --git a/src/main/java/org/dataloader/MappedBatchPublisher.java b/src/main/java/org/dataloader/MappedBatchPublisher.java index 754ee52..493401f 100644 --- a/src/main/java/org/dataloader/MappedBatchPublisher.java +++ b/src/main/java/org/dataloader/MappedBatchPublisher.java @@ -1,5 +1,7 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; import org.reactivestreams.Subscriber; import java.util.Map; @@ -16,6 +18,8 @@ * @param type parameter indicating the type of values returned * @see MappedBatchLoader for the non-reactive version */ +@PublicSpi +@NullMarked public interface MappedBatchPublisher { /** * Called to batch the provided keys into a stream of map entries of keys and values. diff --git a/src/main/java/org/dataloader/MappedBatchPublisherWithContext.java b/src/main/java/org/dataloader/MappedBatchPublisherWithContext.java index 2e94152..7b862ca 100644 --- a/src/main/java/org/dataloader/MappedBatchPublisherWithContext.java +++ b/src/main/java/org/dataloader/MappedBatchPublisherWithContext.java @@ -1,5 +1,7 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; import org.reactivestreams.Subscriber; import java.util.List; @@ -13,6 +15,8 @@ * See {@link MappedBatchPublisher} for more details on the design invariants that you must implement in order to * use this interface. */ +@PublicSpi +@NullMarked public interface MappedBatchPublisherWithContext { /** diff --git a/src/main/java/org/dataloader/ValueCache.java b/src/main/java/org/dataloader/ValueCache.java index a8dabb1..80c8402 100644 --- a/src/main/java/org/dataloader/ValueCache.java +++ b/src/main/java/org/dataloader/ValueCache.java @@ -3,6 +3,7 @@ import org.dataloader.annotations.PublicSpi; import org.dataloader.impl.CompletableFutureKit; import org.dataloader.impl.NoOpValueCache; +import org.jspecify.annotations.NullMarked; import java.util.ArrayList; import java.util.List; @@ -38,6 +39,7 @@ * @author Brad Baker */ @PublicSpi +@NullMarked public interface ValueCache { /** @@ -158,4 +160,4 @@ public Throwable fillInStackTrace() { return this; } } -} \ No newline at end of file +} diff --git a/src/main/java/org/dataloader/ValueCacheOptions.java b/src/main/java/org/dataloader/ValueCacheOptions.java index 7e2f025..b681dda 100644 --- a/src/main/java/org/dataloader/ValueCacheOptions.java +++ b/src/main/java/org/dataloader/ValueCacheOptions.java @@ -1,10 +1,15 @@ package org.dataloader; +import org.dataloader.annotations.PublicSpi; +import org.jspecify.annotations.NullMarked; + /** * Options that control how the {@link ValueCache} is used by {@link DataLoader} * * @author Brad Baker */ +@PublicSpi +@NullMarked public class ValueCacheOptions { private final boolean completeValueAfterCacheSet; diff --git a/src/main/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentation.java b/src/main/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentation.java new file mode 100644 index 0000000..bf8a40c --- /dev/null +++ b/src/main/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentation.java @@ -0,0 +1,124 @@ +package org.dataloader.instrumentation; + +import org.dataloader.BatchLoaderEnvironment; +import org.dataloader.DataLoader; +import org.dataloader.DispatchResult; +import org.dataloader.annotations.PublicApi; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Objects; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * This {@link DataLoaderInstrumentation} can chain together multiple instrumentations and have them all called in + * the order of the provided list. + */ +@PublicApi +public class ChainedDataLoaderInstrumentation implements DataLoaderInstrumentation { + private final List instrumentations; + + public ChainedDataLoaderInstrumentation() { + instrumentations = List.of(); + } + + public ChainedDataLoaderInstrumentation(List instrumentations) { + this.instrumentations = List.copyOf(instrumentations); + } + + public List getInstrumentations() { + return instrumentations; + } + + /** + * Adds a new {@link DataLoaderInstrumentation} to the list and creates a new {@link ChainedDataLoaderInstrumentation} + * + * @param instrumentation the one to add + * @return a new ChainedDataLoaderInstrumentation object + */ + public ChainedDataLoaderInstrumentation add(DataLoaderInstrumentation instrumentation) { + ArrayList list = new ArrayList<>(this.instrumentations); + list.add(instrumentation); + return new ChainedDataLoaderInstrumentation(list); + } + + /** + * Prepends a new {@link DataLoaderInstrumentation} to the list and creates a new {@link ChainedDataLoaderInstrumentation} + * + * @param instrumentation the one to add + * @return a new ChainedDataLoaderInstrumentation object + */ + public ChainedDataLoaderInstrumentation prepend(DataLoaderInstrumentation instrumentation) { + ArrayList list = new ArrayList<>(); + list.add(instrumentation); + list.addAll(this.instrumentations); + return new ChainedDataLoaderInstrumentation(list); + } + + /** + * Adds a collection of {@link DataLoaderInstrumentation} to the list and creates a new {@link ChainedDataLoaderInstrumentation} + * + * @param instrumentations the new ones to add + * @return a new ChainedDataLoaderInstrumentation object + */ + public ChainedDataLoaderInstrumentation addAll(Collection instrumentations) { + ArrayList list = new ArrayList<>(this.instrumentations); + list.addAll(instrumentations); + return new ChainedDataLoaderInstrumentation(list); + } + + + @Override + public DataLoaderInstrumentationContext beginLoad(DataLoader dataLoader, Object key, Object loadContext) { + return chainedCtx(it -> it.beginLoad(dataLoader, key, loadContext)); + } + + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + return chainedCtx(it -> it.beginDispatch(dataLoader)); + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + return chainedCtx(it -> it.beginBatchLoader(dataLoader, keys, environment)); + } + + private DataLoaderInstrumentationContext chainedCtx(Function> mapper) { + // if we have zero or 1 instrumentations (and 1 is the most common), then we can avoid an object allocation + // of the ChainedInstrumentationContext since it won't be needed + if (instrumentations.isEmpty()) { + return DataLoaderInstrumentationHelper.noOpCtx(); + } + if (instrumentations.size() == 1) { + return mapper.apply(instrumentations.get(0)); + } + return new ChainedInstrumentationContext<>(dropNullContexts(mapper)); + } + + private List> dropNullContexts(Function> mapper) { + return instrumentations.stream() + .map(mapper) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + } + + private static class ChainedInstrumentationContext implements DataLoaderInstrumentationContext { + private final List> contexts; + + public ChainedInstrumentationContext(List> contexts) { + this.contexts = contexts; + } + + @Override + public void onDispatched() { + contexts.forEach(DataLoaderInstrumentationContext::onDispatched); + } + + @Override + public void onCompleted(T result, Throwable t) { + contexts.forEach(it -> it.onCompleted(result, t)); + } + } +} diff --git a/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentation.java b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentation.java new file mode 100644 index 0000000..bbdba87 --- /dev/null +++ b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentation.java @@ -0,0 +1,53 @@ +package org.dataloader.instrumentation; + +import org.dataloader.BatchLoaderEnvironment; +import org.dataloader.DataLoader; +import org.dataloader.DispatchResult; +import org.dataloader.annotations.PublicSpi; + +import java.util.List; + +/** + * This interface is called when certain actions happen inside a data loader + */ +@PublicSpi +public interface DataLoaderInstrumentation { + /** + * This call back is done just before the {@link DataLoader#load(Object)} methods are invoked, + * and it completes when the load promise is completed. If the value is a cached {@link java.util.concurrent.CompletableFuture} + * then it might return almost immediately, otherwise it will return + * when the batch load function is invoked and values get returned + * + * @param dataLoader the {@link DataLoader} in question + * @param key the key used during the {@link DataLoader#load(Object)} call + * @param loadContext the load context used during the {@link DataLoader#load(Object, Object)} call + * @return a DataLoaderInstrumentationContext or null to be more performant + */ + default DataLoaderInstrumentationContext beginLoad(DataLoader dataLoader, Object key, Object loadContext) { + return null; + } + + /** + * This call back is done just before the {@link DataLoader#dispatch()} is invoked, + * and it completes when the dispatch call promise is done. + * + * @param dataLoader the {@link DataLoader} in question + * @return a DataLoaderInstrumentationContext or null to be more performant + */ + default DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + return null; + } + + /** + * This call back is done just before the `batch loader` of a {@link DataLoader} is invoked. Remember a batch loader + * could be called multiple times during a dispatch event (because of max batch sizes) + * + * @param dataLoader the {@link DataLoader} in question + * @param keys the set of keys being fetched + * @param environment the {@link BatchLoaderEnvironment} + * @return a DataLoaderInstrumentationContext or null to be more performant + */ + default DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + return null; + } +} diff --git a/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationContext.java b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationContext.java new file mode 100644 index 0000000..88b08ef --- /dev/null +++ b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationContext.java @@ -0,0 +1,33 @@ +package org.dataloader.instrumentation; + +import org.dataloader.annotations.PublicSpi; + +import java.util.concurrent.CompletableFuture; + +/** + * When a {@link DataLoaderInstrumentation}.'beginXXX()' method is called then it must return a {@link DataLoaderInstrumentationContext} + * that will be invoked when the step is first dispatched and then when it completes. Sometimes this is effectively the same time + * whereas at other times it's when an asynchronous {@link CompletableFuture} completes. + *

+ * This pattern of construction of an object then call back is intended to allow "timers" to be created that can instrument what has + * just happened or "loggers" to be called to record what has happened. + */ +@PublicSpi +public interface DataLoaderInstrumentationContext { + /** + * This is invoked when the instrumentation step is initially dispatched. Note this is NOT + * the same time as the {@link DataLoaderInstrumentation}`beginXXX()` starts, but rather after all the inner + * work has been done. + */ + default void onDispatched() { + } + + /** + * This is invoked when the instrumentation step is fully completed. + * + * @param result the result of the step (which may be null) + * @param t this exception will be non-null if an exception was thrown during the step + */ + default void onCompleted(T result, Throwable t) { + } +} diff --git a/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationHelper.java b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationHelper.java new file mode 100644 index 0000000..9e60060 --- /dev/null +++ b/src/main/java/org/dataloader/instrumentation/DataLoaderInstrumentationHelper.java @@ -0,0 +1,74 @@ +package org.dataloader.instrumentation; + +import org.dataloader.annotations.PublicApi; + +import java.util.function.BiConsumer; + +@PublicApi +public class DataLoaderInstrumentationHelper { + + @SuppressWarnings("RedundantMethodOverride") + private static final DataLoaderInstrumentationContext NOOP_CTX = new DataLoaderInstrumentationContext<>() { + @Override + public void onDispatched() { + } + + @Override + public void onCompleted(Object result, Throwable t) { + } + }; + + /** + * Returns a noop {@link DataLoaderInstrumentationContext} of the right type + * + * @param for two + * @return a noop context + */ + public static DataLoaderInstrumentationContext noOpCtx() { + //noinspection unchecked + return (DataLoaderInstrumentationContext) NOOP_CTX; + } + + /** + * A well known noop {@link DataLoaderInstrumentation} + */ + public static final DataLoaderInstrumentation NOOP_INSTRUMENTATION = new DataLoaderInstrumentation() { + }; + + /** + * Allows for the more fluent away to return an instrumentation context that runs the specified + * code on instrumentation step dispatch. + * + * @param codeToRun the code to run on dispatch + * @param the generic type + * @return an instrumentation context + */ + public static DataLoaderInstrumentationContext whenDispatched(Runnable codeToRun) { + return new SimpleDataLoaderInstrumentationContext<>(codeToRun, null); + } + + /** + * Allows for the more fluent away to return an instrumentation context that runs the specified + * code on instrumentation step completion. + * + * @param codeToRun the code to run on completion + * @param the generic type + * @return an instrumentation context + */ + public static DataLoaderInstrumentationContext whenCompleted(BiConsumer codeToRun) { + return new SimpleDataLoaderInstrumentationContext<>(null, codeToRun); + } + + + /** + * Check the {@link DataLoaderInstrumentationContext} to see if its null and returns a noop if it is or else the original + * context. This is a bit of a helper method. + * + * @param ic the context in play + * @param for two + * @return a non null context + */ + public static DataLoaderInstrumentationContext ctxOrNoopCtx(DataLoaderInstrumentationContext ic) { + return ic == null ? noOpCtx() : ic; + } +} diff --git a/src/main/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContext.java b/src/main/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContext.java new file mode 100644 index 0000000..f629a05 --- /dev/null +++ b/src/main/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContext.java @@ -0,0 +1,35 @@ +package org.dataloader.instrumentation; + + +import org.dataloader.annotations.Internal; + +import java.util.function.BiConsumer; + +/** + * A simple implementation of {@link DataLoaderInstrumentationContext} + */ +@Internal +class SimpleDataLoaderInstrumentationContext implements DataLoaderInstrumentationContext { + + private final BiConsumer codeToRunOnComplete; + private final Runnable codeToRunOnDispatch; + + SimpleDataLoaderInstrumentationContext(Runnable codeToRunOnDispatch, BiConsumer codeToRunOnComplete) { + this.codeToRunOnComplete = codeToRunOnComplete; + this.codeToRunOnDispatch = codeToRunOnDispatch; + } + + @Override + public void onDispatched() { + if (codeToRunOnDispatch != null) { + codeToRunOnDispatch.run(); + } + } + + @Override + public void onCompleted(T result, Throwable t) { + if (codeToRunOnComplete != null) { + codeToRunOnComplete.accept(result, t); + } + } +} diff --git a/src/main/java/org/dataloader/registries/ScheduledDataLoaderRegistry.java b/src/main/java/org/dataloader/registries/ScheduledDataLoaderRegistry.java index 6ea9425..b6bc257 100644 --- a/src/main/java/org/dataloader/registries/ScheduledDataLoaderRegistry.java +++ b/src/main/java/org/dataloader/registries/ScheduledDataLoaderRegistry.java @@ -3,6 +3,7 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderRegistry; import org.dataloader.annotations.ExperimentalApi; +import org.dataloader.instrumentation.DataLoaderInstrumentation; import java.time.Duration; import java.util.LinkedHashMap; @@ -64,8 +65,7 @@ public class ScheduledDataLoaderRegistry extends DataLoaderRegistry implements A private volatile boolean closed; private ScheduledDataLoaderRegistry(Builder builder) { - super(); - this.dataLoaders.putAll(builder.dataLoaders); + super(builder.dataLoaders, builder.instrumentation); this.scheduledExecutorService = builder.scheduledExecutorService; this.defaultExecutorUsed = builder.defaultExecutorUsed; this.schedule = builder.schedule; @@ -271,6 +271,8 @@ public static class Builder { private boolean defaultExecutorUsed = false; private Duration schedule = Duration.ofMillis(10); private boolean tickerMode = false; + private DataLoaderInstrumentation instrumentation; + /** * If you provide a {@link ScheduledExecutorService} then it will NOT be shutdown when @@ -363,6 +365,11 @@ public Builder tickerMode(boolean tickerMode) { return this; } + public Builder instrumentation(DataLoaderInstrumentation instrumentation) { + this.instrumentation = instrumentation; + return this; + } + /** * @return the newly built {@link ScheduledDataLoaderRegistry} */ diff --git a/src/test/java/ReadmeExamples.java b/src/test/java/ReadmeExamples.java index 9e30c90..1f718aa 100644 --- a/src/test/java/ReadmeExamples.java +++ b/src/test/java/ReadmeExamples.java @@ -6,12 +6,17 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderFactory; import org.dataloader.DataLoaderOptions; +import org.dataloader.DataLoaderRegistry; +import org.dataloader.DispatchResult; import org.dataloader.MappedBatchLoaderWithContext; import org.dataloader.MappedBatchPublisher; import org.dataloader.Try; import org.dataloader.fixtures.SecurityCtx; import org.dataloader.fixtures.User; import org.dataloader.fixtures.UserManager; +import org.dataloader.instrumentation.DataLoaderInstrumentation; +import org.dataloader.instrumentation.DataLoaderInstrumentationContext; +import org.dataloader.instrumentation.DataLoaderInstrumentationHelper; import org.dataloader.registries.DispatchPredicate; import org.dataloader.registries.ScheduledDataLoaderRegistry; import org.dataloader.scheduler.BatchLoaderScheduler; @@ -228,6 +233,7 @@ private void clearCacheOnError() { } BatchLoader userBatchLoader; + BatchLoader teamsBatchLoader; private void disableCache() { DataLoaderFactory.newDataLoader(userBatchLoader, DataLoaderOptions.newOptions().setCachingEnabled(false)); @@ -380,4 +386,63 @@ private void ScheduledDispatcherChained() { .build(); } + + private DataLoaderInstrumentation timingInstrumentation = DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION; + + private void instrumentationExample() { + + DataLoaderInstrumentation timingInstrumentation = new DataLoaderInstrumentation() { + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + long then = System.currentTimeMillis(); + return DataLoaderInstrumentationHelper.whenCompleted((result, err) -> { + long ms = System.currentTimeMillis() - then; + System.out.println(format("dispatch time: %d ms", ms)); + }); + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + long then = System.currentTimeMillis(); + return DataLoaderInstrumentationHelper.whenCompleted((result, err) -> { + long ms = System.currentTimeMillis() - then; + System.out.println(format("batch loader time: %d ms", ms)); + }); + } + }; + DataLoaderOptions options = DataLoaderOptions.newOptions().setInstrumentation(timingInstrumentation); + DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader, options); + } + + private void registryExample() { + DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader); + DataLoader teamsDataLoader = DataLoaderFactory.newDataLoader(teamsBatchLoader); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(timingInstrumentation) + .register("users", userDataLoader) + .register("teams", teamsDataLoader) + .build(); + + DataLoader changedUsersDataLoader = registry.getDataLoader("users"); + + } + + private void combiningRegistryExample() { + DataLoader userDataLoader = DataLoaderFactory.newDataLoader(userBatchLoader); + DataLoader teamsDataLoader = DataLoaderFactory.newDataLoader(teamsBatchLoader); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .register("users", userDataLoader) + .register("teams", teamsDataLoader) + .build(); + + DataLoaderRegistry registryCombined = DataLoaderRegistry.newRegistry() + .instrumentation(timingInstrumentation) + .registerAll(registry) + .build(); + + DataLoader changedUsersDataLoader = registryCombined.getDataLoader("users"); + + } } diff --git a/src/test/java/org/dataloader/DataLoaderBuilderTest.java b/src/test/java/org/dataloader/DataLoaderBuilderTest.java new file mode 100644 index 0000000..f38ff82 --- /dev/null +++ b/src/test/java/org/dataloader/DataLoaderBuilderTest.java @@ -0,0 +1,76 @@ +package org.dataloader; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; + +public class DataLoaderBuilderTest { + + BatchLoader batchLoader1 = keys -> null; + + BatchLoader batchLoader2 = keys -> null; + + DataLoaderOptions defaultOptions = DataLoaderOptions.newOptions(); + DataLoaderOptions differentOptions = DataLoaderOptions.newOptions().setCachingEnabled(false); + + @Test + void canBuildNewDataLoaders() { + DataLoaderFactory.Builder builder = DataLoaderFactory.builder(); + builder.options(differentOptions); + builder.batchLoadFunction(batchLoader1); + DataLoader dataLoader = builder.build(); + + assertThat(dataLoader.getOptions(), equalTo(differentOptions)); + assertThat(dataLoader.getBatchLoadFunction(), equalTo(batchLoader1)); + // + // and we can copy ok + // + builder = DataLoaderFactory.builder(dataLoader); + dataLoader = builder.build(); + + assertThat(dataLoader.getOptions(), equalTo(differentOptions)); + assertThat(dataLoader.getBatchLoadFunction(), equalTo(batchLoader1)); + // + // and we can copy and transform ok + // + builder = DataLoaderFactory.builder(dataLoader); + builder.options(defaultOptions); + builder.batchLoadFunction(batchLoader2); + dataLoader = builder.build(); + + assertThat(dataLoader.getOptions(), equalTo(defaultOptions)); + assertThat(dataLoader.getBatchLoadFunction(), equalTo(batchLoader2)); + } + + @Test + void theDataLoaderCanTransform() { + DataLoader dataLoaderOrig = DataLoaderFactory.newDataLoader(batchLoader1, defaultOptions); + assertThat(dataLoaderOrig.getOptions(), equalTo(defaultOptions)); + assertThat(dataLoaderOrig.getBatchLoadFunction(), equalTo(batchLoader1)); + // + // we can transform the data loader + // + DataLoader dataLoaderTransformed = dataLoaderOrig.transform(it -> { + it.options(differentOptions); + it.batchLoadFunction(batchLoader2); + }); + + assertThat(dataLoaderTransformed, not(equalTo(dataLoaderOrig))); + assertThat(dataLoaderTransformed.getOptions(), equalTo(differentOptions)); + assertThat(dataLoaderTransformed.getBatchLoadFunction(), equalTo(batchLoader2)); + + // can copy values + dataLoaderOrig = DataLoaderFactory.newDataLoader(batchLoader1, defaultOptions); + + dataLoaderTransformed = dataLoaderOrig.transform(it -> { + it.batchLoadFunction(batchLoader2); + }); + + assertThat(dataLoaderTransformed, not(equalTo(dataLoaderOrig))); + assertThat(dataLoaderTransformed.getOptions(), equalTo(defaultOptions)); + assertThat(dataLoaderTransformed.getBatchLoadFunction(), equalTo(batchLoader2)); + + } +} diff --git a/src/test/java/org/dataloader/DataLoaderCacheMapTest.java b/src/test/java/org/dataloader/DataLoaderCacheMapTest.java index a7b82b7..df364a2 100644 --- a/src/test/java/org/dataloader/DataLoaderCacheMapTest.java +++ b/src/test/java/org/dataloader/DataLoaderCacheMapTest.java @@ -43,7 +43,7 @@ public void should_access_to_future_dependants() { Collection> futures = dataLoader.getCacheMap().getAll(); List> futuresList = new ArrayList<>(futures); - assertThat(futuresList.get(0).getNumberOfDependents(), equalTo(2)); - assertThat(futuresList.get(1).getNumberOfDependents(), equalTo(1)); + assertThat(futuresList.get(0).getNumberOfDependents(), equalTo(4)); // instrumentation is depending on the CF completing + assertThat(futuresList.get(1).getNumberOfDependents(), equalTo(2)); } } diff --git a/src/test/java/org/dataloader/DataLoaderOptionsTest.java b/src/test/java/org/dataloader/DataLoaderOptionsTest.java new file mode 100644 index 0000000..b4ebb9e --- /dev/null +++ b/src/test/java/org/dataloader/DataLoaderOptionsTest.java @@ -0,0 +1,230 @@ +package org.dataloader; + +import org.dataloader.impl.DefaultCacheMap; +import org.dataloader.impl.NoOpValueCache; +import org.dataloader.instrumentation.DataLoaderInstrumentation; +import org.dataloader.scheduler.BatchLoaderScheduler; +import org.dataloader.stats.NoOpStatisticsCollector; +import org.dataloader.stats.StatisticsCollector; +import org.hamcrest.CoreMatchers; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletionStage; +import java.util.function.Supplier; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +@SuppressWarnings("OptionalGetWithoutIsPresent") +class DataLoaderOptionsTest { + + DataLoaderOptions optionsDefault = new DataLoaderOptions(); + + @Test + void canCreateDefaultOptions() { + + assertThat(optionsDefault.batchingEnabled(), equalTo(true)); + assertThat(optionsDefault.cachingEnabled(), equalTo(true)); + assertThat(optionsDefault.cachingExceptionsEnabled(), equalTo(true)); + assertThat(optionsDefault.maxBatchSize(), equalTo(-1)); + assertThat(optionsDefault.getBatchLoaderScheduler(), equalTo(null)); + + DataLoaderOptions builtOptions = DataLoaderOptions.newOptionsBuilder().build(); + assertThat(builtOptions, equalTo(optionsDefault)); + assertThat(builtOptions == optionsDefault, equalTo(false)); + + DataLoaderOptions transformedOptions = optionsDefault.transform(builder -> { + }); + assertThat(transformedOptions, equalTo(optionsDefault)); + assertThat(transformedOptions == optionsDefault, equalTo(false)); + } + + @Test + void canCopyOk() { + DataLoaderOptions optionsNext = new DataLoaderOptions(optionsDefault); + assertThat(optionsNext, equalTo(optionsDefault)); + assertThat(optionsNext == optionsDefault, equalTo(false)); + + optionsNext = DataLoaderOptions.newDataLoaderOptions(optionsDefault).build(); + assertThat(optionsNext, equalTo(optionsDefault)); + assertThat(optionsNext == optionsDefault, equalTo(false)); + } + + BatchLoaderScheduler testBatchLoaderScheduler = new BatchLoaderScheduler() { + @Override + public CompletionStage> scheduleBatchLoader(ScheduledBatchLoaderCall scheduledCall, List keys, BatchLoaderEnvironment environment) { + return null; + } + + @Override + public CompletionStage> scheduleMappedBatchLoader(ScheduledMappedBatchLoaderCall scheduledCall, List keys, BatchLoaderEnvironment environment) { + return null; + } + + @Override + public void scheduleBatchPublisher(ScheduledBatchPublisherCall scheduledCall, List keys, BatchLoaderEnvironment environment) { + + } + }; + + BatchLoaderContextProvider testBatchLoaderContextProvider = () -> null; + + CacheMap testCacheMap = new DefaultCacheMap<>(); + + ValueCache testValueCache = new NoOpValueCache<>(); + + CacheKey testCacheKey = new CacheKey() { + @Override + public Object getKey(Object input) { + return null; + } + }; + + ValueCacheOptions testValueCacheOptions = ValueCacheOptions.newOptions(); + + NoOpStatisticsCollector noOpStatisticsCollector = new NoOpStatisticsCollector(); + Supplier testStatisticsCollectorSupplier = () -> noOpStatisticsCollector; + + @Test + void canBuildOk() { + assertThat(optionsDefault.setBatchingEnabled(false).batchingEnabled(), + equalTo(false)); + assertThat(optionsDefault.setBatchLoaderScheduler(testBatchLoaderScheduler).getBatchLoaderScheduler(), + equalTo(testBatchLoaderScheduler)); + assertThat(optionsDefault.setBatchLoaderContextProvider(testBatchLoaderContextProvider).getBatchLoaderContextProvider(), + equalTo(testBatchLoaderContextProvider)); + assertThat(optionsDefault.setCacheMap(testCacheMap).cacheMap().get(), + equalTo(testCacheMap)); + assertThat(optionsDefault.setCachingEnabled(false).cachingEnabled(), + equalTo(false)); + assertThat(optionsDefault.setValueCacheOptions(testValueCacheOptions).getValueCacheOptions(), + equalTo(testValueCacheOptions)); + assertThat(optionsDefault.setCacheKeyFunction(testCacheKey).cacheKeyFunction().get(), + equalTo(testCacheKey)); + assertThat(optionsDefault.setValueCache(testValueCache).valueCache().get(), + equalTo(testValueCache)); + assertThat(optionsDefault.setMaxBatchSize(10).maxBatchSize(), + equalTo(10)); + assertThat(optionsDefault.setStatisticsCollector(testStatisticsCollectorSupplier).getStatisticsCollector(), + equalTo(testStatisticsCollectorSupplier.get())); + + DataLoaderOptions builtOptions = optionsDefault.transform(builder -> { + builder.setBatchingEnabled(false); + builder.setCachingExceptionsEnabled(false); + builder.setCachingEnabled(false); + builder.setBatchLoaderScheduler(testBatchLoaderScheduler); + builder.setBatchLoaderContextProvider(testBatchLoaderContextProvider); + builder.setCacheMap(testCacheMap); + builder.setValueCache(testValueCache); + builder.setCacheKeyFunction(testCacheKey); + builder.setValueCacheOptions(testValueCacheOptions); + builder.setMaxBatchSize(10); + builder.setStatisticsCollector(testStatisticsCollectorSupplier); + }); + + assertThat(builtOptions.batchingEnabled(), + equalTo(false)); + assertThat(builtOptions.getBatchLoaderScheduler(), + equalTo(testBatchLoaderScheduler)); + assertThat(builtOptions.getBatchLoaderContextProvider(), + equalTo(testBatchLoaderContextProvider)); + assertThat(builtOptions.cacheMap().get(), + equalTo(testCacheMap)); + assertThat(builtOptions.cachingEnabled(), + equalTo(false)); + assertThat(builtOptions.getValueCacheOptions(), + equalTo(testValueCacheOptions)); + assertThat(builtOptions.cacheKeyFunction().get(), + equalTo(testCacheKey)); + assertThat(builtOptions.valueCache().get(), + equalTo(testValueCache)); + assertThat(builtOptions.maxBatchSize(), + equalTo(10)); + assertThat(builtOptions.getStatisticsCollector(), + equalTo(testStatisticsCollectorSupplier.get())); + + } + + @Test + void canBuildViaBuilderOk() { + + DataLoaderOptions.Builder builder = DataLoaderOptions.newOptionsBuilder(); + builder.setBatchingEnabled(false); + builder.setCachingExceptionsEnabled(false); + builder.setCachingEnabled(false); + builder.setBatchLoaderScheduler(testBatchLoaderScheduler); + builder.setBatchLoaderContextProvider(testBatchLoaderContextProvider); + builder.setCacheMap(testCacheMap); + builder.setValueCache(testValueCache); + builder.setCacheKeyFunction(testCacheKey); + builder.setValueCacheOptions(testValueCacheOptions); + builder.setMaxBatchSize(10); + builder.setStatisticsCollector(testStatisticsCollectorSupplier); + + DataLoaderOptions builtOptions = builder.build(); + + assertThat(builtOptions.batchingEnabled(), + equalTo(false)); + assertThat(builtOptions.getBatchLoaderScheduler(), + equalTo(testBatchLoaderScheduler)); + assertThat(builtOptions.getBatchLoaderContextProvider(), + equalTo(testBatchLoaderContextProvider)); + assertThat(builtOptions.cacheMap().get(), + equalTo(testCacheMap)); + assertThat(builtOptions.cachingEnabled(), + equalTo(false)); + assertThat(builtOptions.getValueCacheOptions(), + equalTo(testValueCacheOptions)); + assertThat(builtOptions.cacheKeyFunction().get(), + equalTo(testCacheKey)); + assertThat(builtOptions.valueCache().get(), + equalTo(testValueCache)); + assertThat(builtOptions.maxBatchSize(), + equalTo(10)); + assertThat(builtOptions.getStatisticsCollector(), + equalTo(testStatisticsCollectorSupplier.get())); + } + + @Test + void canCopyExistingOptionValuesOnTransform() { + + DataLoaderInstrumentation instrumentation1 = new DataLoaderInstrumentation() { + }; + DataLoaderInstrumentation instrumentation2 = new DataLoaderInstrumentation() { + }; + BatchLoaderContextProvider contextProvider1 = () -> null; + + DataLoaderOptions startingOptions = DataLoaderOptions.newOptionsBuilder().setBatchingEnabled(false) + .setCachingEnabled(false) + .setInstrumentation(instrumentation1) + .setBatchLoaderContextProvider(contextProvider1) + .build(); + + assertThat(startingOptions.batchingEnabled(), equalTo(false)); + assertThat(startingOptions.cachingEnabled(), equalTo(false)); + assertThat(startingOptions.getInstrumentation(), equalTo(instrumentation1)); + assertThat(startingOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); + + DataLoaderOptions newOptions = startingOptions.transform(builder -> + builder.setBatchingEnabled(true).setInstrumentation(instrumentation2)); + + + // immutable + assertThat(newOptions, CoreMatchers.not(startingOptions)); + assertThat(startingOptions.batchingEnabled(), equalTo(false)); + assertThat(startingOptions.cachingEnabled(), equalTo(false)); + assertThat(startingOptions.getInstrumentation(), equalTo(instrumentation1)); + assertThat(startingOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); + + // stayed the same + assertThat(newOptions.cachingEnabled(), equalTo(false)); + assertThat(newOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); + + // was changed + assertThat(newOptions.batchingEnabled(), equalTo(true)); + assertThat(newOptions.getInstrumentation(), equalTo(instrumentation2)); + + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/DataLoaderTest.java b/src/test/java/org/dataloader/DataLoaderTest.java index c4ae883..069d390 100644 --- a/src/test/java/org/dataloader/DataLoaderTest.java +++ b/src/test/java/org/dataloader/DataLoaderTest.java @@ -29,10 +29,8 @@ import org.dataloader.fixtures.parameterized.TestReactiveDataLoaderFactory; import org.dataloader.impl.CompletableFutureKit; import org.dataloader.impl.DataLoaderAssertionException; -import org.junit.jupiter.api.Named; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import java.util.*; @@ -41,21 +39,14 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; -import java.util.stream.Stream; import static java.util.Arrays.asList; import static java.util.Collections.*; import static java.util.concurrent.CompletableFuture.*; import static org.awaitility.Awaitility.await; import static org.dataloader.DataLoaderFactory.newDataLoader; -import static org.dataloader.DataLoaderFactory.newMappedDataLoader; -import static org.dataloader.DataLoaderFactory.newMappedPublisherDataLoader; -import static org.dataloader.DataLoaderFactory.newMappedPublisherDataLoaderWithTry; -import static org.dataloader.DataLoaderFactory.newPublisherDataLoader; -import static org.dataloader.DataLoaderFactory.newPublisherDataLoaderWithTry; import static org.dataloader.DataLoaderOptions.newOptions; import static org.dataloader.fixtures.TestKit.areAllDone; import static org.dataloader.fixtures.TestKit.listFrom; @@ -63,7 +54,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.fail; /** * Tests for {@link DataLoader}. @@ -125,7 +115,7 @@ public void basic_map_batch_loading() { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Support_loading_multiple_keys_in_one_call_via_list(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -141,7 +131,7 @@ public void should_Support_loading_multiple_keys_in_one_call_via_list(TestDataLo } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Support_loading_multiple_keys_in_one_call_via_map(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -161,7 +151,7 @@ public void should_Support_loading_multiple_keys_in_one_call_via_map(TestDataLoa } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Resolve_to_empty_list_when_no_keys_supplied(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -176,7 +166,7 @@ public void should_Resolve_to_empty_list_when_no_keys_supplied(TestDataLoaderFac } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Resolve_to_empty_map_when_no_keys_supplied(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -191,7 +181,7 @@ public void should_Resolve_to_empty_map_when_no_keys_supplied(TestDataLoaderFact } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Return_zero_entries_dispatched_when_no_keys_supplied_via_list(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -206,7 +196,7 @@ public void should_Return_zero_entries_dispatched_when_no_keys_supplied_via_list } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Return_zero_entries_dispatched_when_no_keys_supplied_via_map(TestDataLoaderFactory factory) { AtomicBoolean success = new AtomicBoolean(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), new ArrayList<>()); @@ -221,7 +211,7 @@ public void should_Return_zero_entries_dispatched_when_no_keys_supplied_via_map( } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Batch_multiple_requests(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -237,7 +227,7 @@ public void should_Batch_multiple_requests(TestDataLoaderFactory factory) throws } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Return_number_of_batched_entries(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -252,7 +242,7 @@ public void should_Return_number_of_batched_entries(TestDataLoaderFactory factor } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Coalesce_identical_requests(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -269,7 +259,7 @@ public void should_Coalesce_identical_requests(TestDataLoaderFactory factory) th } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Cache_repeated_requests(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -305,7 +295,7 @@ public void should_Cache_repeated_requests(TestDataLoaderFactory factory) throws } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Not_redispatch_previous_load(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -323,7 +313,7 @@ public void should_Not_redispatch_previous_load(TestDataLoaderFactory factory) t } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Cache_on_redispatch(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -348,7 +338,7 @@ public void should_Cache_on_redispatch(TestDataLoaderFactory factory) throws Exe } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Clear_single_value_in_loader(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -377,7 +367,7 @@ public void should_Clear_single_value_in_loader(TestDataLoaderFactory factory) t } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Clear_all_values_in_loader(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -405,7 +395,7 @@ public void should_Clear_all_values_in_loader(TestDataLoaderFactory factory) thr } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Allow_priming_the_cache(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -424,7 +414,7 @@ public void should_Allow_priming_the_cache(TestDataLoaderFactory factory) throws } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Not_prime_keys_that_already_exist(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -453,7 +443,7 @@ public void should_Not_prime_keys_that_already_exist(TestDataLoaderFactory facto } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Allow_to_forcefully_prime_the_cache(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -482,7 +472,7 @@ public void should_Allow_to_forcefully_prime_the_cache(TestDataLoaderFactory fac } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Allow_priming_the_cache_with_a_future(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -501,7 +491,7 @@ public void should_Allow_priming_the_cache_with_a_future(TestDataLoaderFactory f } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_not_Cache_failed_fetches_on_complete_failure(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader errorLoader = factory.idLoaderBlowsUps(new DataLoaderOptions(), loadCalls); @@ -523,7 +513,7 @@ public void should_not_Cache_failed_fetches_on_complete_failure(TestDataLoaderFa } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Resolve_to_error_to_indicate_failure(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader evenLoader = factory.idLoaderOddEvenExceptions(new DataLoaderOptions(), loadCalls); @@ -546,7 +536,7 @@ public void should_Resolve_to_error_to_indicate_failure(TestDataLoaderFactory fa // Accept any kind of key. @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Represent_failures_and_successes_simultaneously(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { AtomicBoolean success = new AtomicBoolean(); List> loadCalls = new ArrayList<>(); @@ -573,7 +563,7 @@ public void should_Represent_failures_and_successes_simultaneously(TestDataLoade // Accepts options @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Cache_failed_fetches(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader errorLoader = factory.idLoaderAllExceptions(new DataLoaderOptions(), loadCalls); @@ -596,7 +586,7 @@ public void should_Cache_failed_fetches(TestDataLoaderFactory factory) { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_NOT_Cache_failed_fetches_if_told_not_too(TestDataLoaderFactory factory) { DataLoaderOptions options = DataLoaderOptions.newOptions().setCachingExceptionsEnabled(false); List> loadCalls = new ArrayList<>(); @@ -623,7 +613,7 @@ public void should_NOT_Cache_failed_fetches_if_told_not_too(TestDataLoaderFactor // Accepts object key in custom cacheKey function @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Handle_priming_the_cache_with_an_error(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -640,7 +630,7 @@ public void should_Handle_priming_the_cache_with_an_error(TestDataLoaderFactory } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Clear_values_from_cache_after_errors(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader errorLoader = factory.idLoaderBlowsUps(new DataLoaderOptions(), loadCalls); @@ -676,7 +666,7 @@ public void should_Clear_values_from_cache_after_errors(TestDataLoaderFactory fa } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Propagate_error_to_all_loads(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader errorLoader = factory.idLoaderBlowsUps(new DataLoaderOptions(), loadCalls); @@ -700,7 +690,7 @@ public void should_Propagate_error_to_all_loads(TestDataLoaderFactory factory) { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Accept_objects_as_keys(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(new DataLoaderOptions(), loadCalls); @@ -742,7 +732,7 @@ public void should_Accept_objects_as_keys(TestDataLoaderFactory factory) { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Disable_caching(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = @@ -780,7 +770,7 @@ public void should_Disable_caching(TestDataLoaderFactory factory) throws Executi } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_work_with_duplicate_keys_when_caching_disabled(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = @@ -795,7 +785,7 @@ public void should_work_with_duplicate_keys_when_caching_disabled(TestDataLoader assertThat(future1.get(), equalTo("A")); assertThat(future2.get(), equalTo("B")); assertThat(future3.get(), equalTo("A")); - if (factory instanceof MappedDataLoaderFactory || factory instanceof MappedPublisherDataLoaderFactory) { + if (factory.unwrap() instanceof MappedDataLoaderFactory || factory.unwrap() instanceof MappedPublisherDataLoaderFactory) { assertThat(loadCalls, equalTo(singletonList(asList("A", "B")))); } else { assertThat(loadCalls, equalTo(singletonList(asList("A", "B", "A")))); @@ -803,7 +793,7 @@ public void should_work_with_duplicate_keys_when_caching_disabled(TestDataLoader } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_work_with_duplicate_keys_when_caching_enabled(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = @@ -824,7 +814,7 @@ public void should_work_with_duplicate_keys_when_caching_enabled(TestDataLoaderF // It is resilient to job queue ordering @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Accept_objects_with_a_complex_key(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn()); @@ -846,7 +836,7 @@ public void should_Accept_objects_with_a_complex_key(TestDataLoaderFactory facto // Helper methods @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Clear_objects_with_complex_key(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn()); @@ -871,7 +861,7 @@ public void should_Clear_objects_with_complex_key(TestDataLoaderFactory factory) } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Accept_objects_with_different_order_of_keys(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn()); @@ -894,7 +884,7 @@ public void should_Accept_objects_with_different_order_of_keys(TestDataLoaderFac } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Allow_priming_the_cache_with_an_object_key(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setCacheKeyFunction(getJsonObjectCacheMapFn()); @@ -916,7 +906,7 @@ public void should_Allow_priming_the_cache_with_an_object_key(TestDataLoaderFact } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Accept_a_custom_cache_map_implementation(TestDataLoaderFactory factory) throws ExecutionException, InterruptedException { CustomCacheMap customMap = new CustomCacheMap(); List> loadCalls = new ArrayList<>(); @@ -968,7 +958,7 @@ public void should_Accept_a_custom_cache_map_implementation(TestDataLoaderFactor } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_degrade_gracefully_if_cache_get_throws(TestDataLoaderFactory factory) { CacheMap cache = new ThrowingCacheMap(); DataLoaderOptions options = newOptions().setCachingEnabled(true).setCacheMap(cache); @@ -983,7 +973,7 @@ public void should_degrade_gracefully_if_cache_get_throws(TestDataLoaderFactory } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void batching_disabled_should_dispatch_immediately(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setBatchingEnabled(false); @@ -1012,7 +1002,7 @@ public void batching_disabled_should_dispatch_immediately(TestDataLoaderFactory } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void batching_disabled_and_caching_disabled_should_dispatch_immediately_and_forget(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setBatchingEnabled(false).setCachingEnabled(false); @@ -1044,7 +1034,7 @@ public void batching_disabled_and_caching_disabled_should_dispatch_immediately_a } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void batches_multiple_requests_with_max_batch_size(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(newOptions().setMaxBatchSize(2), loadCalls); @@ -1066,7 +1056,7 @@ public void batches_multiple_requests_with_max_batch_size(TestDataLoaderFactory } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void can_split_max_batch_sizes_correctly(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(newOptions().setMaxBatchSize(5), loadCalls); @@ -1089,7 +1079,7 @@ public void can_split_max_batch_sizes_correctly(TestDataLoaderFactory factory) { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_Batch_loads_occurring_within_futures(TestDataLoaderFactory factory) { List> loadCalls = new ArrayList<>(); DataLoader identityLoader = factory.idLoader(newOptions(), loadCalls); @@ -1122,7 +1112,7 @@ public void should_Batch_loads_occurring_within_futures(TestDataLoaderFactory fa } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void should_blowup_after_N_keys(TestDataLoaderFactory factory) { if (!(factory instanceof TestReactiveDataLoaderFactory)) { return; @@ -1148,7 +1138,7 @@ public void should_blowup_after_N_keys(TestDataLoaderFactory factory) { } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void when_values_size_are_less_then_key_size(TestDataLoaderFactory factory) { // // what happens if we want 4 values but are only given 2 back say @@ -1162,12 +1152,12 @@ public void when_values_size_are_less_then_key_size(TestDataLoaderFactory factor await().atMost(Duration.FIVE_SECONDS).until(() -> areAllDone(cf1, cf2, cf3, cf4)); - if (factory instanceof ListDataLoaderFactory) { + if (factory.unwrap() instanceof ListDataLoaderFactory) { assertThat(cause(cf1), instanceOf(DataLoaderAssertionException.class)); assertThat(cause(cf2), instanceOf(DataLoaderAssertionException.class)); assertThat(cause(cf3), instanceOf(DataLoaderAssertionException.class)); assertThat(cause(cf4), instanceOf(DataLoaderAssertionException.class)); - } else if (factory instanceof PublisherDataLoaderFactory) { + } else if (factory.unwrap() instanceof PublisherDataLoaderFactory) { // some have completed progressively but the other never did assertThat(cf1.join(), equalTo("A")); assertThat(cf2.join(), equalTo("B")); @@ -1183,7 +1173,7 @@ public void when_values_size_are_less_then_key_size(TestDataLoaderFactory factor } @ParameterizedTest - @MethodSource("dataLoaderFactories") + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") public void when_values_size_are_more_then_key_size(TestDataLoaderFactory factory) { // // what happens if we want 4 values but only given 6 back say @@ -1197,7 +1187,7 @@ public void when_values_size_are_more_then_key_size(TestDataLoaderFactory factor await().atMost(Duration.FIVE_SECONDS).until(() -> areAllDone(cf1, cf2, cf3, cf4)); - if (factory instanceof ListDataLoaderFactory) { + if (factory.unwrap() instanceof ListDataLoaderFactory) { assertThat(cause(cf1), instanceOf(DataLoaderAssertionException.class)); assertThat(cause(cf2), instanceOf(DataLoaderAssertionException.class)); assertThat(cause(cf3), instanceOf(DataLoaderAssertionException.class)); @@ -1307,14 +1297,5 @@ public CompletableFuture get(String key) { throw new RuntimeException("Cache implementation failed."); } } - - private static Stream dataLoaderFactories() { - return Stream.of( - Arguments.of(Named.of("List DataLoader", new ListDataLoaderFactory())), - Arguments.of(Named.of("Mapped DataLoader", new MappedDataLoaderFactory())), - Arguments.of(Named.of("Publisher DataLoader", new PublisherDataLoaderFactory())), - Arguments.of(Named.of("Mapped Publisher DataLoader", new MappedPublisherDataLoaderFactory())) - ); - } } diff --git a/src/test/java/org/dataloader/DataLoaderValueCacheTest.java b/src/test/java/org/dataloader/DataLoaderValueCacheTest.java index 1fb5ea2..732febe 100644 --- a/src/test/java/org/dataloader/DataLoaderValueCacheTest.java +++ b/src/test/java/org/dataloader/DataLoaderValueCacheTest.java @@ -4,10 +4,13 @@ import com.github.benmanes.caffeine.cache.Caffeine; import org.dataloader.fixtures.CaffeineValueCache; import org.dataloader.fixtures.CustomValueCache; +import org.dataloader.fixtures.parameterized.TestDataLoaderFactory; import org.dataloader.impl.DataLoaderAssertionException; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; @@ -18,7 +21,6 @@ import static java.util.Collections.singletonList; import static org.awaitility.Awaitility.await; import static org.dataloader.DataLoaderOptions.newOptions; -import static org.dataloader.fixtures.TestKit.idLoader; import static org.dataloader.fixtures.TestKit.snooze; import static org.dataloader.fixtures.TestKit.sort; import static org.dataloader.impl.CompletableFutureKit.failedFuture; @@ -30,11 +32,12 @@ public class DataLoaderValueCacheTest { - @Test - public void test_by_default_we_have_no_value_caching() { - List> loadCalls = new ArrayList<>(); + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void test_by_default_we_have_no_value_caching(TestDataLoaderFactory factory) { + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions(); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -64,12 +67,13 @@ public void test_by_default_we_have_no_value_caching() { assertThat(loadCalls, equalTo(emptyList())); } - @Test - public void should_accept_a_remote_value_store_for_caching() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void should_accept_a_remote_value_store_for_caching(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache(); - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); // Fetches as expected @@ -108,8 +112,9 @@ public void should_accept_a_remote_value_store_for_caching() { assertArrayEquals(customValueCache.store.keySet().toArray(), emptyList().toArray()); } - @Test - public void can_use_caffeine_for_caching() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void can_use_caffeine_for_caching(TestDataLoaderFactory factory) { // // Mostly to prove that some other CACHE library could be used // as the backing value cache. Not really Caffeine specific. @@ -121,9 +126,9 @@ public void can_use_caffeine_for_caching() { ValueCache caffeineValueCache = new CaffeineValueCache(caffeineCache); - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(caffeineValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); // Fetches as expected @@ -148,8 +153,9 @@ public void can_use_caffeine_for_caching() { assertArrayEquals(caffeineCache.asMap().keySet().toArray(), asList("a", "b", "c").toArray()); } - @Test - public void will_invoke_loader_if_CACHE_GET_call_throws_exception() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void will_invoke_loader_if_CACHE_GET_call_throws_exception(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache() { @Override @@ -163,9 +169,9 @@ public CompletableFuture get(String key) { customValueCache.set("a", "Not From Cache"); customValueCache.set("b", "From Cache"); - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -178,8 +184,9 @@ public CompletableFuture get(String key) { assertThat(loadCalls, equalTo(singletonList(singletonList("a")))); } - @Test - public void will_still_work_if_CACHE_SET_call_throws_exception() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void will_still_work_if_CACHE_SET_call_throws_exception(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache() { @Override public CompletableFuture set(String key, Object value) { @@ -190,9 +197,9 @@ public CompletableFuture set(String key, Object value) { } }; - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -206,8 +213,9 @@ public CompletableFuture set(String key, Object value) { assertArrayEquals(customValueCache.store.keySet().toArray(), singletonList("b").toArray()); } - @Test - public void caching_can_take_some_time_complete() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void caching_can_take_some_time_complete(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache() { @Override @@ -228,9 +236,9 @@ public CompletableFuture get(String key) { }; - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -247,8 +255,9 @@ public CompletableFuture get(String key) { assertThat(loadCalls, equalTo(singletonList(asList("missC", "missD")))); } - @Test - public void batch_caching_works_as_expected() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void batch_caching_works_as_expected(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache() { @Override @@ -269,9 +278,9 @@ public CompletableFuture>> getValues(List keys) { }; - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -293,8 +302,9 @@ public CompletableFuture>> getValues(List keys) { assertThat(values, equalTo(asList("missC", "missD"))); } - @Test - public void assertions_will_be_thrown_if_the_cache_does_not_follow_contract() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void assertions_will_be_thrown_if_the_cache_does_not_follow_contract(TestDataLoaderFactory factory) { CustomValueCache customValueCache = new CustomValueCache() { @Override @@ -312,9 +322,9 @@ public CompletableFuture>> getValues(List keys) { } }; - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -335,8 +345,9 @@ private boolean isAssertionException(CompletableFuture fA) { } - @Test - public void if_caching_is_off_its_never_hit() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void if_caching_is_off_its_never_hit(TestDataLoaderFactory factory) { AtomicInteger getCalls = new AtomicInteger(); CustomValueCache customValueCache = new CustomValueCache() { @@ -347,9 +358,9 @@ public CompletableFuture get(String key) { } }; - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache).setCachingEnabled(false); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -368,8 +379,9 @@ public CompletableFuture get(String key) { assertTrue(customValueCache.asMap().isEmpty()); } - @Test - public void if_everything_is_cached_no_batching_happens() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void if_everything_is_cached_no_batching_happens(TestDataLoaderFactory factory) { AtomicInteger getCalls = new AtomicInteger(); AtomicInteger setCalls = new AtomicInteger(); CustomValueCache customValueCache = new CustomValueCache() { @@ -390,9 +402,9 @@ public CompletableFuture> setValues(List keys, List customValueCache.asMap().put("b", "cachedB"); customValueCache.asMap().put("c", "cachedC"); - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache).setCachingEnabled(true); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); @@ -410,8 +422,9 @@ public CompletableFuture> setValues(List keys, List } - @Test - public void if_batching_is_off_it_still_can_cache() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void if_batching_is_off_it_still_can_cache(TestDataLoaderFactory factory) { AtomicInteger getCalls = new AtomicInteger(); AtomicInteger setCalls = new AtomicInteger(); CustomValueCache customValueCache = new CustomValueCache() { @@ -430,9 +443,9 @@ public CompletableFuture> setValues(List keys, List }; customValueCache.asMap().put("a", "cachedA"); - List> loadCalls = new ArrayList<>(); + List> loadCalls = new ArrayList<>(); DataLoaderOptions options = newOptions().setValueCache(customValueCache).setCachingEnabled(true).setBatchingEnabled(false); - DataLoader identityLoader = idLoader(options, loadCalls); + DataLoader identityLoader = factory.idLoader(options, loadCalls); CompletableFuture fA = identityLoader.load("a"); CompletableFuture fB = identityLoader.load("b"); diff --git a/src/test/java/org/dataloader/DelegatingDataLoaderTest.java b/src/test/java/org/dataloader/DelegatingDataLoaderTest.java new file mode 100644 index 0000000..9103eca --- /dev/null +++ b/src/test/java/org/dataloader/DelegatingDataLoaderTest.java @@ -0,0 +1,64 @@ +package org.dataloader; + +import org.dataloader.fixtures.TestKit; +import org.dataloader.fixtures.parameterized.DelegatingDataLoaderFactory; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.concurrent.CompletableFuture; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * There are WAY more tests via the {@link DelegatingDataLoaderFactory} + * parameterized tests. All the basic {@link DataLoader} tests pass when wrapped in a {@link DelegatingDataLoader} + */ +public class DelegatingDataLoaderTest { + + @Test + void canUnwrapDataLoaders() { + DataLoader rawLoader = TestKit.idLoader(); + DataLoader delegateLoader = new DelegatingDataLoader<>(rawLoader); + + assertThat(DelegatingDataLoader.unwrap(rawLoader), is(rawLoader)); + assertThat(DelegatingDataLoader.unwrap(delegateLoader), is(rawLoader)); + } + + @Test + void canCreateAClassOk() { + DataLoader rawLoader = TestKit.idLoader(); + DelegatingDataLoader delegatingDataLoader = new DelegatingDataLoader<>(rawLoader) { + @Override + public CompletableFuture load(@NonNull String key, @Nullable Object keyContext) { + CompletableFuture cf = super.load(key, keyContext); + return cf.thenApply(v -> "|" + v + "|"); + } + }; + + assertThat(delegatingDataLoader.getDelegate(), is(rawLoader)); + + + CompletableFuture cfA = delegatingDataLoader.load("A"); + CompletableFuture cfB = delegatingDataLoader.load("B"); + CompletableFuture> cfCD = delegatingDataLoader.loadMany(List.of("C", "D")); + + CompletableFuture> dispatch = delegatingDataLoader.dispatch(); + + await().until(dispatch::isDone); + + assertThat(cfA.join(), equalTo("|A|")); + assertThat(cfB.join(), equalTo("|B|")); + assertThat(cfCD.join(), equalTo(List.of("|C|", "|D|"))); + + assertThat(delegatingDataLoader.getIfPresent("A").isEmpty(), equalTo(false)); + assertThat(delegatingDataLoader.getIfPresent("X").isEmpty(), equalTo(true)); + + assertThat(delegatingDataLoader.getIfCompleted("A").isEmpty(), equalTo(false)); + assertThat(delegatingDataLoader.getIfCompleted("X").isEmpty(), equalTo(true)); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/ValueCacheOptionsTest.java b/src/test/java/org/dataloader/ValueCacheOptionsTest.java new file mode 100644 index 0000000..469e291 --- /dev/null +++ b/src/test/java/org/dataloader/ValueCacheOptionsTest.java @@ -0,0 +1,19 @@ +package org.dataloader; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +class ValueCacheOptionsTest { + + @Test + void saneDefaults() { + ValueCacheOptions newOptions = ValueCacheOptions.newOptions(); + assertThat(newOptions.isCompleteValueAfterCacheSet(), equalTo(false)); + + ValueCacheOptions differentOptions = newOptions.setCompleteValueAfterCacheSet(true); + assertThat(differentOptions.isCompleteValueAfterCacheSet(), equalTo(true)); + assertThat(differentOptions == newOptions, equalTo(false)); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/fixtures/Stopwatch.java b/src/test/java/org/dataloader/fixtures/Stopwatch.java new file mode 100644 index 0000000..c815a8b --- /dev/null +++ b/src/test/java/org/dataloader/fixtures/Stopwatch.java @@ -0,0 +1,57 @@ +package org.dataloader.fixtures; + +import java.time.Duration; + +public class Stopwatch { + + public static Stopwatch stopwatchStarted() { + return new Stopwatch().start(); + } + + public static Stopwatch stopwatchUnStarted() { + return new Stopwatch(); + } + + private long started = -1; + private long stopped = -1; + + public Stopwatch start() { + synchronized (this) { + if (started != -1) { + throw new IllegalStateException("You have started it before"); + } + started = System.currentTimeMillis(); + } + return this; + } + + private Stopwatch() { + } + + public long elapsed() { + synchronized (this) { + if (started == -1) { + throw new IllegalStateException("You haven't started it"); + } + if (stopped == -1) { + return System.currentTimeMillis() - started; + } else { + return stopped - started; + } + } + } + + public Duration duration() { + return Duration.ofMillis(elapsed()); + } + + public Duration stop() { + synchronized (this) { + if (started != -1) { + throw new IllegalStateException("You have started it"); + } + stopped = System.currentTimeMillis(); + return duration(); + } + } +} diff --git a/src/test/java/org/dataloader/fixtures/TestKit.java b/src/test/java/org/dataloader/fixtures/TestKit.java index c22988d..04ec5e5 100644 --- a/src/test/java/org/dataloader/fixtures/TestKit.java +++ b/src/test/java/org/dataloader/fixtures/TestKit.java @@ -8,7 +8,6 @@ import org.dataloader.MappedBatchLoader; import org.dataloader.MappedBatchLoaderWithContext; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -61,43 +60,14 @@ public static BatchLoader keysAsValues(List> loadCalls) { }; } - public static BatchLoader keysAsValuesAsync(Duration delay) { - return keysAsValuesAsync(new ArrayList<>(), delay); - } - - public static BatchLoader keysAsValuesAsync(List> loadCalls, Duration delay) { - return keys -> CompletableFuture.supplyAsync(() -> { - snooze(delay.toMillis()); - List ks = new ArrayList<>(keys); - loadCalls.add(ks); - @SuppressWarnings("unchecked") - List values = keys.stream() - .map(k -> (V) k) - .collect(toList()); - return values; - }); - } - public static DataLoader idLoader() { return idLoader(null, new ArrayList<>()); } - public static DataLoader idLoader(List> loadCalls) { - return idLoader(null, loadCalls); - } - public static DataLoader idLoader(DataLoaderOptions options, List> loadCalls) { return DataLoaderFactory.newDataLoader(keysAsValues(loadCalls), options); } - public static DataLoader idLoaderAsync(Duration delay) { - return idLoaderAsync(null, new ArrayList<>(), delay); - } - - public static DataLoader idLoaderAsync(DataLoaderOptions options, List> loadCalls, Duration delay) { - return DataLoaderFactory.newDataLoader(keysAsValuesAsync(loadCalls, delay), options); - } - public static Collection listFrom(int i, int max) { List ints = new ArrayList<>(); for (int j = i; j < max; j++) { diff --git a/src/test/java/org/dataloader/fixtures/parameterized/DelegatingDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/DelegatingDataLoaderFactory.java new file mode 100644 index 0000000..0cbd3f3 --- /dev/null +++ b/src/test/java/org/dataloader/fixtures/parameterized/DelegatingDataLoaderFactory.java @@ -0,0 +1,71 @@ +package org.dataloader.fixtures.parameterized; + +import org.dataloader.DataLoader; +import org.dataloader.DataLoaderOptions; +import org.dataloader.DelegatingDataLoader; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +public class DelegatingDataLoaderFactory implements TestDataLoaderFactory { + // its delegates all the way down to the turtles + private final TestDataLoaderFactory delegateFactory; + + public DelegatingDataLoaderFactory(TestDataLoaderFactory delegateFactory) { + this.delegateFactory = delegateFactory; + } + + @Override + public String toString() { + return "DelegatingDataLoaderFactory{" + + "delegateFactory=" + delegateFactory + + '}'; + } + + @Override + public TestDataLoaderFactory unwrap() { + return delegateFactory.unwrap(); + } + + private DataLoader mkDelegateDataLoader(DataLoader dataLoader) { + return new DelegatingDataLoader<>(dataLoader); + } + + @Override + public DataLoader idLoader(DataLoaderOptions options, List> loadCalls) { + return mkDelegateDataLoader(delegateFactory.idLoader(options, loadCalls)); + } + + @Override + public DataLoader idLoaderDelayed(DataLoaderOptions options, List> loadCalls, Duration delay) { + return mkDelegateDataLoader(delegateFactory.idLoaderDelayed(options, loadCalls, delay)); + } + + @Override + public DataLoader idLoaderBlowsUps( + DataLoaderOptions options, List> loadCalls) { + return mkDelegateDataLoader(delegateFactory.idLoaderBlowsUps(options, loadCalls)); + } + + @Override + public DataLoader idLoaderAllExceptions(DataLoaderOptions options, List> loadCalls) { + return mkDelegateDataLoader(delegateFactory.idLoaderAllExceptions(options, loadCalls)); + } + + @Override + public DataLoader idLoaderOddEvenExceptions(DataLoaderOptions options, List> loadCalls) { + return mkDelegateDataLoader(delegateFactory.idLoaderOddEvenExceptions(options, loadCalls)); + } + + @Override + public DataLoader onlyReturnsNValues(int N, DataLoaderOptions options, ArrayList loadCalls) { + return mkDelegateDataLoader(delegateFactory.onlyReturnsNValues(N, options, loadCalls)); + } + + @Override + public DataLoader idLoaderReturnsTooMany(int howManyMore, DataLoaderOptions options, ArrayList loadCalls) { + return mkDelegateDataLoader(delegateFactory.idLoaderReturnsTooMany(howManyMore, options, loadCalls)); + } +} diff --git a/src/test/java/org/dataloader/fixtures/parameterized/ListDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/ListDataLoaderFactory.java index ee1f1d7..0644d3c 100644 --- a/src/test/java/org/dataloader/fixtures/parameterized/ListDataLoaderFactory.java +++ b/src/test/java/org/dataloader/fixtures/parameterized/ListDataLoaderFactory.java @@ -4,9 +4,11 @@ import org.dataloader.DataLoaderOptions; import org.dataloader.fixtures.TestKit; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import static java.util.concurrent.CompletableFuture.completedFuture; @@ -21,6 +23,15 @@ public DataLoader idLoader(DataLoaderOptions options, List DataLoader idLoaderDelayed(DataLoaderOptions options, List> loadCalls, Duration delay) { + return newDataLoader(keys -> CompletableFuture.supplyAsync(() -> { + TestKit.snooze(delay.toMillis()); + loadCalls.add(new ArrayList<>(keys)); + return keys; + })); + } + @Override public DataLoader idLoaderBlowsUps( DataLoaderOptions options, List> loadCalls) { diff --git a/src/test/java/org/dataloader/fixtures/parameterized/MappedDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/MappedDataLoaderFactory.java index 8f41441..e7c47ec 100644 --- a/src/test/java/org/dataloader/fixtures/parameterized/MappedDataLoaderFactory.java +++ b/src/test/java/org/dataloader/fixtures/parameterized/MappedDataLoaderFactory.java @@ -2,15 +2,19 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderOptions; +import org.dataloader.fixtures.TestKit; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.dataloader.DataLoaderFactory.newDataLoader; import static org.dataloader.DataLoaderFactory.newMappedDataLoader; import static org.dataloader.fixtures.TestKit.futureError; @@ -27,6 +31,18 @@ public DataLoader idLoader( }, options); } + @Override + public DataLoader idLoaderDelayed( + DataLoaderOptions options, List> loadCalls, Duration delay) { + return newMappedDataLoader(keys -> CompletableFuture.supplyAsync(() -> { + TestKit.snooze(delay.toMillis()); + loadCalls.add(new ArrayList<>(keys)); + Map map = new HashMap<>(); + keys.forEach(k -> map.put(k, k)); + return map; + })); + } + @Override public DataLoader idLoaderBlowsUps(DataLoaderOptions options, List> loadCalls) { return newMappedDataLoader((keys) -> { diff --git a/src/test/java/org/dataloader/fixtures/parameterized/MappedPublisherDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/MappedPublisherDataLoaderFactory.java index 9c92330..fa920cf 100644 --- a/src/test/java/org/dataloader/fixtures/parameterized/MappedPublisherDataLoaderFactory.java +++ b/src/test/java/org/dataloader/fixtures/parameterized/MappedPublisherDataLoaderFactory.java @@ -3,14 +3,17 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderOptions; import org.dataloader.Try; +import org.dataloader.fixtures.TestKit; import reactor.core.publisher.Flux; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -18,6 +21,7 @@ import static java.util.stream.Collectors.toSet; import static org.dataloader.DataLoaderFactory.newMappedPublisherDataLoader; import static org.dataloader.DataLoaderFactory.newMappedPublisherDataLoaderWithTry; +import static org.dataloader.DataLoaderFactory.newPublisherDataLoader; public class MappedPublisherDataLoaderFactory implements TestDataLoaderFactory, TestReactiveDataLoaderFactory { @@ -32,6 +36,20 @@ public DataLoader idLoader( }, options); } + @Override + public DataLoader idLoaderDelayed( + DataLoaderOptions options, List> loadCalls, Duration delay) { + return newMappedPublisherDataLoader((keys, subscriber) -> { + CompletableFuture.runAsync(() -> { + TestKit.snooze(delay.toMillis()); + loadCalls.add(new ArrayList<>(keys)); + Map map = new HashMap<>(); + keys.forEach(k -> map.put(k, k)); + Flux.fromIterable(map.entrySet()).subscribe(subscriber); + }); + }, options); + } + @Override public DataLoader idLoaderBlowsUps(DataLoaderOptions options, List> loadCalls) { return newMappedPublisherDataLoader((keys, subscriber) -> { diff --git a/src/test/java/org/dataloader/fixtures/parameterized/PublisherDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/PublisherDataLoaderFactory.java index d75ff38..2049719 100644 --- a/src/test/java/org/dataloader/fixtures/parameterized/PublisherDataLoaderFactory.java +++ b/src/test/java/org/dataloader/fixtures/parameterized/PublisherDataLoaderFactory.java @@ -3,13 +3,17 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderOptions; import org.dataloader.Try; +import org.dataloader.fixtures.TestKit; import reactor.core.publisher.Flux; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.stream.Stream; +import static org.dataloader.DataLoaderFactory.newDataLoader; import static org.dataloader.DataLoaderFactory.newPublisherDataLoader; import static org.dataloader.DataLoaderFactory.newPublisherDataLoaderWithTry; @@ -24,6 +28,17 @@ public DataLoader idLoader( }, options); } + @Override + public DataLoader idLoaderDelayed(DataLoaderOptions options, List> loadCalls, Duration delay) { + return newPublisherDataLoader((keys, subscriber) -> { + CompletableFuture.runAsync(() -> { + TestKit.snooze(delay.toMillis()); + loadCalls.add(new ArrayList<>(keys)); + Flux.fromIterable(keys).subscribe(subscriber); + }); + }, options); + } + @Override public DataLoader idLoaderBlowsUps(DataLoaderOptions options, List> loadCalls) { return newPublisherDataLoader((keys, subscriber) -> { diff --git a/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactories.java b/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactories.java new file mode 100644 index 0000000..48678c4 --- /dev/null +++ b/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactories.java @@ -0,0 +1,25 @@ +package org.dataloader.fixtures.parameterized; + +import org.junit.jupiter.api.Named; +import org.junit.jupiter.params.provider.Arguments; + +import java.util.stream.Stream; + +@SuppressWarnings("unused") +public class TestDataLoaderFactories { + + public static Stream get() { + return Stream.of( + Arguments.of(Named.of("List DataLoader", new ListDataLoaderFactory())), + Arguments.of(Named.of("Mapped DataLoader", new MappedDataLoaderFactory())), + Arguments.of(Named.of("Publisher DataLoader", new PublisherDataLoaderFactory())), + Arguments.of(Named.of("Mapped Publisher DataLoader", new MappedPublisherDataLoaderFactory())), + + // runs all the above via a DelegateDataLoader + Arguments.of(Named.of("Delegate List DataLoader", new DelegatingDataLoaderFactory(new ListDataLoaderFactory()))), + Arguments.of(Named.of("Delegate Mapped DataLoader", new DelegatingDataLoaderFactory(new MappedDataLoaderFactory()))), + Arguments.of(Named.of("Delegate Publisher DataLoader", new DelegatingDataLoaderFactory(new PublisherDataLoaderFactory()))), + Arguments.of(Named.of("Delegate Mapped Publisher DataLoader", new DelegatingDataLoaderFactory(new MappedPublisherDataLoaderFactory()))) + ); + } +} diff --git a/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactory.java b/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactory.java index 8c1bc22..789b136 100644 --- a/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactory.java +++ b/src/test/java/org/dataloader/fixtures/parameterized/TestDataLoaderFactory.java @@ -3,6 +3,7 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderOptions; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -10,6 +11,8 @@ public interface TestDataLoaderFactory { DataLoader idLoader(DataLoaderOptions options, List> loadCalls); + DataLoader idLoaderDelayed(DataLoaderOptions options, List> loadCalls, Duration delay); + DataLoader idLoaderBlowsUps(DataLoaderOptions options, List> loadCalls); DataLoader idLoaderAllExceptions(DataLoaderOptions options, List> loadCalls); @@ -19,4 +22,25 @@ public interface TestDataLoaderFactory { DataLoader onlyReturnsNValues(int N, DataLoaderOptions options, ArrayList loadCalls); DataLoader idLoaderReturnsTooMany(int howManyMore, DataLoaderOptions options, ArrayList loadCalls); + + // Convenience methods + + default DataLoader idLoader(DataLoaderOptions options) { + return idLoader(options, new ArrayList<>()); + } + + default DataLoader idLoader(List> calls) { + return idLoader(null, calls); + } + default DataLoader idLoader() { + return idLoader(null, new ArrayList<>()); + } + + default DataLoader idLoaderDelayed(Duration delay) { + return idLoaderDelayed(null, new ArrayList<>(), delay); + } + + default TestDataLoaderFactory unwrap() { + return this; + } } diff --git a/src/test/java/org/dataloader/instrumentation/CapturingInstrumentation.java b/src/test/java/org/dataloader/instrumentation/CapturingInstrumentation.java new file mode 100644 index 0000000..b11bc27 --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/CapturingInstrumentation.java @@ -0,0 +1,83 @@ +package org.dataloader.instrumentation; + +import org.dataloader.BatchLoaderEnvironment; +import org.dataloader.DataLoader; +import org.dataloader.DispatchResult; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +class CapturingInstrumentation implements DataLoaderInstrumentation { + protected String name; + protected List methods = new ArrayList<>(); + + public CapturingInstrumentation(String name) { + this.name = name; + } + + public String getName() { + return name; + } + + public List methods() { + return methods; + } + + public List notLoads() { + return methods.stream().filter(method -> !method.contains("beginLoad")).collect(Collectors.toList()); + } + + public List onlyLoads() { + return methods.stream().filter(method -> method.contains("beginLoad")).collect(Collectors.toList()); + } + + + @Override + public DataLoaderInstrumentationContext beginLoad(DataLoader dataLoader, Object key, Object loadContext) { + methods.add(name + "_beginLoad" +"_k:" + key); + return new DataLoaderInstrumentationContext<>() { + @Override + public void onDispatched() { + methods.add(name + "_beginLoad_onDispatched"+"_k:" + key); + } + + @Override + public void onCompleted(Object result, Throwable t) { + methods.add(name + "_beginLoad_onCompleted"+"_k:" + key); + } + }; + } + + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + methods.add(name + "_beginDispatch"); + return new DataLoaderInstrumentationContext<>() { + @Override + public void onDispatched() { + methods.add(name + "_beginDispatch_onDispatched"); + } + + @Override + public void onCompleted(DispatchResult result, Throwable t) { + methods.add(name + "_beginDispatch_onCompleted"); + } + }; + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + methods.add(name + "_beginBatchLoader"); + return new DataLoaderInstrumentationContext<>() { + @Override + public void onDispatched() { + methods.add(name + "_beginBatchLoader_onDispatched"); + } + + @Override + public void onCompleted(List result, Throwable t) { + methods.add(name + "_beginBatchLoader_onCompleted"); + } + }; + } +} diff --git a/src/test/java/org/dataloader/instrumentation/CapturingInstrumentationReturnsNull.java b/src/test/java/org/dataloader/instrumentation/CapturingInstrumentationReturnsNull.java new file mode 100644 index 0000000..4d2f0f4 --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/CapturingInstrumentationReturnsNull.java @@ -0,0 +1,32 @@ +package org.dataloader.instrumentation; + +import org.dataloader.BatchLoaderEnvironment; +import org.dataloader.DataLoader; +import org.dataloader.DispatchResult; + +import java.util.List; + +class CapturingInstrumentationReturnsNull extends CapturingInstrumentation { + + public CapturingInstrumentationReturnsNull(String name) { + super(name); + } + + @Override + public DataLoaderInstrumentationContext beginLoad(DataLoader dataLoader, Object key, Object loadContext) { + methods.add(name + "_beginLoad" +"_k:" + key); + return null; + } + + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + methods.add(name + "_beginDispatch"); + return null; + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + methods.add(name + "_beginBatchLoader"); + return null; + } +} diff --git a/src/test/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentationTest.java b/src/test/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentationTest.java new file mode 100644 index 0000000..0d5ddb1 --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/ChainedDataLoaderInstrumentationTest.java @@ -0,0 +1,130 @@ +package org.dataloader.instrumentation; + +import org.dataloader.DataLoader; +import org.dataloader.DataLoaderFactory; +import org.dataloader.DataLoaderOptions; +import org.dataloader.fixtures.TestKit; +import org.dataloader.fixtures.parameterized.TestDataLoaderFactory; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.concurrent.CompletableFuture; + +import static org.awaitility.Awaitility.await; +import static org.dataloader.DataLoaderOptions.newOptionsBuilder; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +public class ChainedDataLoaderInstrumentationTest { + + CapturingInstrumentation capturingA; + CapturingInstrumentation capturingB; + CapturingInstrumentation capturingButReturnsNull; + + + @BeforeEach + void setUp() { + capturingA = new CapturingInstrumentation("A"); + capturingB = new CapturingInstrumentation("B"); + capturingButReturnsNull = new CapturingInstrumentationReturnsNull("NULL"); + } + + @Test + void canChainTogetherZeroInstrumentation() { + // just to prove its useless but harmless + ChainedDataLoaderInstrumentation chainedItn = new ChainedDataLoaderInstrumentation(); + + DataLoaderOptions options = newOptionsBuilder().setInstrumentation(chainedItn).build(); + + DataLoader dl = DataLoaderFactory.newDataLoader(TestKit.keysAsValues(), options); + + dl.load("A"); + dl.load("B"); + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + assertThat(dispatch.join(), equalTo(List.of("A", "B"))); + } + + @Test + void canChainTogetherOneInstrumentation() { + CapturingInstrumentation capturingA = new CapturingInstrumentation("A"); + + ChainedDataLoaderInstrumentation chainedItn = new ChainedDataLoaderInstrumentation() + .add(capturingA); + + DataLoaderOptions options = newOptionsBuilder().setInstrumentation(chainedItn).build(); + + DataLoader dl = DataLoaderFactory.newDataLoader(TestKit.keysAsValues(), options); + + dl.load("X"); + dl.load("Y"); + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + + assertThat(capturingA.notLoads(), equalTo(List.of("A_beginDispatch", + "A_beginBatchLoader", "A_beginBatchLoader_onDispatched", "A_beginBatchLoader_onCompleted", + "A_beginDispatch_onDispatched", "A_beginDispatch_onCompleted"))); + + assertThat(capturingA.onlyLoads(), equalTo(List.of( + "A_beginLoad_k:X", "A_beginLoad_onDispatched_k:X", "A_beginLoad_k:Y", "A_beginLoad_onDispatched_k:Y", + "A_beginLoad_onCompleted_k:X", "A_beginLoad_onCompleted_k:Y" + ))); + } + + + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void canChainTogetherManyInstrumentationsWithDifferentBatchLoaders(TestDataLoaderFactory factory) { + + ChainedDataLoaderInstrumentation chainedItn = new ChainedDataLoaderInstrumentation() + .add(capturingA) + .add(capturingB) + .add(capturingButReturnsNull); + + DataLoaderOptions options = newOptionsBuilder().setInstrumentation(chainedItn).build(); + + DataLoader dl = factory.idLoader(options); + + dl.load("X"); + dl.load("Y"); + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + + // + // A_beginBatchLoader happens before A_beginDispatch_onDispatched because these are sync + // and no async - a batch scheduler or async batch loader would change that + // + assertThat(capturingA.notLoads(), equalTo(List.of("A_beginDispatch", + "A_beginBatchLoader", "A_beginBatchLoader_onDispatched", "A_beginBatchLoader_onCompleted", + "A_beginDispatch_onDispatched", "A_beginDispatch_onCompleted"))); + + assertThat(capturingA.onlyLoads(), equalTo(List.of( + "A_beginLoad_k:X", "A_beginLoad_onDispatched_k:X", "A_beginLoad_k:Y", "A_beginLoad_onDispatched_k:Y", + "A_beginLoad_onCompleted_k:X", "A_beginLoad_onCompleted_k:Y" + ))); + + assertThat(capturingB.notLoads(), equalTo(List.of("B_beginDispatch", + "B_beginBatchLoader", "B_beginBatchLoader_onDispatched", "B_beginBatchLoader_onCompleted", + "B_beginDispatch_onDispatched", "B_beginDispatch_onCompleted"))); + + // it returned null on all its contexts - nothing to call back on + assertThat(capturingButReturnsNull.notLoads(), equalTo(List.of("NULL_beginDispatch", "NULL_beginBatchLoader"))); + } + + @Test + void addition_works() { + ChainedDataLoaderInstrumentation chainedItn = new ChainedDataLoaderInstrumentation() + .add(capturingA).prepend(capturingB).addAll(List.of(capturingButReturnsNull)); + + assertThat(chainedItn.getInstrumentations(), equalTo(List.of(capturingB, capturingA, capturingButReturnsNull))); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/instrumentation/DataLoaderInstrumentationTest.java b/src/test/java/org/dataloader/instrumentation/DataLoaderInstrumentationTest.java new file mode 100644 index 0000000..97f21d3 --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/DataLoaderInstrumentationTest.java @@ -0,0 +1,171 @@ +package org.dataloader.instrumentation; + +import org.dataloader.BatchLoader; +import org.dataloader.BatchLoaderEnvironment; +import org.dataloader.DataLoader; +import org.dataloader.DataLoaderFactory; +import org.dataloader.DataLoaderOptions; +import org.dataloader.DispatchResult; +import org.dataloader.fixtures.Stopwatch; +import org.dataloader.fixtures.TestKit; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; + +public class DataLoaderInstrumentationTest { + + BatchLoader snoozingBatchLoader = keys -> CompletableFuture.supplyAsync(() -> { + TestKit.snooze(100); + return keys; + }); + + @Test + void canMonitorLoading() { + AtomicReference> dlRef = new AtomicReference<>(); + + CapturingInstrumentation instrumentation = new CapturingInstrumentation("x") { + + @Override + public DataLoaderInstrumentationContext beginLoad(DataLoader dataLoader, Object key, Object loadContext) { + DataLoaderInstrumentationContext superCtx = super.beginLoad(dataLoader, key, loadContext); + dlRef.set(dataLoader); + return superCtx; + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + return DataLoaderInstrumentationHelper.noOpCtx(); + } + }; + + DataLoaderOptions options = new DataLoaderOptions() + .setInstrumentation(instrumentation) + .setMaxBatchSize(5); + + DataLoader dl = DataLoaderFactory.newDataLoader(snoozingBatchLoader, options); + + List keys = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + String key = "X" + i; + keys.add(key); + dl.load(key); + } + + // load a key that is cached + dl.load("X0"); + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + assertThat(dlRef.get(), is(dl)); + assertThat(dispatch.join(), equalTo(keys)); + + // the batch loading means they start and are instrumentation dispatched before they all end up completing + assertThat(instrumentation.onlyLoads(), + equalTo(List.of( + "x_beginLoad_k:X0", "x_beginLoad_onDispatched_k:X0", + "x_beginLoad_k:X1", "x_beginLoad_onDispatched_k:X1", + "x_beginLoad_k:X2", "x_beginLoad_onDispatched_k:X2", + "x_beginLoad_k:X0", "x_beginLoad_onDispatched_k:X0", // second cached call counts + "x_beginLoad_onCompleted_k:X0", + "x_beginLoad_onCompleted_k:X0", // each load call counts + "x_beginLoad_onCompleted_k:X1", "x_beginLoad_onCompleted_k:X2"))); + + } + + + @Test + void canMonitorDispatching() { + Stopwatch stopwatch = Stopwatch.stopwatchUnStarted(); + AtomicReference> dlRef = new AtomicReference<>(); + + DataLoaderInstrumentation instrumentation = new DataLoaderInstrumentation() { + + @Override + public DataLoaderInstrumentationContext> beginDispatch(DataLoader dataLoader) { + dlRef.set(dataLoader); + stopwatch.start(); + return new DataLoaderInstrumentationContext<>() { + @Override + public void onCompleted(DispatchResult result, Throwable t) { + stopwatch.stop(); + } + }; + } + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + return DataLoaderInstrumentationHelper.noOpCtx(); + } + }; + + DataLoaderOptions options = new DataLoaderOptions() + .setInstrumentation(instrumentation) + .setMaxBatchSize(5); + + DataLoader dl = DataLoaderFactory.newDataLoader(snoozingBatchLoader, options); + + List keys = new ArrayList<>(); + for (int i = 0; i < 20; i++) { + String key = "X" + i; + keys.add(key); + dl.load(key); + } + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + // we must have called batch load 4 times at 100ms snooze per call + // but its in parallel via supplyAsync + assertThat(stopwatch.elapsed(), greaterThan(75L)); + assertThat(dlRef.get(), is(dl)); + assertThat(dispatch.join(), equalTo(keys)); + } + + @Test + void canMonitorBatchLoading() { + Stopwatch stopwatch = Stopwatch.stopwatchUnStarted(); + AtomicReference beRef = new AtomicReference<>(); + AtomicReference> dlRef = new AtomicReference<>(); + + DataLoaderInstrumentation instrumentation = new DataLoaderInstrumentation() { + + @Override + public DataLoaderInstrumentationContext> beginBatchLoader(DataLoader dataLoader, List keys, BatchLoaderEnvironment environment) { + dlRef.set(dataLoader); + beRef.set(environment); + + stopwatch.start(); + return new DataLoaderInstrumentationContext<>() { + @Override + public void onCompleted(List result, Throwable t) { + stopwatch.stop(); + } + }; + } + }; + + DataLoaderOptions options = new DataLoaderOptions().setInstrumentation(instrumentation); + DataLoader dl = DataLoaderFactory.newDataLoader(snoozingBatchLoader, options); + + dl.load("A", "kcA"); + dl.load("B", "kcB"); + + CompletableFuture> dispatch = dl.dispatch(); + + await().until(dispatch::isDone); + assertThat(stopwatch.elapsed(), greaterThan(50L)); + assertThat(dlRef.get(), is(dl)); + assertThat(beRef.get().getKeyContexts().keySet(), equalTo(Set.of("A", "B"))); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/instrumentation/DataLoaderRegistryInstrumentationTest.java b/src/test/java/org/dataloader/instrumentation/DataLoaderRegistryInstrumentationTest.java new file mode 100644 index 0000000..49ccf0e --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/DataLoaderRegistryInstrumentationTest.java @@ -0,0 +1,231 @@ +package org.dataloader.instrumentation; + +import org.dataloader.DataLoader; +import org.dataloader.DataLoaderOptions; +import org.dataloader.DataLoaderRegistry; +import org.dataloader.fixtures.TestKit; +import org.dataloader.fixtures.parameterized.TestDataLoaderFactory; +import org.dataloader.registries.ScheduledDataLoaderRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; + +public class DataLoaderRegistryInstrumentationTest { + DataLoader dlX; + DataLoader dlY; + DataLoader dlZ; + + CapturingInstrumentation instrA; + CapturingInstrumentation instrB; + ChainedDataLoaderInstrumentation chainedInstrA; + ChainedDataLoaderInstrumentation chainedInstrB; + + @BeforeEach + void setUp() { + dlX = TestKit.idLoader(); + dlY = TestKit.idLoader(); + dlZ = TestKit.idLoader(); + instrA = new CapturingInstrumentation("A"); + instrB = new CapturingInstrumentation("B"); + chainedInstrA = new ChainedDataLoaderInstrumentation().add(instrA); + chainedInstrB = new ChainedDataLoaderInstrumentation().add(instrB); + } + + @Test + void canInstrumentRegisteredDLsViaBuilder() { + + assertThat(dlX.getOptions().getInstrumentation(), equalTo(DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION)); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(chainedInstrA) + .register("X", dlX) + .register("Y", dlY) + .register("Z", dlZ) + .build(); + + assertThat(registry.getInstrumentation(), equalTo(chainedInstrA)); + + for (String key : List.of("X", "Y", "Z")) { + DataLoaderInstrumentation instrumentation = registry.getDataLoader(key).getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + assertThat(instrumentations, equalTo(List.of(instrA))); + } + } + + @Test + void canInstrumentRegisteredDLsViaBuilderCombined() { + + DataLoaderRegistry registry1 = DataLoaderRegistry.newRegistry() + .register("X", dlX) + .register("Y", dlY) + .build(); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(chainedInstrA) + .register("Z", dlZ) + .registerAll(registry1) + .build(); + + for (String key : List.of("X", "Y", "Z")) { + DataLoaderInstrumentation instrumentation = registry.getDataLoader(key).getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + assertThat(instrumentations, equalTo(List.of(instrA))); + } + } + + @Test + void canInstrumentViaMutativeRegistration() { + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(chainedInstrA) + .build(); + + registry.register("X", dlX); + registry.computeIfAbsent("Y", l -> dlY); + registry.computeIfAbsent("Z", l -> dlZ); + + for (String key : List.of("X", "Y", "Z")) { + DataLoaderInstrumentation instrumentation = registry.getDataLoader(key).getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + assertThat(instrumentations, equalTo(List.of(instrA))); + } + } + + @Test + void wontDoAnyThingIfThereIsNoRegistryInstrumentation() { + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .register("X", dlX) + .register("Y", dlY) + .register("Z", dlZ) + .build(); + + for (String key : List.of("X", "Y", "Z")) { + DataLoaderInstrumentation instrumentation = registry.getDataLoader(key).getOptions().getInstrumentation(); + assertThat(instrumentation, equalTo(DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION)); + } + } + + @Test + void wontDoAnyThingIfThereTheyAreTheSameInstrumentationAlready() { + DataLoader newX = dlX.transform(builder -> builder.options(dlX.getOptions().setInstrumentation(instrA))); + DataLoader newY = dlX.transform(builder -> builder.options(dlY.getOptions().setInstrumentation(instrA))); + DataLoader newZ = dlX.transform(builder -> builder.options(dlZ.getOptions().setInstrumentation(instrA))); + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(instrA) + .register("X", newX) + .register("Y", newY) + .register("Z", newZ) + .build(); + + Map> dls = Map.of("X", newX, "Y", newY, "Z", newZ); + + assertThat(registry.getInstrumentation(), equalTo(instrA)); + + for (String key : List.of("X", "Y", "Z")) { + DataLoader dataLoader = registry.getDataLoader(key); + DataLoaderInstrumentation instrumentation = dataLoader.getOptions().getInstrumentation(); + assertThat(instrumentation, equalTo(instrA)); + // it's the same DL - it's not changed because it has the same instrumentation + assertThat(dls.get(key), equalTo(dataLoader)); + } + } + + @Test + void ifTheDLHasAInstrumentationThenItsTurnedIntoAChainedOne() { + DataLoaderOptions options = dlX.getOptions().setInstrumentation(instrA); + DataLoader newX = dlX.transform(builder -> builder.options(options)); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(instrB) + .register("X", newX) + .build(); + + DataLoader dataLoader = registry.getDataLoader("X"); + DataLoaderInstrumentation instrumentation = dataLoader.getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + // it gets turned into a chained one and the registry one goes first + assertThat(instrumentations, equalTo(List.of(instrB, instrA))); + } + + @Test + void chainedInstrumentationsWillBeCombined() { + DataLoaderOptions options = dlX.getOptions().setInstrumentation(chainedInstrB); + DataLoader newX = dlX.transform(builder -> builder.options(options)); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(instrA) + .register("X", newX) + .build(); + + DataLoader dataLoader = registry.getDataLoader("X"); + DataLoaderInstrumentation instrumentation = dataLoader.getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + // it gets turned into a chained one and the registry one goes first + assertThat(instrumentations, equalTo(List.of(instrA, instrB))); + } + + @SuppressWarnings("resource") + @Test + void canInstrumentScheduledRegistryViaBuilder() { + + assertThat(dlX.getOptions().getInstrumentation(), equalTo(DataLoaderInstrumentationHelper.NOOP_INSTRUMENTATION)); + + ScheduledDataLoaderRegistry registry = ScheduledDataLoaderRegistry.newScheduledRegistry() + .instrumentation(chainedInstrA) + .register("X", dlX) + .register("Y", dlY) + .register("Z", dlZ) + .build(); + + assertThat(registry.getInstrumentation(), equalTo(chainedInstrA)); + + for (String key : List.of("X", "Y", "Z")) { + DataLoaderInstrumentation instrumentation = registry.getDataLoader(key).getOptions().getInstrumentation(); + assertThat(instrumentation, instanceOf(ChainedDataLoaderInstrumentation.class)); + List instrumentations = ((ChainedDataLoaderInstrumentation) instrumentation).getInstrumentations(); + assertThat(instrumentations, equalTo(List.of(instrA))); + } + } + + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void endToEndIntegrationTest(TestDataLoaderFactory factory) { + DataLoader dl = factory.idLoader(); + + DataLoaderRegistry registry = DataLoaderRegistry.newRegistry() + .instrumentation(instrA) + .register("X", dl) + .build(); + + // since the data-loader changed when registered you MUST get the data loader from the registry + // not direct to the old one + DataLoader dataLoader = registry.getDataLoader("X"); + CompletableFuture loadA = dataLoader.load("A"); + + registry.dispatchAll(); + + await().until(loadA::isDone); + assertThat(loadA.join(), equalTo("A")); + + assertThat(instrA.notLoads(), equalTo(List.of("A_beginDispatch", + "A_beginBatchLoader", "A_beginBatchLoader_onDispatched", "A_beginBatchLoader_onCompleted", + "A_beginDispatch_onDispatched", "A_beginDispatch_onCompleted"))); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContextTest.java b/src/test/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContextTest.java new file mode 100644 index 0000000..38328eb --- /dev/null +++ b/src/test/java/org/dataloader/instrumentation/SimpleDataLoaderInstrumentationContextTest.java @@ -0,0 +1,49 @@ +package org.dataloader.instrumentation; + +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.nullValue; + +public class SimpleDataLoaderInstrumentationContextTest { + + @Test + void canRunCompletedCodeAsExpected() { + AtomicReference actual = new AtomicReference<>(); + AtomicReference actualErr = new AtomicReference<>(); + + DataLoaderInstrumentationContext ctx = DataLoaderInstrumentationHelper.whenCompleted((r, err) -> { + actualErr.set(err); + actual.set(r); + }); + + ctx.onDispatched(); // nothing happens + assertThat(actual.get(), nullValue()); + assertThat(actualErr.get(), nullValue()); + + ctx.onCompleted("X", null); + assertThat(actual.get(), Matchers.equalTo("X")); + assertThat(actualErr.get(), nullValue()); + + ctx.onCompleted(null, new RuntimeException()); + assertThat(actual.get(), nullValue()); + assertThat(actualErr.get(), Matchers.instanceOf(RuntimeException.class)); + } + + @Test + void canRunOnDispatchCodeAsExpected() { + AtomicBoolean dispatchedCalled = new AtomicBoolean(); + + DataLoaderInstrumentationContext ctx = DataLoaderInstrumentationHelper.whenDispatched(() -> dispatchedCalled.set(true)); + + ctx.onCompleted("X", null); // nothing happens + assertThat(dispatchedCalled.get(), Matchers.equalTo(false)); + + ctx.onDispatched(); + assertThat(dispatchedCalled.get(), Matchers.equalTo(true)); + } +} \ No newline at end of file diff --git a/src/test/java/org/dataloader/registries/ScheduledDataLoaderRegistryTest.java b/src/test/java/org/dataloader/registries/ScheduledDataLoaderRegistryTest.java index e82205d..e89939c 100644 --- a/src/test/java/org/dataloader/registries/ScheduledDataLoaderRegistryTest.java +++ b/src/test/java/org/dataloader/registries/ScheduledDataLoaderRegistryTest.java @@ -2,13 +2,15 @@ import org.awaitility.core.ConditionTimeoutException; import org.dataloader.DataLoader; -import org.dataloader.DataLoaderFactory; import org.dataloader.DataLoaderRegistry; -import org.dataloader.fixtures.TestKit; +import org.dataloader.fixtures.parameterized.TestDataLoaderFactory; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import java.time.Duration; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executors; @@ -20,7 +22,6 @@ import static java.util.Collections.singletonList; import static org.awaitility.Awaitility.await; import static org.awaitility.Duration.TWO_SECONDS; -import static org.dataloader.fixtures.TestKit.keysAsValues; import static org.dataloader.fixtures.TestKit.snooze; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -36,17 +37,18 @@ public class ScheduledDataLoaderRegistryTest { DispatchPredicate neverDispatch = (key, dl) -> false; - @Test - public void basic_setup_works_like_a_normal_dlr() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void basic_setup_works_like_a_normal_dlr(TestDataLoaderFactory factory) { - List> aCalls = new ArrayList<>(); - List> bCalls = new ArrayList<>(); + List> aCalls = new ArrayList<>(); + List> bCalls = new ArrayList<>(); - DataLoader dlA = TestKit.idLoader(aCalls); + DataLoader dlA = factory.idLoader(aCalls); dlA.load("AK1"); dlA.load("AK2"); - DataLoader dlB = TestKit.idLoader(bCalls); + DataLoader dlB = factory.idLoader(bCalls); dlB.load("BK1"); dlB.load("BK2"); @@ -68,11 +70,12 @@ public void basic_setup_works_like_a_normal_dlr() { assertThat(bCalls, equalTo(singletonList(asList("BK1", "BK2")))); } - @Test - public void predicate_always_false() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void predicate_always_false(TestDataLoaderFactory factory) { - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); dlA.load("K1"); dlA.load("K2"); @@ -98,15 +101,16 @@ public void predicate_always_false() { assertThat(calls.size(), equalTo(0)); } - @Test - public void predicate_that_eventually_returns_true() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void predicate_that_eventually_returns_true(TestDataLoaderFactory factory) { AtomicInteger counter = new AtomicInteger(); DispatchPredicate neverDispatch = (key, dl) -> counter.incrementAndGet() > 5; - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); CompletableFuture p1 = dlA.load("K1"); CompletableFuture p2 = dlA.load("K2"); @@ -130,10 +134,11 @@ public void predicate_that_eventually_returns_true() { assertTrue(p2.isDone()); } - @Test - public void dispatchAllWithCountImmediately() { - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void dispatchAllWithCountImmediately(TestDataLoaderFactory factory) { + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); dlA.load("K1"); dlA.load("K2"); @@ -148,10 +153,11 @@ public void dispatchAllWithCountImmediately() { assertThat(calls, equalTo(singletonList(asList("K1", "K2")))); } - @Test - public void dispatchAllImmediately() { - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void dispatchAllImmediately(TestDataLoaderFactory factory) { + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); dlA.load("K1"); dlA.load("K2"); @@ -165,13 +171,14 @@ public void dispatchAllImmediately() { assertThat(calls, equalTo(singletonList(asList("K1", "K2")))); } - @Test - public void rescheduleNow() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void rescheduleNow(TestDataLoaderFactory factory) { AtomicInteger i = new AtomicInteger(); DispatchPredicate countingPredicate = (dataLoaderKey, dataLoader) -> i.incrementAndGet() > 5; - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); dlA.load("K1"); dlA.load("K2"); @@ -189,13 +196,14 @@ public void rescheduleNow() { assertThat(calls, equalTo(singletonList(asList("K1", "K2")))); } - @Test - public void it_will_take_out_the_schedule_once_it_dispatches() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void it_will_take_out_the_schedule_once_it_dispatches(TestDataLoaderFactory factory) { AtomicInteger counter = new AtomicInteger(); DispatchPredicate countingPredicate = (dataLoaderKey, dataLoader) -> counter.incrementAndGet() > 5; - List> calls = new ArrayList<>(); - DataLoader dlA = DataLoaderFactory.newDataLoader(keysAsValues(calls)); + List> calls = new ArrayList<>(); + DataLoader dlA = factory.idLoader(calls); dlA.load("K1"); dlA.load("K2"); @@ -231,15 +239,16 @@ public void it_will_take_out_the_schedule_once_it_dispatches() { assertThat(calls, equalTo(asList(asList("K1", "K2"), asList("K3", "K4")))); } - @Test - public void close_is_a_one_way_door() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void close_is_a_one_way_door(TestDataLoaderFactory factory) { AtomicInteger counter = new AtomicInteger(); DispatchPredicate countingPredicate = (dataLoaderKey, dataLoader) -> { counter.incrementAndGet(); return false; }; - DataLoader dlA = TestKit.idLoader(); + DataLoader dlA = factory.idLoader(); dlA.load("K1"); dlA.load("K2"); @@ -276,12 +285,13 @@ public void close_is_a_one_way_door() { assertEquals(counter.get(), countThen + 1); } - @Test - public void can_tick_after_first_dispatch_for_chain_data_loaders() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void can_tick_after_first_dispatch_for_chain_data_loaders(TestDataLoaderFactory factory) { // delays much bigger than the tick rate will mean multiple calls to dispatch - DataLoader dlA = TestKit.idLoaderAsync(Duration.ofMillis(100)); - DataLoader dlB = TestKit.idLoaderAsync(Duration.ofMillis(200)); + DataLoader dlA = factory.idLoaderDelayed(Duration.ofMillis(100)); + DataLoader dlB = factory.idLoaderDelayed(Duration.ofMillis(200)); CompletableFuture chainedCF = dlA.load("AK1").thenCompose(dlB::load); @@ -306,12 +316,13 @@ public void can_tick_after_first_dispatch_for_chain_data_loaders() { registry.close(); } - @Test - public void chain_data_loaders_will_hang_if_not_in_ticker_mode() { + @ParameterizedTest + @MethodSource("org.dataloader.fixtures.parameterized.TestDataLoaderFactories#get") + public void chain_data_loaders_will_hang_if_not_in_ticker_mode(TestDataLoaderFactory factory) { // delays much bigger than the tick rate will mean multiple calls to dispatch - DataLoader dlA = TestKit.idLoaderAsync(Duration.ofMillis(100)); - DataLoader dlB = TestKit.idLoaderAsync(Duration.ofMillis(200)); + DataLoader dlA = factory.idLoaderDelayed(Duration.ofMillis(100)); + DataLoader dlB = factory.idLoaderDelayed(Duration.ofMillis(200)); CompletableFuture chainedCF = dlA.load("AK1").thenCompose(dlB::load);