From d0d7c4ed9a2d8d10ade35e5dc4c318782f27b8cf Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Nov 2020 15:05:01 -0800 Subject: [PATCH 1/3] Replace Integer options with defaulted int --- .../SearchIndexingBufferedSenderOptions.java | 30 ++++++++++--------- ...rchIndexingBufferedSenderOptionsTests.java | 6 ---- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java index be2c722e5e28..cd98830021c4 100644 --- a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java +++ b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java @@ -31,8 +31,8 @@ public final class SearchIndexingBufferedSenderOptions { private Boolean autoFlush; private Duration autoFlushWindow; - private Integer initialBatchActionCount; - private Integer maxRetries; + private int initialBatchActionCount = DEFAULT_INITIAL_BATCH_ACTION_COUNT; + private int maxRetries = DEFAULT_MAX_RETRIES; private Duration retryDelay; private Duration maxRetryDelay; @@ -45,7 +45,7 @@ public final class SearchIndexingBufferedSenderOptions { /** * Sets the flag determining whether a buffered sender will automatically flush its document batch based on the - * configurations of {@link #setAutoFlushWindow(Duration)} and {@link #setInitialBatchActionCount(Integer)}. + * configurations of {@link #setAutoFlushWindow(Duration)} and {@link #setInitialBatchActionCount(int)}. *

* If {@code autoFlush} is null the buffered sender will be set to automatically flush. * @@ -70,11 +70,11 @@ public boolean getAutoFlush() { * Sets the duration between a buffered sender sending documents to be indexed. *

* The buffered sender will reset the duration when documents are sent for indexing, either by reaching {@link - * #setInitialBatchActionCount(Integer)} or by a manual trigger. + * #setInitialBatchActionCount(int)} or by a manual trigger. *

* If {@code flushWindow} is negative or zero and {@link #setAutoFlush(Boolean)} is enabled the buffered sender will - * only flush when {@link #setInitialBatchActionCount(Integer)} is met. If {@code flushWindow} is null a default - * value of 60 seconds is used. + * only flush when {@link #setInitialBatchActionCount(int)} is met. If {@code flushWindow} is null a default value + * of 60 seconds is used. * * @param autoFlushWindow Duration between document batches being sent for indexing. * @return The updated SearchIndexingBufferedSenderOptions object. @@ -88,7 +88,7 @@ public SearchIndexingBufferedSenderOptions setAutoFlushWindow(Duration autoFl * Gets the {@link Duration} that the buffered sender will wait between sending documents to be indexed. *

* The buffered sender will reset the duration when documents are sent for indexing, either by reaching {@link - * #setInitialBatchActionCount(Integer)} or by a manual trigger. + * #setInitialBatchActionCount(int)} or by a manual trigger. *

* If the duration is less than or equal to zero the buffered sender will only flush when {@link * #getInitialBatchActionCount()} is triggered. @@ -112,10 +112,11 @@ public Duration getAutoFlushWindow() { * @return The updated SearchIndexingBufferedSenderOptions object. * @throws IllegalArgumentException If {@code batchSize} is less than one. */ - public SearchIndexingBufferedSenderOptions setInitialBatchActionCount(Integer initialBatchActionCount) { - if (initialBatchActionCount != null && initialBatchActionCount < 1) { + public SearchIndexingBufferedSenderOptions setInitialBatchActionCount(int initialBatchActionCount) { + if (initialBatchActionCount < 1) { throw logger.logExceptionAsError(new IllegalArgumentException("'batchSize' cannot be less than one.")); } + this.initialBatchActionCount = initialBatchActionCount; return this; } @@ -128,7 +129,7 @@ public SearchIndexingBufferedSenderOptions setInitialBatchActionCount(Integer * @return The number of documents required before a flush is triggered. */ public int getInitialBatchActionCount() { - return (initialBatchActionCount == null) ? DEFAULT_INITIAL_BATCH_ACTION_COUNT : initialBatchActionCount; + return initialBatchActionCount; } /** @@ -142,8 +143,8 @@ public int getInitialBatchActionCount() { * @return The updated SearchIndexingBufferedSenderOptions object. * @throws IllegalArgumentException If {@code documentTryLimit} is less than one. */ - public SearchIndexingBufferedSenderOptions setMaxRetries(Integer maxRetries) { - if (maxRetries != null && maxRetries < 1) { + public SearchIndexingBufferedSenderOptions setMaxRetries(int maxRetries) { + if (maxRetries < 1) { throw logger.logExceptionAsError( new IllegalArgumentException("'maxRetries' cannot be less than one.")); } @@ -158,7 +159,7 @@ public SearchIndexingBufferedSenderOptions setMaxRetries(Integer maxRetries) * @return The number of times a document will attempt indexing. */ public int getMaxRetries() { - return (maxRetries == null) ? DEFAULT_MAX_RETRIES : maxRetries; + return maxRetries; } /** @@ -198,7 +199,8 @@ public Duration getRetryDelay() { * * @param maxRetryDelay The maximum duration requests will delay when the service is throttling. * @return The updated SearchIndexingBufferedSenderOptions object. - * @throws IllegalArgumentException If {@code maxRetryDelay.isNegative()} or {@code maxRetryDelay.isZero()} is true. + * @throws IllegalArgumentException If {@code maxRetryDelay.isNegative()} or {@code maxRetryDelay.isZero()} is + * true. */ public SearchIndexingBufferedSenderOptions setMaxRetryDelay(Duration maxRetryDelay) { if (maxRetryDelay != null && (maxRetryDelay.isNegative() || maxRetryDelay.isZero())) { diff --git a/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java b/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java index 691058a95f06..386f6d8ef7b7 100644 --- a/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java +++ b/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java @@ -37,9 +37,6 @@ public void flushWindowDefaults() { public void initialBatchActionCountDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertEquals(512, options.getInitialBatchActionCount()); - - options.setInitialBatchActionCount(null); - assertEquals(512, options.getInitialBatchActionCount()); } @Test @@ -53,9 +50,6 @@ public void invalidBatchSizeThrows() { public void maxRetriesDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertEquals(3, options.getMaxRetries()); - - options.setMaxRetries(null); - assertEquals(3, options.getMaxRetries()); } @Test From 6f141feb6378d4b09608ddcf3ba6fe73512546d4 Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Nov 2020 16:09:04 -0800 Subject: [PATCH 2/3] Update Duration APIs to require non-null, changed Boolean to boolean --- .../SearchIndexingBufferedSenderOptions.java | 36 +++++++++++-------- ...rchIndexingBufferedSenderOptionsTests.java | 18 ++++------ 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java index cd98830021c4..be06139ad41b 100644 --- a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java +++ b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java @@ -29,12 +29,12 @@ public final class SearchIndexingBufferedSenderOptions { private final ClientLogger logger = new ClientLogger(SearchIndexingBufferedSenderOptions.class); - private Boolean autoFlush; - private Duration autoFlushWindow; + private boolean autoFlush = DEFAULT_AUTO_FLUSH; + private Duration autoFlushWindow = DEFAULT_FLUSH_WINDOW; private int initialBatchActionCount = DEFAULT_INITIAL_BATCH_ACTION_COUNT; private int maxRetries = DEFAULT_MAX_RETRIES; - private Duration retryDelay; - private Duration maxRetryDelay; + private Duration retryDelay = DEFAULT_RETRY_DELAY; + private Duration maxRetryDelay = DEFAULT_MAX_RETRY_DELAY; private Consumer> onActionAddedConsumer; private Consumer> onActionSucceededConsumer; @@ -52,7 +52,7 @@ public final class SearchIndexingBufferedSenderOptions { * @param autoFlush Flag determining whether a buffered sender will automatically flush. * @return The updated SearchIndexingBufferedSenderOptions object. */ - public SearchIndexingBufferedSenderOptions setAutoFlush(Boolean autoFlush) { + public SearchIndexingBufferedSenderOptions setAutoFlush(boolean autoFlush) { this.autoFlush = autoFlush; return this; } @@ -63,7 +63,7 @@ public SearchIndexingBufferedSenderOptions setAutoFlush(Boolean autoFlush) { * @return Flag indicating if the buffered sender will automatically flush. */ public boolean getAutoFlush() { - return (autoFlush == null) ? DEFAULT_AUTO_FLUSH : autoFlush; + return autoFlush; } /** @@ -72,14 +72,16 @@ public boolean getAutoFlush() { * The buffered sender will reset the duration when documents are sent for indexing, either by reaching {@link * #setInitialBatchActionCount(int)} or by a manual trigger. *

- * If {@code flushWindow} is negative or zero and {@link #setAutoFlush(Boolean)} is enabled the buffered sender will - * only flush when {@link #setInitialBatchActionCount(int)} is met. If {@code flushWindow} is null a default value - * of 60 seconds is used. + * If {@code flushWindow} is negative or zero and {@link #setAutoFlush(boolean)} is enabled the buffered sender will + * only flush when {@link #setInitialBatchActionCount(int)} is met. * * @param autoFlushWindow Duration between document batches being sent for indexing. * @return The updated SearchIndexingBufferedSenderOptions object. + * @throws NullPointerException If {@code autoFlushWindow} is null. */ public SearchIndexingBufferedSenderOptions setAutoFlushWindow(Duration autoFlushWindow) { + Objects.requireNonNull(autoFlushWindow, "'autoFlushWindow' cannot be null."); + this.autoFlushWindow = autoFlushWindow; return this; } @@ -99,7 +101,7 @@ public SearchIndexingBufferedSenderOptions setAutoFlushWindow(Duration autoFl * flushed. */ public Duration getAutoFlushWindow() { - return (autoFlushWindow == null) ? DEFAULT_FLUSH_WINDOW : autoFlushWindow; + return autoFlushWindow; } /** @@ -170,9 +172,12 @@ public int getMaxRetries() { * @param retryDelay The initial duration requests will delay when the service is throttling. * @return The updated SearchIndexingBufferedSenderOptions object. * @throws IllegalArgumentException If {@code retryDelay.isNegative()} or {@code retryDelay.isZero()} is true. + * @throws NullPointerException If {@code retryDelay} is null. */ public SearchIndexingBufferedSenderOptions setRetryDelay(Duration retryDelay) { - if (retryDelay != null && (retryDelay.isNegative() || retryDelay.isZero())) { + Objects.requireNonNull(retryDelay, "'retryDelay' cannot be null."); + + if (retryDelay.isNegative() || retryDelay.isZero()) { throw logger.logExceptionAsError(new IllegalArgumentException("'retryDelay' cannot be negative or zero.")); } @@ -186,7 +191,7 @@ public SearchIndexingBufferedSenderOptions setRetryDelay(Duration retryDelay) * @return The initial duration requests will delay when the service is throttling. */ public Duration getRetryDelay() { - return (retryDelay == null) ? DEFAULT_RETRY_DELAY : retryDelay; + return retryDelay; } /** @@ -201,9 +206,12 @@ public Duration getRetryDelay() { * @return The updated SearchIndexingBufferedSenderOptions object. * @throws IllegalArgumentException If {@code maxRetryDelay.isNegative()} or {@code maxRetryDelay.isZero()} is * true. + * @throws NullPointerException If {@code maxRetryDelay} is null. */ public SearchIndexingBufferedSenderOptions setMaxRetryDelay(Duration maxRetryDelay) { - if (maxRetryDelay != null && (maxRetryDelay.isNegative() || maxRetryDelay.isZero())) { + Objects.requireNonNull(maxRetryDelay, "'maxRetryDelay' cannot be null."); + + if (maxRetryDelay.isNegative() || maxRetryDelay.isZero()) { throw logger.logExceptionAsError( new IllegalArgumentException("'maxRetryDelay' cannot be negative or zero.")); } @@ -218,7 +226,7 @@ public SearchIndexingBufferedSenderOptions setMaxRetryDelay(Duration maxRetry * @return The maximum duration requests will delay when the service is throttling. */ public Duration getMaxRetryDelay() { - return (maxRetryDelay == null) ? DEFAULT_MAX_RETRY_DELAY : maxRetryDelay; + return maxRetryDelay; } /** diff --git a/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java b/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java index 386f6d8ef7b7..f25086cbb631 100644 --- a/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java +++ b/sdk/search/azure-search-documents/src/test/java/com/azure/search/documents/SearchIndexingBufferedSenderOptionsTests.java @@ -19,18 +19,18 @@ public class SearchIndexingBufferedSenderOptionsTests { public void autoFlushDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertTrue(options.getAutoFlush()); - - options.setAutoFlush(null); - assertTrue(options.getAutoFlush()); } @Test public void flushWindowDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertEquals(Duration.ofSeconds(60), options.getAutoFlushWindow()); + } - options.setAutoFlushWindow(null); - assertEquals(Duration.ofSeconds(60), options.getAutoFlushWindow()); + @Test + public void invalidFlushWindowThrows() { + SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); + assertThrows(NullPointerException.class, () -> options.setAutoFlushWindow(null)); } @Test @@ -63,14 +63,12 @@ public void invalidMaxRetriesThrows() { public void retryDelayDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertEquals(Duration.ofMillis(800), options.getRetryDelay()); - - options.setRetryDelay(null); - assertEquals(Duration.ofMillis(800), options.getRetryDelay()); } @Test public void invalidRetryDelayThrows() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); + assertThrows(NullPointerException.class, () -> options.setRetryDelay(null)); assertThrows(IllegalArgumentException.class, () -> options.setRetryDelay(Duration.ZERO)); assertThrows(IllegalArgumentException.class, () -> options.setRetryDelay(Duration.ofMillis(-1))); } @@ -79,14 +77,12 @@ public void invalidRetryDelayThrows() { public void maxRetryDelayDefaults() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); assertEquals(Duration.ofMinutes(1), options.getMaxRetryDelay()); - - options.setRetryDelay(null); - assertEquals(Duration.ofMinutes(1), options.getMaxRetryDelay()); } @Test public void invalidMaxRetryDelayThrows() { SearchIndexingBufferedSenderOptions options = new SearchIndexingBufferedSenderOptions<>(); + assertThrows(NullPointerException.class, () -> options.setMaxRetryDelay(null)); assertThrows(IllegalArgumentException.class, () -> options.setMaxRetryDelay(Duration.ZERO)); assertThrows(IllegalArgumentException.class, () -> options.setMaxRetryDelay(Duration.ofMillis(-1))); } From be7494b28e90be665cc06c67e719a9cfb71824ac Mon Sep 17 00:00:00 2001 From: alzimmermsft <48699787+alzimmermsft@users.noreply.github.com> Date: Mon, 9 Nov 2020 16:12:50 -0800 Subject: [PATCH 3/3] Cleanup documentation --- .../documents/SearchIndexingBufferedSenderOptions.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java index be06139ad41b..e2153e5ab837 100644 --- a/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java +++ b/sdk/search/azure-search-documents/src/main/java/com/azure/search/documents/SearchIndexingBufferedSenderOptions.java @@ -46,8 +46,6 @@ public final class SearchIndexingBufferedSenderOptions { /** * Sets the flag determining whether a buffered sender will automatically flush its document batch based on the * configurations of {@link #setAutoFlushWindow(Duration)} and {@link #setInitialBatchActionCount(int)}. - *

- * If {@code autoFlush} is null the buffered sender will be set to automatically flush. * * @param autoFlush Flag determining whether a buffered sender will automatically flush. * @return The updated SearchIndexingBufferedSenderOptions object. @@ -95,7 +93,7 @@ public SearchIndexingBufferedSenderOptions setAutoFlushWindow(Duration autoFl * If the duration is less than or equal to zero the buffered sender will only flush when {@link * #getInitialBatchActionCount()} is triggered. *

- * This configuration is only taken into account if {@link #getAutoFlush()} is true or null. + * This configuration is only taken into account if {@link #getAutoFlush()} is true. * * @return The {@link Duration} to wait after the last document has been added to the batch before the batch is * flushed. @@ -126,7 +124,7 @@ public SearchIndexingBufferedSenderOptions setInitialBatchActionCount(int ini /** * Gets the number of documents required in a batch for it to be flushed. *

- * This configuration is only taken into account if {@link #getAutoFlush()} is true or null. + * This configuration is only taken into account if {@link #getAutoFlush()} is true. * * @return The number of documents required before a flush is triggered. */