Merge pull request #286 from alex268/master

Removed ReadyOperation implementation
ydb-platform · Jun 21, 2024 · 55cbfdd · 55cbfdd
2 parents 0744b6b + cc2e97a
commit 55cbfdd
Show file tree

Hide file tree

Showing 7 changed files with 93 additions and 115 deletions.
diff --git a/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java b/core/src/main/java/tech/ydb/core/grpc/GrpcTransportBuilder.java
@@ -160,6 +160,10 @@ public GrpcCompression getGrpcCompression() {
         return compression;
     }
 
+    public InitMode getInitMode() {
+        return initMode;
+    }
+
     public boolean isEnableRetry() {
         return grpcRetry;
     }

diff --git a/core/src/main/java/tech/ydb/core/operation/OperationBinder.java b/core/src/main/java/tech/ydb/core/operation/OperationBinder.java
@@ -86,11 +86,7 @@ public static <R> Function<Result<R>, Operation<Status>> bindAsync(
             }
 
             OperationProtos.Operation operation = method.apply(result.getValue());
-            if (operation.getReady()) {
-                return new ReadyOperation<>(operation.getId(), status(operation));
-            }
-
-            return new OperationImpl<>(transport, operation.getId(), OperationBinder::status);
+            return new OperationImpl<>(transport, operation, OperationBinder::status);
         };
     }
 
@@ -104,11 +100,7 @@ public static <R, M extends Message> Function<Result<R>, Operation<Result<M>>> b
             }
 
             OperationProtos.Operation operation = method.apply(result.getValue());
-            if (operation.getReady()) {
-                return new ReadyOperation<>(operation.getId(), result(operation, resultClass));
-            }
-
-            return new OperationImpl<>(transport, operation.getId(), o -> result(o, resultClass));
+            return new OperationImpl<>(transport, operation, o -> result(o, resultClass));
         };
     }
 }
diff --git a/core/src/main/java/tech/ydb/core/operation/OperationImpl.java b/core/src/main/java/tech/ydb/core/operation/OperationImpl.java
@@ -31,10 +31,13 @@ class OperationImpl<T> implements AsyncOperation<T> {
     private final Function<OperationProtos.Operation, T> valueExtractor;
     private volatile T value = null;
 
-    OperationImpl(GrpcTransport transport, String id, Function<OperationProtos.Operation, T> extractor) {
-        this.transport = transport;
-        this.id = id;
-        this.valueExtractor = extractor;
+    OperationImpl(GrpcTransport tr, OperationProtos.Operation op, Function<OperationProtos.Operation, T> ve) {
+        this.transport = tr;
+        this.id = op.getId();
+        this.valueExtractor = ve;
+        if (op.getReady()) {
+            this.value = ve.apply(op);
+        }
     }
 
     @Override
@@ -59,7 +62,7 @@ public T getValue() {
 
     @Override
     public String toString() {
-        return "AsyncOperation{id=" + id + ", ready=" + (value != null) + "}";
+        return "Operation{id=" + id + ", ready=" + (value != null) + "}";
     }
 
     @Override
@@ -88,10 +91,6 @@ public CompletableFuture<Status> forget() {
 
     @Override
     public CompletableFuture<Result<Boolean>> fetch() {
-        if (value != null) {
-            return CompletableFuture.completedFuture(Result.success(Boolean.TRUE));
-        }
-
         GrpcRequestSettings settings = GrpcRequestSettings.newBuilder().build();
         OperationProtos.GetOperationRequest request = OperationProtos.GetOperationRequest.newBuilder()
                         .setId(id)
@@ -169,7 +168,7 @@ public <Z> Operation<Z> transform(Function<R, Z> func) {
 
         @Override
         public String toString() {
-            return "ProxyAsyncOperation{id=" + id + ", ready=" + (value != null) + "}";
+            return "OperationProxy{id=" + id + ", ready=" + (value != null) + "}";
         }
     }
 }
diff --git a/core/src/main/java/tech/ydb/core/operation/ReadyOperation.java b/core/src/main/java/tech/ydb/core/operation/ReadyOperation.java
diff --git a/core/src/test/java/tech/ydb/core/operation/OperationBinderTest.java b/core/src/test/java/tech/ydb/core/operation/OperationBinderTest.java
@@ -1,5 +1,6 @@
 package tech.ydb.core.operation;
 
+
 import com.google.protobuf.Any;
 import org.junit.Assert;
 import org.junit.Test;
@@ -203,14 +204,13 @@ public void syncResultBinderMixClassesTest() {
     @Test
     public void asyncStatusBinderTest() {
         YdbTable.AlterTableResponse response = YdbTable.AlterTableResponse.newBuilder()
-                .setOperation(
-                        OperationProtos.Operation.newBuilder()
-                                .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS)
-                                .setId("ready-id")
-                                .setReady(true)
-                                .setCostInfo(CommonProtos.CostInfo.newBuilder().setConsumedUnits(15d).build())
-                                .build())
-                .build();
+                .setOperation(OperationProtos.Operation.newBuilder()
+                        .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS)
+                        .setId("ready-id")
+                        .setReady(true)
+                        .setCostInfo(CommonProtos.CostInfo.newBuilder().setConsumedUnits(15d).build())
+                        .build()
+                ).build();
 
         Operation<Status> operation = OperationBinder
                 .bindAsync(mocked, YdbTable.AlterTableResponse::getOperation)
@@ -225,10 +225,6 @@ public void asyncStatusBinderTest() {
         Assert.assertTrue(status.hasConsumedRu());
         Assert.assertEquals(Double.valueOf(15d), status.getConsumedRu());
         Assert.assertEquals(0, status.getIssues().length);
-
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.cancel().join());
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.forget().join());
-        Assert.assertTrue(operation.fetch().join().getValue());
     }
 
     @Test
@@ -254,10 +250,6 @@ public void asyncStatusBinderFailTest() {
         Status status = operation.getValue();
         Assert.assertNotNull(status);
         Assert.assertEquals(error, status);
-
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.cancel().join());
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.forget().join());
-        Assert.assertTrue(operation.fetch().join().getValue());
     }
 
     @Test
