[GitHub] [ignite-3] sk0x50 commented on a change in pull request #113: IGNITE-14670 Update metastorage manager with: range with applied revision, invoke for multiple updates.

[GitBox <gi...@apache.org>]

sk0x50 commented on a change in pull request #113:
URL: https://github.com/apache/ignite-3/pull/113#discussion_r625701920



##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Condition.java
##########
@@ -64,11 +64,16 @@ public boolean test(Entry e) {
          */
         private long rev;
 
+        /**
+         * Key of entry, which will be tested for condition.

Review comment:
       It should be a one-line comment.

##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Condition.java
##########
@@ -254,11 +263,16 @@ public Condition le(long rev) {
          */
         private byte[] val;
 
+        /**
+         * Key of entry, which will be tested for condition.

Review comment:
       Please use one-line Javadoc for such cases.

##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Conditions.java
##########
@@ -40,8 +53,12 @@
      * @return Condition on entry value.
      * @see Condition.ValueCondition
      */
-    public static Condition.ValueCondition value() {
-        return new Condition.ValueCondition();
+    public Condition.ValueCondition value() {
+        return new Condition.ValueCondition(key);
+    }
+
+    public static Conditions key(Key key) {

Review comment:
       Please add Javadoc here.

##########
File path: modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -303,15 +306,21 @@ public synchronized void deployWatches() {
     }
 
     /**
-     * @see MetaStorageService#invoke(Key, Condition, Operation, Operation)
+     * Invoke with single success/failure operation.
+     *
+     * @see MetaStorageService#invoke(Condition, Collection, Collection)
      */
-    public @NotNull CompletableFuture<Boolean> invoke(
-        @NotNull Key key,
-        @NotNull Condition cond,
-        @NotNull Operation success,
-        @NotNull Operation failure
-    ) {
-        return metaStorageSvc.invoke(key, cond, success, failure);
+    public @NotNull CompletableFuture<Boolean> invoke(@NotNull Condition cond,

Review comment:
       coding guideline does not allow that. please see https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodArguments
   
   I think it should be as follows:
   ```
       public @NotNull CompletableFuture<Boolean> invoke(
           @NotNull Condition cond,
           @NotNull Operation success,
           @NotNull Operation failure
       ) {
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org