Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce java.time variables and methods #1935

Merged
merged 5 commits into from
Nov 21, 2024
Merged

feat: introduce java.time variables and methods #1935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
36e0d05
feat: introduce `java.time` variables and methods
diegomarquezp Nov 19, 2024
d072d5d
chore: generate libraries at Tue Nov 19 22:27:50 UTC 2024
cloud-java-bot Nov 19, 2024
2a5eedc
fix tests
diegomarquezp Nov 19, 2024
60643fe
remove overload for package private method
diegomarquezp Nov 20, 2024
cc3e939
chore: generate libraries at Wed Nov 20 20:18:13 UTC 2024
cloud-java-bot Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ If you are using Maven without the BOM, add this to your dependencies:
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-firestore</artifactId>
<version>3.29.0</version>
<version>3.29.1</version>
</dependency>

```
Expand Down
14 changes: 11 additions & 3 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

package com.google.cloud.firestore;

import static com.google.api.gax.util.TimeConversionUtils.toThreetenDuration;
import static com.google.cloud.firestore.telemetry.TraceUtil.*;

import com.google.api.core.ApiClock;
import com.google.api.core.ApiFuture;
import com.google.api.core.NanoClock;
import com.google.api.core.ObsoleteApi;
import com.google.api.core.SettableApiFuture;
import com.google.api.gax.rpc.ApiStreamObserver;
import com.google.api.gax.rpc.BidiStreamObserver;
Expand Down Expand Up @@ -52,7 +54,6 @@
import java.util.Random;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* Main implementation of the Firestore client. This is the entry point for all Firestore
Expand Down Expand Up @@ -500,9 +501,16 @@ public FirestoreRpc getClient() {
return firestoreClient;
}

/** This method is obsolete. Use {@link #getTotalRequestTimeoutDuration()} instead. */
@ObsoleteApi("Use getTotalRequestTimeoutDuration() instead")
@Override
public Duration getTotalRequestTimeout() {
return firestoreOptions.getRetrySettings().getTotalTimeout();
public org.threeten.bp.Duration getTotalRequestTimeout() {
return toThreetenDuration(getTotalRequestTimeoutDuration());
}

@Override
public java.time.Duration getTotalRequestTimeoutDuration() {
return firestoreOptions.getRetrySettings().getTotalTimeoutDuration();
Comment on lines +511 to +513
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Would be nice to get javadocs from firestore for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehsannas for visibility - the superclass does not have javadocs on this method

}

@Override
Expand Down
12 changes: 10 additions & 2 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreRpcContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,20 @@

package com.google.cloud.firestore;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;

import com.google.api.core.ApiClock;
import com.google.api.core.ApiFuture;
import com.google.api.core.InternalApi;
import com.google.api.core.InternalExtensionOnly;
import com.google.api.core.ObsoleteApi;
import com.google.api.gax.rpc.BidiStreamObserver;
import com.google.api.gax.rpc.BidiStreamingCallable;
import com.google.api.gax.rpc.ClientStream;
import com.google.api.gax.rpc.ResponseObserver;
import com.google.api.gax.rpc.ServerStreamingCallable;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.cloud.firestore.spi.v1.FirestoreRpc;
import org.threeten.bp.Duration;

@InternalApi
@InternalExtensionOnly
Expand All @@ -41,7 +43,13 @@ interface FirestoreRpcContext<FS extends Firestore> {

FirestoreRpc getClient();

Duration getTotalRequestTimeout();
/** This method is obsolete. Use {@link #getTotalRequestTimeoutDuration()} instead. */
@ObsoleteApi("Use getTotalRequestTimeoutDuration() instead")
org.threeten.bp.Duration getTotalRequestTimeout();

default java.time.Duration getTotalRequestTimeoutDuration() {
return toJavaTimeDuration(getTotalRequestTimeout());
}

ApiClock getClock();

Expand Down
4 changes: 2 additions & 2 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/StreamableQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@
import com.google.firestore.v1.RunQueryResponse;
import com.google.protobuf.ByteString;
import io.grpc.Status;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* Represents a query whose results can be streamed. If the stream fails with a retryable error,
Expand Down Expand Up @@ -452,7 +452,7 @@ boolean shouldRetryQuery(
}

Duration duration = Duration.ofNanos(rpcContext.getClock().nanoTime() - startTimeNanos);
return duration.compareTo(rpcContext.getTotalRequestTimeout()) < 0;
return duration.compareTo(rpcContext.getTotalRequestTimeoutDuration()) < 0;
}

/** Verifies whether the given exception is retryable based on the RunQuery configuration. */
Expand Down
7 changes: 4 additions & 3 deletions ...loud-firestore/src/main/java/com/google/cloud/firestore/telemetry/CompositeApiTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

package com.google.cloud.firestore.telemetry;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;

import com.google.api.gax.tracing.ApiTracer;
import com.google.api.gax.tracing.BaseApiTracer;
import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import org.threeten.bp.Duration;

/** Combines multiple {@link ApiTracer}s into a single {@link ApiTracer}. */
class CompositeApiTracer extends BaseApiTracer {
Expand Down Expand Up @@ -83,8 +84,8 @@ public void attemptCancelled() {
}

@Override
public void attemptFailed(Throwable error, Duration delay) {
children.forEach(child -> child.attemptFailed(error, delay));
public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
attemptFailedDuration(error, toJavaTimeDuration(delay));
}

@Override
Expand Down
16 changes: 10 additions & 6 deletions ...-cloud-firestore/src/main/java/com/google/cloud/firestore/telemetry/EnabledTraceUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> getChannelConfi
// ends in `s` to indicate seconds and is preceded by the number of seconds, with nanoseconds
// expressed as fractional
// seconds.
String durationString(org.threeten.bp.Duration duration) {
String durationString(java.time.Duration duration) {
int nanos = duration.getNano();
long seconds = duration.getSeconds();
int numLeadingZeros = 9;
Expand Down Expand Up @@ -330,10 +330,12 @@ private SpanBuilder addSettingsAttributesToCurrentSpan(SpanBuilder spanBuilder)
Attributes.builder()
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.initial_retry_delay",
durationString(firestoreOptions.getRetrySettings().getInitialRetryDelay()))
durationString(
firestoreOptions.getRetrySettings().getInitialRetryDelayDuration()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.max_retry_delay",
durationString(firestoreOptions.getRetrySettings().getMaxRetryDelay()))
durationString(
firestoreOptions.getRetrySettings().getMaxRetryDelayDuration()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.retry_delay_multiplier",
String.valueOf(firestoreOptions.getRetrySettings().getRetryDelayMultiplier()))
Expand All @@ -342,16 +344,18 @@ private SpanBuilder addSettingsAttributesToCurrentSpan(SpanBuilder spanBuilder)
String.valueOf(firestoreOptions.getRetrySettings().getMaxAttempts()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.initial_rpc_timeout",
durationString(firestoreOptions.getRetrySettings().getInitialRpcTimeout()))
durationString(
firestoreOptions.getRetrySettings().getInitialRpcTimeoutDuration()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.max_rpc_timeout",
durationString(firestoreOptions.getRetrySettings().getMaxRpcTimeout()))
durationString(
firestoreOptions.getRetrySettings().getMaxRpcTimeoutDuration()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.rpc_timeout_multiplier",
String.valueOf(firestoreOptions.getRetrySettings().getRpcTimeoutMultiplier()))
.put(
ATTRIBUTE_SERVICE_PREFIX + "settings.retry_settings.total_timeout",
durationString(firestoreOptions.getRetrySettings().getTotalTimeout()))
durationString(firestoreOptions.getRetrySettings().getTotalTimeoutDuration()))
.build());
}

Expand Down
6 changes: 3 additions & 3 deletions google-cloud-firestore/src/test/java/com/google/cloud/firestore/LocalFirestoreHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import java.nio.charset.StandardCharsets;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -88,14 +89,13 @@
import org.mockito.ArgumentMatchers;
import org.mockito.stubbing.Answer;
import org.mockito.stubbing.Stubber;
import org.threeten.bp.Duration;

public final class LocalFirestoreHelper {

protected static RetrySettings IMMEDIATE_RETRY_SETTINGS =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ZERO)
.setMaxRetryDelay(Duration.ZERO)
.setInitialRetryDelayDuration(Duration.ZERO)
.setMaxRetryDelayDuration(Duration.ZERO)
.setRetryDelayMultiplier(1)
.setJittered(false)
.build();
Expand Down
10 changes: 5 additions & 5 deletions google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryCountTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.firestore.v1.RunAggregationQueryResponse;
import com.google.firestore.v1.StructuredQuery;
import io.grpc.Status;
import java.time.Duration;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.junit.Before;
Expand All @@ -50,7 +51,6 @@
import org.mockito.Captor;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.threeten.bp.Duration;

@RunWith(MockitoJUnitRunner.class)
public class QueryCountTest {
Expand All @@ -70,7 +70,7 @@ public class QueryCountTest {

@Before
public void before() {
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeoutDuration();
query = firestoreMock.collection(COLLECTION_ID);
}

Expand Down Expand Up @@ -230,7 +230,7 @@ public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus(
public void
shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithInfiniteTimeoutWindow()
throws Exception {
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeoutDuration();
doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL)))
.doAnswer(countQueryResponse(42))
.when(firestoreMock)
Expand All @@ -245,7 +245,7 @@ public void shouldNotRetryIfExceptionIsFirestoreExceptionWithNonRetryableStatus(
@Test
public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithinTimeoutWindow()
throws Exception {
doReturn(Duration.ofDays(999)).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ofDays(999)).when(firestoreMock).getTotalRequestTimeoutDuration();
doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL)))
.doAnswer(countQueryResponse(42))
.when(firestoreMock)
Expand All @@ -267,7 +267,7 @@ public void shouldRetryIfExceptionIsFirestoreExceptionWithRetryableStatusWithinT
.doReturn(TimeUnit.SECONDS.toNanos(30))
.when(clockMock)
.nanoTime();
doReturn(Duration.ofSeconds(5)).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ofSeconds(5)).when(firestoreMock).getTotalRequestTimeoutDuration();
doAnswer(countQueryResponse(new FirestoreException("reason", Status.INTERNAL)))
.doAnswer(countQueryResponse(42))
.when(firestoreMock)
Expand Down
6 changes: 3 additions & 3 deletions google-cloud-firestore/src/test/java/com/google/cloud/firestore/QueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import io.grpc.Status;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -82,7 +83,6 @@
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import org.threeten.bp.Duration;

@RunWith(MockitoJUnitRunner.class)
public class QueryTest {
Expand Down Expand Up @@ -123,7 +123,7 @@ public long millisTime() {
public void before() {
clock = new MockClock();
doReturn(clock).when(firestoreMock).getClock();
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ZERO).when(firestoreMock).getTotalRequestTimeoutDuration();

query = firestoreMock.collection(COLLECTION_ID);
}
Expand Down Expand Up @@ -1130,7 +1130,7 @@ public void retriesWithoutTimeout() throws Exception {

@Test
public void doesNotRetryWithTimeout() {
doReturn(Duration.ofMinutes(1)).when(firestoreMock).getTotalRequestTimeout();
doReturn(Duration.ofMinutes(1)).when(firestoreMock).getTotalRequestTimeoutDuration();

doAnswer(
invocation -> {
Expand Down
8 changes: 4 additions & 4 deletions google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import io.grpc.Status;
import io.grpc.Status.Code;
import io.grpc.StatusRuntimeException;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -106,7 +107,6 @@
import org.junit.rules.TestName;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.threeten.bp.Duration;

@RunWith(JUnit4.class)
public class ITSystemTest extends ITBaseTest {
Expand Down Expand Up @@ -2281,9 +2281,9 @@ public void testEnforcesTimeouts() {
FirestoreOptions.newBuilder()
.setRetrySettings(
RetrySettings.newBuilder()
.setMaxRpcTimeout(Duration.ofMillis(1))
.setTotalTimeout(Duration.ofMillis(1))
.setInitialRpcTimeout(Duration.ofMillis(1))
.setMaxRpcTimeoutDuration(Duration.ofMillis(1))
.setTotalTimeoutDuration(Duration.ofMillis(1))
.setInitialRpcTimeoutDuration(Duration.ofMillis(1))
.build())
.build();
firestore = firestoreOptions.getService();
Expand Down
2 changes: 1 addition & 1 deletion ...ud-firestore/src/test/java/com/google/cloud/firestore/telemetry/EnabledTraceUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import java.time.Duration;
import org.junit.Before;
import org.junit.Test;
import org.threeten.bp.Duration;

public class EnabledTraceUtilTest {
@Before
Expand Down
Loading