Skip to content

Commit

Permalink
Replace Integer Options with Defaulted int (#17385)
Browse files Browse the repository at this point in the history
* Replace Integer options with defaulted int

* Update Duration APIs to require non-null, changed Boolean to boolean

* Cleanup documentation
  • Loading branch information
alzimmermsft authored Nov 10, 2020
1 parent 423673d commit 91387e1
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ public final class SearchIndexingBufferedSenderOptions<T> {

private final ClientLogger logger = new ClientLogger(SearchIndexingBufferedSenderOptions.class);

private Boolean autoFlush;
private Duration autoFlushWindow;
private Integer initialBatchActionCount;
private Integer maxRetries;
private Duration retryDelay;
private Duration maxRetryDelay;
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 = DEFAULT_RETRY_DELAY;
private Duration maxRetryDelay = DEFAULT_MAX_RETRY_DELAY;

private Consumer<IndexAction<T>> onActionAddedConsumer;
private Consumer<IndexAction<T>> onActionSucceededConsumer;
Expand All @@ -45,14 +45,12 @@ public final class SearchIndexingBufferedSenderOptions<T> {

/**
* 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)}.
* <p>
* If {@code autoFlush} is null the buffered sender will be set to automatically flush.
* configurations of {@link #setAutoFlushWindow(Duration)} and {@link #setInitialBatchActionCount(int)}.
*
* @param autoFlush Flag determining whether a buffered sender will automatically flush.
* @return The updated SearchIndexingBufferedSenderOptions object.
*/
public SearchIndexingBufferedSenderOptions<T> setAutoFlush(Boolean autoFlush) {
public SearchIndexingBufferedSenderOptions<T> setAutoFlush(boolean autoFlush) {
this.autoFlush = autoFlush;
return this;
}
Expand All @@ -63,23 +61,25 @@ public SearchIndexingBufferedSenderOptions<T> 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;
}

/**
* Sets the duration between a buffered sender sending documents to be indexed.
* <p>
* 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.
* <p>
* 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.
* 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<T> setAutoFlushWindow(Duration autoFlushWindow) {
Objects.requireNonNull(autoFlushWindow, "'autoFlushWindow' cannot be null.");

this.autoFlushWindow = autoFlushWindow;
return this;
}
Expand All @@ -88,18 +88,18 @@ public SearchIndexingBufferedSenderOptions<T> setAutoFlushWindow(Duration autoFl
* Gets the {@link Duration} that the buffered sender will wait between sending documents to be indexed.
* <p>
* 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.
* <p>
* If the duration is less than or equal to zero the buffered sender will only flush when {@link
* #getInitialBatchActionCount()} is triggered.
* <p>
* 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.
*/
public Duration getAutoFlushWindow() {
return (autoFlushWindow == null) ? DEFAULT_FLUSH_WINDOW : autoFlushWindow;
return autoFlushWindow;
}

/**
Expand All @@ -112,23 +112,24 @@ public Duration getAutoFlushWindow() {
* @return The updated SearchIndexingBufferedSenderOptions object.
* @throws IllegalArgumentException If {@code batchSize} is less than one.
*/
public SearchIndexingBufferedSenderOptions<T> setInitialBatchActionCount(Integer initialBatchActionCount) {
if (initialBatchActionCount != null && initialBatchActionCount < 1) {
public SearchIndexingBufferedSenderOptions<T> setInitialBatchActionCount(int initialBatchActionCount) {
if (initialBatchActionCount < 1) {
throw logger.logExceptionAsError(new IllegalArgumentException("'batchSize' cannot be less than one."));
}

this.initialBatchActionCount = initialBatchActionCount;
return this;
}

/**
* Gets the number of documents required in a batch for it to be flushed.
* <p>
* 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.
*/
public int getInitialBatchActionCount() {
return (initialBatchActionCount == null) ? DEFAULT_INITIAL_BATCH_ACTION_COUNT : initialBatchActionCount;
return initialBatchActionCount;
}

/**
Expand All @@ -142,8 +143,8 @@ public int getInitialBatchActionCount() {
* @return The updated SearchIndexingBufferedSenderOptions object.
* @throws IllegalArgumentException If {@code documentTryLimit} is less than one.
*/
public SearchIndexingBufferedSenderOptions<T> setMaxRetries(Integer maxRetries) {
if (maxRetries != null && maxRetries < 1) {
public SearchIndexingBufferedSenderOptions<T> setMaxRetries(int maxRetries) {
if (maxRetries < 1) {
throw logger.logExceptionAsError(
new IllegalArgumentException("'maxRetries' cannot be less than one."));
}
Expand All @@ -158,7 +159,7 @@ public SearchIndexingBufferedSenderOptions<T> 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;
}

/**
Expand All @@ -169,9 +170,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<T> 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."));
}

Expand All @@ -185,7 +189,7 @@ public SearchIndexingBufferedSenderOptions<T> 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;
}

/**
Expand All @@ -198,10 +202,14 @@ 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.
* @throws NullPointerException If {@code maxRetryDelay} is null.
*/
public SearchIndexingBufferedSenderOptions<T> 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."));
}
Expand All @@ -216,7 +224,7 @@ public SearchIndexingBufferedSenderOptions<T> 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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,24 @@ public class SearchIndexingBufferedSenderOptionsTests {
public void autoFlushDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertTrue(options.getAutoFlush());

options.setAutoFlush(null);
assertTrue(options.getAutoFlush());
}

@Test
public void flushWindowDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertEquals(Duration.ofSeconds(60), options.getAutoFlushWindow());
}

options.setAutoFlushWindow(null);
assertEquals(Duration.ofSeconds(60), options.getAutoFlushWindow());
@Test
public void invalidFlushWindowThrows() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertThrows(NullPointerException.class, () -> options.setAutoFlushWindow(null));
}

@Test
public void initialBatchActionCountDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertEquals(512, options.getInitialBatchActionCount());

options.setInitialBatchActionCount(null);
assertEquals(512, options.getInitialBatchActionCount());
}

@Test
Expand All @@ -53,9 +50,6 @@ public void invalidBatchSizeThrows() {
public void maxRetriesDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertEquals(3, options.getMaxRetries());

options.setMaxRetries(null);
assertEquals(3, options.getMaxRetries());
}

@Test
Expand All @@ -69,14 +63,12 @@ public void invalidMaxRetriesThrows() {
public void retryDelayDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertEquals(Duration.ofMillis(800), options.getRetryDelay());

options.setRetryDelay(null);
assertEquals(Duration.ofMillis(800), options.getRetryDelay());
}

@Test
public void invalidRetryDelayThrows() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertThrows(NullPointerException.class, () -> options.setRetryDelay(null));
assertThrows(IllegalArgumentException.class, () -> options.setRetryDelay(Duration.ZERO));
assertThrows(IllegalArgumentException.class, () -> options.setRetryDelay(Duration.ofMillis(-1)));
}
Expand All @@ -85,14 +77,12 @@ public void invalidRetryDelayThrows() {
public void maxRetryDelayDefaults() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertEquals(Duration.ofMinutes(1), options.getMaxRetryDelay());

options.setRetryDelay(null);
assertEquals(Duration.ofMinutes(1), options.getMaxRetryDelay());
}

@Test
public void invalidMaxRetryDelayThrows() {
SearchIndexingBufferedSenderOptions<Integer> options = new SearchIndexingBufferedSenderOptions<>();
assertThrows(NullPointerException.class, () -> options.setMaxRetryDelay(null));
assertThrows(IllegalArgumentException.class, () -> options.setMaxRetryDelay(Duration.ZERO));
assertThrows(IllegalArgumentException.class, () -> options.setMaxRetryDelay(Duration.ofMillis(-1)));
}
Expand Down

0 comments on commit 91387e1

Please sign in to comment.