Skip to content

Commit

Permalink
Firestore - remove dead code (#137)
Browse files Browse the repository at this point in the history
fixes #136
  • Loading branch information
dmitry-s authored Nov 24, 2020
1 parent a737933 commit 8dcaf25
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@
import com.google.firestore.v1.StructuredQuery;
import com.google.firestore.v1.Write;
import com.google.firestore.v1.Write.Builder;
import com.google.firestore.v1.WriteRequest;
import com.google.firestore.v1.WriteResponse;
import io.grpc.stub.StreamObserver;
import org.reactivestreams.Publisher;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
Expand Down Expand Up @@ -295,23 +292,6 @@ private Flux<String> deleteDocumentsByName(Flux<String> documentNames) {
});
}

// Visible for Testing
WriteRequest buildDeleteRequest(
List<String> documentIds, WriteResponse writeResponse) {

WriteRequest.Builder writeRequestBuilder = WriteRequest.newBuilder();

if (isUsingStreamTokens()) {
writeRequestBuilder
.setStreamId(writeResponse.getStreamId())
.setStreamToken(writeResponse.getStreamToken());
}

documentIds.stream().map(this::createDeleteWrite).forEach(writeRequestBuilder::addWrites);

return writeRequestBuilder.build();
}

private <T> Flux<T> commitWrites(Publisher<T> instances, Function<T, Write> converterToWrite) {
return Flux.from(instances).bufferTimeout(this.writeBufferSize, this.writeBufferTimeout)
.flatMap(batch -> {
Expand Down Expand Up @@ -389,29 +369,6 @@ private void doIfTransaction(Context ctx, Consumer<ReactiveFirestoreResourceHold
});
}

private StreamObserver<WriteRequest> openWriteStream(StreamObserver<WriteResponse> obs) {
WriteRequest openStreamRequest =
WriteRequest.newBuilder().setDatabase(this.databasePath).build();
StreamObserver<WriteRequest> requestStreamObserver = this.firestore.write(obs);
requestStreamObserver.onNext(openStreamRequest);
return requestStreamObserver;
}

// Visible for Testing
<T> WriteRequest buildWriteRequest(List<T> entityList, WriteResponse writeResponse) {
WriteRequest.Builder writeRequestBuilder = WriteRequest.newBuilder();

if (isUsingStreamTokens()) {
writeRequestBuilder
.setStreamId(writeResponse.getStreamId())
.setStreamToken(writeResponse.getStreamToken());
}

entityList.stream().map(this::createUpdateWrite).forEach(writeRequestBuilder::addWrites);

return writeRequestBuilder.build();
}

private <T> Write createUpdateWrite(T entity) {
Builder builder = Write.newBuilder();
if (getIdValue(entity) == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,31 @@

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

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

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.firestore.v1.CommitRequest;
import com.google.firestore.v1.CommitResponse;
import com.google.firestore.v1.Document.Builder;
import com.google.firestore.v1.DocumentMask;
import com.google.firestore.v1.FirestoreGrpc.FirestoreStub;
import com.google.firestore.v1.GetDocumentRequest;
import com.google.firestore.v1.Precondition.ConditionTypeCase;
import com.google.firestore.v1.RunQueryRequest;
import com.google.firestore.v1.RunQueryResponse;
import com.google.firestore.v1.StructuredQuery;
import com.google.firestore.v1.Value;
import com.google.firestore.v1.WriteRequest;
import com.google.firestore.v1.WriteResponse;
import com.google.protobuf.ByteString;
import com.google.firestore.v1.Write;
import io.grpc.stub.StreamObserver;
import org.junit.Before;
import org.junit.Test;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
Expand All @@ -72,64 +66,6 @@ public void setup() {
new FirestoreDefaultClassMapper(), new FirestoreMappingContext());
}

@Test
public void excludeStreamTokensTest() {
firestoreTemplate.setUsingStreamTokens(false);

WriteResponse writeResponse =
WriteResponse.newBuilder()
.setStreamId("test-stream")
.setStreamToken(ByteString.copyFromUtf8("the-token"))
.build();

WriteRequest request = firestoreTemplate.buildWriteRequest(
Arrays.asList(new TestEntity("e1", 100L), new TestEntity(null, 10L)), writeResponse);

assertThat(request.getWrites(0).getCurrentDocument().getConditionTypeCase())
.isEqualTo(ConditionTypeCase.CONDITIONTYPE_NOT_SET);
assertThat(request.getWrites(1).getCurrentDocument().getConditionTypeCase())
.isEqualTo(ConditionTypeCase.EXISTS);
assertThat(request.getWrites(1).getCurrentDocument().getExists()).isFalse();
assertThat(request.getWrites(1).getUpdate().getName()).matches(".*/testEntities/[a-zA-Z0-9]{20}$");

assertThat(request.getStreamId()).isEmpty();
assertThat(request.getStreamToken().toStringUtf8()).isEmpty();

request = firestoreTemplate.buildDeleteRequest(Arrays.asList("hello"), writeResponse);
assertThat(request.getStreamId()).isEmpty();
assertThat(request.getStreamToken().toStringUtf8()).isEmpty();
}

@Test
public void includeStreamTokensTest() {
WriteResponse writeResponse =
WriteResponse.newBuilder()
.setStreamId("test-stream")
.setStreamToken(ByteString.copyFromUtf8("the-token"))
.build();

WriteRequest request = firestoreTemplate.buildWriteRequest(
Arrays.asList(new TestEntity("e1", 100L)), writeResponse);
assertThat(request.getStreamId()).isEqualTo("test-stream");
assertThat(request.getStreamToken().toStringUtf8()).isEqualTo("the-token");

request = firestoreTemplate.buildDeleteRequest(Arrays.asList("hello"), writeResponse);
assertThat(request.getStreamId()).isEqualTo("test-stream");
assertThat(request.getStreamToken().toStringUtf8()).isEqualTo("the-token");
}

@Test
public void testIntegerId() {

assertThatThrownBy(() -> {
List<TestEntityIntegerId> entities =
Collections.singletonList(new TestEntityIntegerId(1, "abc"));
firestoreTemplate.buildWriteRequest(entities, WriteResponse.newBuilder().build());
})
.isInstanceOf(FirestoreDataException.class)
.hasMessageContaining("An ID property is expected to be of String type; was class java.lang.Integer");
}

@Test
public void findAllTest() {
mockRunQueryMethod();
Expand All @@ -150,6 +86,54 @@ public void findAllTest() {
verify(this.firestoreStub, times(1)).runQuery(any(), any());
}

@Test
public void saveAllTest() {
mockCommitMethod();

StepVerifier.create(
this.firestoreTemplate
.saveAll(Flux.just(new TestEntity("e1", 100L), new TestEntity("e2", 200L))))
.expectNext(new TestEntity("e1", 100L), new TestEntity("e2", 200L))
.verifyComplete();

CommitRequest.Builder builder = CommitRequest.newBuilder()
.setDatabase("projects/my-project/databases/(default)");

builder.addWrites(Write.newBuilder().setUpdate(buildDocument("e1", 100)).build());
builder.addWrites(Write.newBuilder().setUpdate(buildDocument("e2", 200)).build());


verify(this.firestoreStub, times(1)).commit(eq(builder.build()), any());
}

@Test
public void deleteTest() {
mockCommitMethod();

StepVerifier.create(
this.firestoreTemplate
.delete(Flux.just(new TestEntity("e1", 100L), new TestEntity("e2", 200L))))
.verifyComplete();

CommitRequest.Builder builder = CommitRequest.newBuilder()
.setDatabase("projects/my-project/databases/(default)");

builder.addWrites(Write.newBuilder().setDelete(parent + "/testEntities/e1").build());
builder.addWrites(Write.newBuilder().setDelete(parent + "/testEntities/e2").build());

verify(this.firestoreStub, times(1)).commit(eq(builder.build()), any());
}


private void mockCommitMethod() {
doAnswer(invocation -> {
StreamObserver<CommitResponse> streamObserver = invocation.getArgument(1);
streamObserver.onNext(CommitResponse.newBuilder().build());
streamObserver.onCompleted();
return null;
}).when(this.firestoreStub).commit(any(), any());
}

@Test
public void findByIdTest() {
doAnswer(invocation -> {
Expand Down

0 comments on commit 8dcaf25

Please sign in to comment.