Skip to content

Commit

Permalink
chore: add selected firestore sample and module it tests to native ci (
Browse files Browse the repository at this point in the history
…#2318)

This pr adds `spring-cloud-gcp-data-firestore` and `spring-cloud-gcp-data-firestore-sample` module to native ci, excludes transactional tests that rely on Mockito to verify behavior, adds a simple transaction test for native test.

Changes in this pr:
- Adds runtime hint for firestore repository.
- Adds data-firestore-sample and data-firestore module to native ci.
- Separate out transaction tests and exclude from native tests. These tests utilizes Mockito to verify that actions within one transaction are actually committed only once. For native, add a simple transaction test verifying transaction related code path are accessible.
- Adds runtime hint for domain class `PhoneNumber` in integration test. 
- Adds a package-private constructor for `FirestoreTemplate` to allow for adding uuid suffix to collection name. (Data is organized as Documents within Collection in Firestore) This is used in test configuration to setup for parallel test runs. Alternative is to add uuid to each Document created in test. However, some methods, such as find by class behavior may be unpredictable in parallel run, and integration test may need some refactoring. For future, consider a new feature to let user override collectionName setup from `@Document`.
  • Loading branch information
zhumin8 authored Nov 10, 2023
1 parent 8a2f9eb commit 28ab0c2
Show file tree
Hide file tree
Showing 17 changed files with 678 additions and 232 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/NativeTests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ jobs:
it:
- spanner
- datastore-basic-sample
- data-firestore-sample
- data-firestore
steps:
- name: Get current date
id: date
Expand Down
6 changes: 3 additions & 3 deletions docs/src/main/asciidoc/firestore.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ In order to use it, you would need to autowire `ReactiveFirestoreTransactionMana
[source, java]
----
public class MyApplication {
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryIntegrationTests.java[tag=autowire_tx_manager]
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryTransactionIntegrationTests.java[tag=autowire_tx_manager]
}
----

Expand All @@ -244,14 +244,14 @@ Note that you can switch between read-only and read-write transactions using `Tr

[source, java, indent=0]
----
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryIntegrationTests.java[tag=repository_transactional_operator]
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryTransactionIntegrationTests.java[tag=repository_transactional_operator]
----

When you have an instance of `TransactionalOperator`, you can invoke a sequence of Firestore operations in a transaction by using `operator::transactional`:

[source, java, indent=0]
----
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryIntegrationTests.java[tag=repository_operations_in_a_transaction]
include::{project-root}/spring-cloud-gcp-data-firestore/src/test/java/com/google/cloud/spring/data/firestore/it/FirestoreRepositoryTransactionIntegrationTests.java[tag=repository_operations_in_a_transaction]
----

NOTE: Read operations in a transaction can only happen before write operations.
Expand Down
2 changes: 2 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@
<module>spring-cloud-gcp-secretmanager</module>
<module>spring-cloud-gcp-kms</module>
<module>spring-cloud-gcp-data-datastore</module>
<module>spring-cloud-gcp-data-firestore</module>
<module>spring-cloud-gcp-core</module>
</modules>

Expand Down Expand Up @@ -392,6 +393,7 @@
<it.secretmanager>true</it.secretmanager>
<it.kms>true</it.kms>
<it.datastore>true</it.datastore>
<it.firestore>true</it.firestore>
</systemPropertyVariables>
</configuration>
</plugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ public class FirestoreTemplate implements FirestoreReactiveOperations {

private int writeBufferSize = FIRESTORE_WRITE_MAX_SIZE;


private String collectionNameSuffix = "";

/**
* Constructor for FirestoreTemplate.
*
Expand All @@ -108,6 +111,20 @@ public FirestoreTemplate(
this.mappingContext = mappingContext;
}

FirestoreTemplate(
FirestoreStub firestoreStub,
String parent,
FirestoreClassMapper classMapper,
FirestoreMappingContext mappingContext,
String collectionNameSuffix) {
this.firestoreStub = firestoreStub;
this.parent = parent;
this.databasePath = Util.extractDatabasePath(parent);
this.classMapper = classMapper;
this.mappingContext = mappingContext;
this.collectionNameSuffix = collectionNameSuffix;
}

/**
* Sets the {@link Duration} for how long to wait for the entity buffer to fill before sending the
* buffered entities to Firestore for insert/update/delete operations.
Expand Down Expand Up @@ -280,7 +297,7 @@ public <T> FirestoreReactiveOperations withParent(T parent) {

@Override
public String buildResourceName(FirestorePersistentEntity<?> persistentEntity, String resource) {
return this.parent + "/" + persistentEntity.collectionName() + "/" + resource;
return this.parent + "/" + persistentEntity.collectionName() + collectionNameSuffix + "/" + resource;
}

private FirestoreReactiveOperations withParent(String resourceName) {
Expand Down Expand Up @@ -362,7 +379,7 @@ private <T> Flux<Document> findAllDocuments(
queryBuilder != null ? queryBuilder.clone() : StructuredQuery.newBuilder();
builder.addFrom(
StructuredQuery.CollectionSelector.newBuilder()
.setCollectionId(persistentEntity.collectionName())
.setCollectionId(persistentEntity.collectionName() + collectionNameSuffix)
.build());
if (projection != null) {
builder.setSelect(projection);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.spring.data.firestore.aot;

import com.google.cloud.spring.data.firestore.SimpleFirestoreReactiveRepository;
import java.util.Arrays;
import org.springframework.aot.hint.MemberCategory;
import org.springframework.aot.hint.RuntimeHints;
import org.springframework.aot.hint.RuntimeHintsRegistrar;
import org.springframework.aot.hint.TypeReference;

public class FirestoreRepositoryRuntimeHints implements RuntimeHintsRegistrar {

@Override
public void registerHints(RuntimeHints hints, ClassLoader classLoader) {
hints
.reflection()
.registerTypes(
Arrays.asList(TypeReference.of(SimpleFirestoreReactiveRepository.class)),
hint ->
hint.withMembers(
MemberCategory.INVOKE_DECLARED_CONSTRUCTORS,
MemberCategory.INVOKE_DECLARED_METHODS));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
package com.google.cloud.spring.data.firestore.repository.support;

import com.google.cloud.spring.data.firestore.FirestoreTemplate;
import com.google.cloud.spring.data.firestore.aot.FirestoreRepositoryRuntimeHints;
import com.google.cloud.spring.data.firestore.mapping.FirestoreMappingContext;
import org.springframework.context.annotation.ImportRuntimeHints;
import org.springframework.data.repository.Repository;
import org.springframework.data.repository.core.support.RepositoryFactoryBeanSupport;
import org.springframework.data.repository.core.support.RepositoryFactorySupport;
Expand All @@ -30,6 +32,7 @@
* @param <T> the repository type
* @since 1.2
*/
@ImportRuntimeHints(FirestoreRepositoryRuntimeHints.class)
public class FirestoreRepositoryFactoryBean<T extends Repository<S, I>, S, I>
extends RepositoryFactoryBeanSupport<T, S, I> {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
* limitations under the License.
*/

package com.google.cloud.spring.data.firestore.it;
package com.google.cloud.spring.data.firestore;

import com.google.api.client.util.escape.PercentEscaper;
import com.google.api.gax.rpc.internal.Headers;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.spring.core.DefaultGcpProjectIdProvider;
import com.google.cloud.spring.data.firestore.FirestoreTemplate;
import com.google.cloud.spring.data.firestore.entities.UserRepository;
import com.google.cloud.spring.data.firestore.it.UserService;
import com.google.cloud.spring.data.firestore.mapping.FirestoreClassMapper;
import com.google.cloud.spring.data.firestore.mapping.FirestoreDefaultClassMapper;
import com.google.cloud.spring.data.firestore.mapping.FirestoreMappingContext;
Expand All @@ -36,7 +36,7 @@
import io.grpc.auth.MoreCallCredentials;
import io.grpc.stub.MetadataUtils;
import java.io.IOException;
import org.mockito.Mockito;
import java.util.UUID;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
Expand All @@ -53,6 +53,8 @@
public class FirestoreIntegrationTestsConfiguration {
String defaultParent;

String uuid = UUID.randomUUID().toString();

String projectId;

String databaseId;
Expand Down Expand Up @@ -93,7 +95,7 @@ public FirestoreTemplate firestoreTemplate(
FirestoreClassMapper classMapper,
FirestoreMappingContext firestoreMappingContext) {
return new FirestoreTemplate(
firestoreStub, this.defaultParent, classMapper, firestoreMappingContext);
firestoreStub, this.defaultParent, classMapper, firestoreMappingContext, uuid);
}

@Bean
Expand All @@ -114,24 +116,24 @@ public ClientInterceptor firestoreRoutingHeadersInterceptor() {
return MetadataUtils.newAttachHeadersInterceptor(routingHeader);
}

@Bean
@ConditionalOnMissingBean
public ReactiveFirestoreTransactionManager firestoreTransactionManager(
FirestoreGrpc.FirestoreStub firestoreStub, FirestoreClassMapper classMapper) {
return Mockito.spy(
new ReactiveFirestoreTransactionManager(firestoreStub, this.defaultParent, classMapper));
}

// tag::user_service_bean[]
@Bean
public UserService userService() {
return new UserService();
}

// end::user_service_bean[]

@Bean
@ConditionalOnMissingBean
public FirestoreClassMapper getClassMapper(FirestoreMappingContext mappingContext) {
return new FirestoreDefaultClassMapper(mappingContext);
}

@Bean
@ConditionalOnMissingBean
public ReactiveFirestoreTransactionManager firestoreTransactionManager(
FirestoreGrpc.FirestoreStub firestoreStub, FirestoreClassMapper classMapper) {
return new ReactiveFirestoreTransactionManager(firestoreStub, this.defaultParent, classMapper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.spring.data.firestore;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.mockito.ArgumentMatchers.any;
Expand All @@ -29,6 +30,7 @@
import com.google.cloud.firestore.annotation.DocumentId;
import com.google.cloud.spring.data.firestore.mapping.FirestoreDefaultClassMapper;
import com.google.cloud.spring.data.firestore.mapping.FirestoreMappingContext;
import com.google.cloud.spring.data.firestore.mapping.FirestorePersistentEntityImpl;
import com.google.cloud.spring.data.firestore.mapping.UpdateTime;
import com.google.firestore.v1.CommitRequest;
import com.google.firestore.v1.CommitResponse;
Expand All @@ -49,6 +51,7 @@
import java.util.Objects;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.data.util.TypeInformation;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
Expand All @@ -75,6 +78,23 @@ void setup() {
mappingContext);
}

@Test
void templateWithSuffixForTestTest() {
FirestoreMappingContext mappingContext = new FirestoreMappingContext();
FirestoreTemplate firestoreTemplateWithSuffix =
new FirestoreTemplate(
this.firestoreStub,
parent,
new FirestoreDefaultClassMapper(mappingContext),
mappingContext,
"_suffix");

FirestorePersistentEntityImpl<TestEntity> persistentEntity =
new FirestorePersistentEntityImpl<TestEntity>(TypeInformation.of(TestEntity.class));
String name = firestoreTemplateWithSuffix.buildResourceName(persistentEntity, "resource");
assertThat(name).isEqualTo(parent + "/testEntities_suffix/resource");
}

@Test
void findAllTest() {
mockRunQueryMethod();
Expand Down
Loading

0 comments on commit 28ab0c2

Please sign in to comment.