@@ -322,10 +314,6 @@ public void asyncResultBinderTest() {
         Assert.assertTrue(result.getStatus().hasConsumedRu());
         Assert.assertEquals(Double.valueOf(15d), result.getStatus().getConsumedRu());
         Assert.assertEquals(0, result.getStatus().getIssues().length);
-
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.cancel().join());
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.forget().join());
-        Assert.assertTrue(operation.fetch().join().getValue());
     }
 
     @Test
@@ -351,10 +339,6 @@ public void asyncResultBinderFailTest() {
         Result<YdbTable.ExplainQueryResult> result = operation.getValue();
         Assert.assertNotNull(result);
         Assert.assertEquals(error, result.getStatus());
-
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.cancel().join());
-        Assert.assertEquals(ReadyOperation.ALREADY_DONE_STATUS, operation.forget().join());
-        Assert.assertTrue(operation.fetch().join().getValue());
     }
 
     @Test

diff --git a/core/src/test/java/tech/ydb/core/operation/OperationImplTest.java b/core/src/test/java/tech/ydb/core/operation/OperationImplTest.java
@@ -0,0 +1,70 @@
+package tech.ydb.core.operation;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ScheduledExecutorService;
+
+import com.google.protobuf.Any;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import tech.ydb.core.Result;
+import tech.ydb.core.StatusCode;
+import tech.ydb.core.grpc.GrpcTransport;
+import tech.ydb.proto.OperationProtos;
+import tech.ydb.proto.StatusCodesProtos;
+import tech.ydb.proto.operation.v1.OperationServiceGrpc;
+
+/**
+ *
+ * @author Aleksandr Gorshenin
+ */
+public class OperationImplTest {
+    private final GrpcTransport mocked = Mockito.mock(GrpcTransport.class);
+    private final ScheduledExecutorService scheduler = Mockito.mock(ScheduledExecutorService.class);
+
+    @Before
+    public void setup() {
+        Mockito.when(mocked.getScheduler()).thenReturn(scheduler);
+    }
+
+    @Test
+    public void asyncNotReadyOperationTest() {
+        OperationProtos.Operation operation = OperationProtos.Operation.newBuilder()
+                .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS)
+                .setId("test-id")
+                .setReady(false)
+                .build();
+
+        Mockito.when(mocked.unaryCall(
+                Mockito.eq(OperationServiceGrpc.getCancelOperationMethod()), Mockito.any(), Mockito.any()
+        )).thenReturn(CompletableFuture.completedFuture(Result.success(
+                OperationProtos.CancelOperationResponse.newBuilder()
+                        .setStatus(StatusCodesProtos.StatusIds.StatusCode.SUCCESS)
+                        .build()
+        )));
+        Mockito.when(mocked.unaryCall(
+                Mockito.eq(OperationServiceGrpc.getForgetOperationMethod()), Mockito.any(), Mockito.any()
+        )).thenReturn(CompletableFuture.completedFuture(Result.success(
+                OperationProtos.ForgetOperationResponse.newBuilder()
+                        .setStatus(StatusCodesProtos.StatusIds.StatusCode.BAD_REQUEST)
+                        .build()
+        )));
+        Mockito.when(mocked.unaryCall(
+                Mockito.eq(OperationServiceGrpc.getGetOperationMethod()), Mockito.any(), Mockito.any()
+        )).thenReturn(CompletableFuture.completedFuture(Result.success(
+                OperationProtos.GetOperationResponse.newBuilder().setOperation(operation).build()
+        )));
+
+        AsyncOperation<Any> o = new OperationImpl<>(mocked, operation, OperationProtos.Operation::getResult);
+
+        Assert.assertEquals(scheduler, o.getScheduler());
+        Assert.assertEquals("Operation{id=test-id, ready=false}", o.toString());
+        Assert.assertFalse(o.isReady());
+        Assert.assertNull(o.getValue());
+
+        Assert.assertEquals(StatusCode.BAD_REQUEST, o.forget().join().getCode());
+        Assert.assertEquals(StatusCode.SUCCESS, o.cancel().join().getCode());
+    }
+}
diff --git a/core/src/test/java/tech/ydb/core/operation/OperationTrayTest.java b/core/src/test/java/tech/ydb/core/operation/OperationTrayTest.java
@@ -42,11 +42,6 @@ public class OperationTrayTest {
     @Before
     public void prepare() {
         Mockito.when(transport.getScheduler()).thenReturn(scheduler);
-//        Mockito.when(scheduler.schedule(Mockito.any(Runnable.class), Mockito.anyInt(), Mockito.any())).thenAnswer(
-//                (InvocationOnMock iom) -> {
-//                    scheduledActions.offer(iom.getArgument(0, Runnable.class));
-//                    return null;
-//                });
     }
 
     @Test