Copilot commented on code in PR #13215:
URL: https://github.com/apache/ignite/pull/13215#discussion_r3373745254


##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -151,214 +132,291 @@ public CacheOperationContext keepBinary() {
         return dataCenterId;
     }
 
-    /**
-     * @return Skip store.
-     */
-    public boolean skipStore() {
-        return skipStore;
+    /** Context with dataCenterId. */
+    public CacheOperationContext withDataCenterId(Byte dataCenterId) {
+        return builder(this).dataCenterId(dataCenterId).build();
     }
 
     /**
-     * See {@link IgniteInternalCache#setSkipStore(boolean)}.
-     *
-     * @param skipStore Skip store flag.
-     * @return New instance of CacheOperationContext with skip store flag.
+     * @return Partition recover flag.
      */
-    public CacheOperationContext setSkipStore(boolean skipStore) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    public boolean recovery() {
+        return recovery;
     }
 
-    /** @return Skip read-through cache store. */
-    public boolean skipReadThrough() {
-        return skipReadThrough;
+    /** Context with recovery flag. */
+    public CacheOperationContext withRecovery() {
+        return builder(this).recovery(true).build();
     }
 
     /**
-     * See {@link IgniteInternalCache#withApplicationAttributes(Map)}.
-     *
-     * @return New instance of CacheOperationContext with new application 
attributes.
+     * @return Read Repair strategy.
      */
-    public CacheOperationContext withApplicationAttributes(Map<String, String> 
attrs) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            Collections.unmodifiableMap(attrs));
+    @Nullable public ReadRepairStrategy readRepairStrategy() {
+        return readRepairStrategy;
     }
 
-    /**
-     * See {@link IgniteInternalCache#withSkipReadThrough()}.
-     *
-     * @return New instance of CacheOperationContext with skip store flag.
-     */
-    public CacheOperationContext withSkipReadThrough() {
-        return new CacheOperationContext(
-            skipStore,
-            true,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with read repair strategy. */
+    public CacheOperationContext withReadRepairStrategy(ReadRepairStrategy 
strategy) {
+        return builder(this).readRepairStrategy(strategy).build();
     }
 
     /**
-     * @return {@link ExpiryPolicy} associated with this projection.
+     * @return No retries flag.
      */
-    @Nullable public ExpiryPolicy expiry() {
-        return expiryPlc;
+    public boolean noRetries() {
+        return noRetries;
     }
 
-    /**
-     * See {@link IgniteInternalCache#withExpiryPolicy(ExpiryPolicy)}.
-     *
-     * @param plc {@link ExpiryPolicy} to associate with this projection.
-     * @return New instance of CacheOperationContext with skip store flag.
-     */
-    public CacheOperationContext withExpiryPolicy(ExpiryPolicy plc) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            plc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with noRetries flag. */
+    public CacheOperationContext withNoRetries() {
+        return builder(this).noRetries(true).build();
     }
 
     /**
-     * @param noRetries No retries flag.
-     * @return Operation context.
+     * @return Application attributes.
      */
-    public CacheOperationContext setNoRetries(boolean noRetries) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    @Nullable public Map<String, String> applicationAttributes() {
+        return appAttrs;
     }
 
-    /**
-     * @param dataCenterId Data center id.
-     * @return Operation context.
-     */
-    public CacheOperationContext setDataCenterId(byte dataCenterId) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with application attributes. */
+    public CacheOperationContext withApplicationAttributes(Map<String, String> 
attrs) {
+        return builder(this).applicationAttributes(attrs).build();
     }
 
     /**
-     * @param recovery Recovery flag.
-     * @return New instance of CacheOperationContext with recovery flag.
+     * @return Skip store.
      */
-    public CacheOperationContext setRecovery(boolean recovery) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    public boolean skipStore() {
+        return skipStore;
     }
 
-    /**
-     * @param readRepairStrategy Read Repair strategy.
-     * @return New instance of CacheOperationContext with Read Repair flag.
-     */
-    public CacheOperationContext setReadRepairStrategy(ReadRepairStrategy 
readRepairStrategy) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with skipStore flag. */
+    public CacheOperationContext withSkipStore() {
+        return builder(this).skipStore(true).build();
     }
 
     /**
-     * @param appAttrs Application attributes.
-     * @return New instance of CacheOperationContext with application 
attributes.
+     * @return Skip read-through cache store.
      */
-    public CacheOperationContext setApplicationAttributes(Map<String, String> 
appAttrs) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            new HashMap<>(appAttrs));
+    public boolean skipReadThrough() {
+        return skipReadThrough;
     }
 
-    /**
-     * @return Partition recover flag.
-     */
-    public boolean recovery() {
-        return recovery;
+    /** Context with {@link CacheOperationContext#skipReadThrough} flag. */
+    public CacheOperationContext withSkipReadThrough() {
+        return builder(this).skipReadThrough(true).build();
+    }
+
+    /** @return Calcite engine execution flag. */
+    public boolean calciteEngine() {
+        return calciteEngineCall;
+    }
+
+    /** Context with {@link CacheOperationContext#calciteEngine} flag. */
+    public CacheOperationContext withCalciteEngine() {
+        return builder(this).calciteEngine(true).build();
     }
 
     /**
-     * @return Read Repair strategy.
+     * @return {@link ExpiryPolicy} associated with this projection.
      */
-    public ReadRepairStrategy readRepairStrategy() {
-        return readRepairStrategy;
+    @Nullable public ExpiryPolicy expiry() {
+        return expiryPlc;
+    }
+
+    /** Context with {@link CacheOperationContext#expiryPlc}. */
+    public CacheOperationContext withExpiryPolicy(ExpiryPolicy plc) {
+        return builder(this).expiryPolicy(plc).build();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String toString() {
+        return S.toString(CacheOperationContext.class, this);
     }
 
     /**
-     * @return No retries flag.
+     * Creates the builder for cache operations context.
+     *
+     * @return Builder for cache operations context.
      */

Review Comment:
   The Javadoc for builder(CacheOperationContext ctx) says it "Creates the 
builder for cache operations context", but this overload actually creates a 
builder initialized from an existing context (the no-arg builder() is the one 
that creates an empty builder). Updating the Javadoc will avoid misleading API 
docs.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -151,214 +132,291 @@ public CacheOperationContext keepBinary() {
         return dataCenterId;
     }
 
-    /**
-     * @return Skip store.
-     */
-    public boolean skipStore() {
-        return skipStore;
+    /** Context with dataCenterId. */
+    public CacheOperationContext withDataCenterId(Byte dataCenterId) {
+        return builder(this).dataCenterId(dataCenterId).build();
     }
 
     /**
-     * See {@link IgniteInternalCache#setSkipStore(boolean)}.
-     *
-     * @param skipStore Skip store flag.
-     * @return New instance of CacheOperationContext with skip store flag.
+     * @return Partition recover flag.
      */
-    public CacheOperationContext setSkipStore(boolean skipStore) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    public boolean recovery() {
+        return recovery;
     }
 
-    /** @return Skip read-through cache store. */
-    public boolean skipReadThrough() {
-        return skipReadThrough;
+    /** Context with recovery flag. */
+    public CacheOperationContext withRecovery() {
+        return builder(this).recovery(true).build();
     }
 
     /**
-     * See {@link IgniteInternalCache#withApplicationAttributes(Map)}.
-     *
-     * @return New instance of CacheOperationContext with new application 
attributes.
+     * @return Read Repair strategy.
      */
-    public CacheOperationContext withApplicationAttributes(Map<String, String> 
attrs) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            Collections.unmodifiableMap(attrs));
+    @Nullable public ReadRepairStrategy readRepairStrategy() {
+        return readRepairStrategy;
     }
 
-    /**
-     * See {@link IgniteInternalCache#withSkipReadThrough()}.
-     *
-     * @return New instance of CacheOperationContext with skip store flag.
-     */
-    public CacheOperationContext withSkipReadThrough() {
-        return new CacheOperationContext(
-            skipStore,
-            true,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with read repair strategy. */
+    public CacheOperationContext withReadRepairStrategy(ReadRepairStrategy 
strategy) {
+        return builder(this).readRepairStrategy(strategy).build();
     }
 
     /**
-     * @return {@link ExpiryPolicy} associated with this projection.
+     * @return No retries flag.
      */
-    @Nullable public ExpiryPolicy expiry() {
-        return expiryPlc;
+    public boolean noRetries() {
+        return noRetries;
     }
 
-    /**
-     * See {@link IgniteInternalCache#withExpiryPolicy(ExpiryPolicy)}.
-     *
-     * @param plc {@link ExpiryPolicy} to associate with this projection.
-     * @return New instance of CacheOperationContext with skip store flag.
-     */
-    public CacheOperationContext withExpiryPolicy(ExpiryPolicy plc) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            plc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with noRetries flag. */
+    public CacheOperationContext withNoRetries() {
+        return builder(this).noRetries(true).build();
     }
 
     /**
-     * @param noRetries No retries flag.
-     * @return Operation context.
+     * @return Application attributes.
      */
-    public CacheOperationContext setNoRetries(boolean noRetries) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    @Nullable public Map<String, String> applicationAttributes() {
+        return appAttrs;
     }
 
-    /**
-     * @param dataCenterId Data center id.
-     * @return Operation context.
-     */
-    public CacheOperationContext setDataCenterId(byte dataCenterId) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with application attributes. */
+    public CacheOperationContext withApplicationAttributes(Map<String, String> 
attrs) {
+        return builder(this).applicationAttributes(attrs).build();
     }
 
     /**
-     * @param recovery Recovery flag.
-     * @return New instance of CacheOperationContext with recovery flag.
+     * @return Skip store.
      */
-    public CacheOperationContext setRecovery(boolean recovery) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    public boolean skipStore() {
+        return skipStore;
     }
 
-    /**
-     * @param readRepairStrategy Read Repair strategy.
-     * @return New instance of CacheOperationContext with Read Repair flag.
-     */
-    public CacheOperationContext setReadRepairStrategy(ReadRepairStrategy 
readRepairStrategy) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            appAttrs);
+    /** Context with skipStore flag. */
+    public CacheOperationContext withSkipStore() {
+        return builder(this).skipStore(true).build();
     }
 
     /**
-     * @param appAttrs Application attributes.
-     * @return New instance of CacheOperationContext with application 
attributes.
+     * @return Skip read-through cache store.
      */
-    public CacheOperationContext setApplicationAttributes(Map<String, String> 
appAttrs) {
-        return new CacheOperationContext(
-            skipStore,
-            skipReadThrough,
-            keepBinary,
-            expiryPlc,
-            noRetries,
-            dataCenterId,
-            recovery,
-            readRepairStrategy,
-            new HashMap<>(appAttrs));
+    public boolean skipReadThrough() {
+        return skipReadThrough;
     }
 
-    /**
-     * @return Partition recover flag.
-     */
-    public boolean recovery() {
-        return recovery;
+    /** Context with {@link CacheOperationContext#skipReadThrough} flag. */
+    public CacheOperationContext withSkipReadThrough() {
+        return builder(this).skipReadThrough(true).build();
+    }
+
+    /** @return Calcite engine execution flag. */
+    public boolean calciteEngine() {
+        return calciteEngineCall;
+    }
+
+    /** Context with {@link CacheOperationContext#calciteEngine} flag. */
+    public CacheOperationContext withCalciteEngine() {
+        return builder(this).calciteEngine(true).build();
     }
 
     /**
-     * @return Read Repair strategy.
+     * @return {@link ExpiryPolicy} associated with this projection.
      */
-    public ReadRepairStrategy readRepairStrategy() {
-        return readRepairStrategy;
+    @Nullable public ExpiryPolicy expiry() {
+        return expiryPlc;
+    }
+
+    /** Context with {@link CacheOperationContext#expiryPlc}. */
+    public CacheOperationContext withExpiryPolicy(ExpiryPolicy plc) {
+        return builder(this).expiryPolicy(plc).build();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String toString() {
+        return S.toString(CacheOperationContext.class, this);
     }
 
     /**
-     * @return No retries flag.
+     * Creates the builder for cache operations context.
+     *
+     * @return Builder for cache operations context.
      */
-    public boolean noRetries() {
-        return noRetries;
+    public static Builder builder(CacheOperationContext ctx) {
+        return new Builder(ctx);
     }
 
     /**
-     * @return Application attributes.
+     * Creates the builder from existing context.
+     *
+     * @return Builder for cache operations context.
      */

Review Comment:
   The Javadoc for the no-arg builder() says it "Creates the builder from 
existing context", but this overload actually creates an empty builder with 
default values. This looks swapped with the builder(CacheOperationContext) 
overload.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/rest/handlers/cache/GridCacheCommandHandler.java:
##########
@@ -751,8 +751,10 @@ private IgniteInternalFuture<GridRestResponse> 
executeCommand(
             destId == null || destId.equals(ctx.localNodeId()) || 
replicatedCacheAvailable(cacheName);
 
         if (locExec) {
-            IgniteInternalCache<?, ?> prj = localCache(cacheName)
-                .setSkipStore(cacheFlags.contains(SKIP_STORE));
+            IgniteInternalCache<?, ?> prj = localCache(cacheName);
+
+            if (cacheFlags.contains(SKIP_STORE))
+                prj = prj.withSkipStore();
 

Review Comment:
   This code path now uses IgniteInternalCache.withSkipStore(). There is an 
existing unit test 
(GridCacheCommandHandlerSelfTest.TestableCacheCommandHandler) that proxies 
IgniteInternalCache and specifically rewrites the result of invoking a method 
named "setSkipStore" to keep intercepting subsequent calls; with the rename, 
the proxy will no longer intercept and the test is likely to start failing. The 
test should be updated to look for "withSkipStore" instead.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to