From 893bb0d4b1ae1c918ac94637f53236b699b327ad Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 1 Apr 2025 18:09:00 +1100 Subject: [PATCH 1/3] Fixed a bug that the Spring team found --- .../org/dataloader/DataLoaderOptions.java | 2 +- .../org/dataloader/DataLoaderOptionsTest.java | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/dataloader/DataLoaderOptions.java b/src/main/java/org/dataloader/DataLoaderOptions.java index e4b286a..8667943 100644 --- a/src/main/java/org/dataloader/DataLoaderOptions.java +++ b/src/main/java/org/dataloader/DataLoaderOptions.java @@ -139,7 +139,7 @@ public static DataLoaderOptions.Builder newDataLoaderOptions(DataLoaderOptions o * @return a new {@link DataLoaderOptions} object */ public DataLoaderOptions transform(Consumer builderConsumer) { - Builder builder = newOptionsBuilder(); + Builder builder = newDataLoaderOptions(this); builderConsumer.accept(builder); return builder.build(); } diff --git a/src/test/java/org/dataloader/DataLoaderOptionsTest.java b/src/test/java/org/dataloader/DataLoaderOptionsTest.java index f6e06e8..6c012d8 100644 --- a/src/test/java/org/dataloader/DataLoaderOptionsTest.java +++ b/src/test/java/org/dataloader/DataLoaderOptionsTest.java @@ -2,9 +2,11 @@ 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; @@ -184,4 +186,39 @@ void canBuildViaBuilderOk() { assertThat(builtOptions.getStatisticsCollector(), equalTo(testStatisticsCollectorSupplier.get())); } + + @Test + void canCopyExistingOptionValuesOnTransform() { + + DataLoaderInstrumentation instrumentation1 = 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)); + + + // 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)); + + // copied values + assertThat(newOptions.batchingEnabled(), equalTo(true)); + assertThat(newOptions.cachingEnabled(), equalTo(false)); + assertThat(newOptions.getInstrumentation(), equalTo(instrumentation1)); + assertThat(newOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); + } } \ No newline at end of file From 5d0a2023caf3bd0e469f9a029ae4910cc7643a25 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 1 Apr 2025 18:11:29 +1100 Subject: [PATCH 2/3] Fixed a bug that the Spring team found - tweak --- .../java/org/dataloader/DataLoaderOptionsTest.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/dataloader/DataLoaderOptionsTest.java b/src/test/java/org/dataloader/DataLoaderOptionsTest.java index 6c012d8..b4ebb9e 100644 --- a/src/test/java/org/dataloader/DataLoaderOptionsTest.java +++ b/src/test/java/org/dataloader/DataLoaderOptionsTest.java @@ -192,6 +192,8 @@ void canCopyExistingOptionValuesOnTransform() { DataLoaderInstrumentation instrumentation1 = new DataLoaderInstrumentation() { }; + DataLoaderInstrumentation instrumentation2 = new DataLoaderInstrumentation() { + }; BatchLoaderContextProvider contextProvider1 = () -> null; DataLoaderOptions startingOptions = DataLoaderOptions.newOptionsBuilder().setBatchingEnabled(false) @@ -205,7 +207,8 @@ void canCopyExistingOptionValuesOnTransform() { assertThat(startingOptions.getInstrumentation(), equalTo(instrumentation1)); assertThat(startingOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); - DataLoaderOptions newOptions = startingOptions.transform(builder -> builder.setBatchingEnabled(true)); + DataLoaderOptions newOptions = startingOptions.transform(builder -> + builder.setBatchingEnabled(true).setInstrumentation(instrumentation2)); // immutable @@ -215,10 +218,13 @@ void canCopyExistingOptionValuesOnTransform() { assertThat(startingOptions.getInstrumentation(), equalTo(instrumentation1)); assertThat(startingOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); - // copied values - assertThat(newOptions.batchingEnabled(), equalTo(true)); + // stayed the same assertThat(newOptions.cachingEnabled(), equalTo(false)); - assertThat(newOptions.getInstrumentation(), equalTo(instrumentation1)); assertThat(newOptions.getBatchLoaderContextProvider(), equalTo(contextProvider1)); + + // was changed + assertThat(newOptions.batchingEnabled(), equalTo(true)); + assertThat(newOptions.getInstrumentation(), equalTo(instrumentation2)); + } } \ No newline at end of file From f54faf92f7683c0ad9188cfa262f62f99fe91d48 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 1 Apr 2025 18:14:47 +1100 Subject: [PATCH 3/3] Fixed a bug that the Spring team found - tweak - extra test --- .../org/dataloader/DataLoaderBuilderTest.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/dataloader/DataLoaderBuilderTest.java b/src/test/java/org/dataloader/DataLoaderBuilderTest.java index 523b9a5..f38ff82 100644 --- a/src/test/java/org/dataloader/DataLoaderBuilderTest.java +++ b/src/test/java/org/dataloader/DataLoaderBuilderTest.java @@ -46,19 +46,31 @@ void canBuildNewDataLoaders() { @Test void theDataLoaderCanTransform() { - DataLoader dataLoader1 = DataLoaderFactory.newDataLoader(batchLoader1, defaultOptions); - assertThat(dataLoader1.getOptions(), equalTo(defaultOptions)); - assertThat(dataLoader1.getBatchLoadFunction(), equalTo(batchLoader1)); + DataLoader dataLoaderOrig = DataLoaderFactory.newDataLoader(batchLoader1, defaultOptions); + assertThat(dataLoaderOrig.getOptions(), equalTo(defaultOptions)); + assertThat(dataLoaderOrig.getBatchLoadFunction(), equalTo(batchLoader1)); // // we can transform the data loader // - DataLoader dataLoader2 = dataLoader1.transform(it -> { + DataLoader dataLoaderTransformed = dataLoaderOrig.transform(it -> { it.options(differentOptions); it.batchLoadFunction(batchLoader2); }); - assertThat(dataLoader2, not(equalTo(dataLoader1))); - assertThat(dataLoader2.getOptions(), equalTo(differentOptions)); - assertThat(dataLoader2.getBatchLoadFunction(), equalTo(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)); + } }