Copilot commented on code in PR #13215:
URL: https://github.com/apache/ignite/pull/13215#discussion_r3363324019
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -251,25 +251,20 @@ public IgniteInternalCache<K, V> delegate() {
}
/** {@inheritDoc} */
- @Override public GridCacheProxyImpl<K, V> setSkipStore(boolean skipStore) {
+ @Override public GridCacheProxyImpl<K, V> withSkipStore() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- if (opCtx != null && opCtx.skipStore() == skipStore)
- return this;
+ if (opCtx != null) {
+ if (opCtx.skipStore())
+ return this;
+ else
+ opCtx.withSkipStore();
+ }
Review Comment:
`CacheOperationContext.withSkipStore()` returns a new immutable context, but
the returned value is ignored here, so the skip-store flag is never applied
when `opCtx` already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -281,21 +276,16 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- if (opCtx != null && opCtx.skipReadThrough())
- return this;
+ if (opCtx != null) {
+ if (opCtx.skipReadThrough())
+ return this;
+ else
+ opCtx.withSkipReadThrough();
+ }
Review Comment:
`CacheOperationContext.withSkipReadThrough()` returns a new immutable
context, but the returned value is ignored here, so the flag is never applied
when `opCtx` already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -307,41 +297,30 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- return new GridCacheProxyImpl<>(ctx, delegate,
- opCtx != null ? opCtx.setApplicationAttributes(attrs) :
- new CacheOperationContext(
- false,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- attrs));
+ if (opCtx != null)
+ opCtx.withApplicationAttributes(attrs);
+ else
+ opCtx =
CacheOperationContext.builder().applicationAttributes(attrs).build();
+
Review Comment:
`CacheOperationContext.withApplicationAttributes(...)` returns a new
context, but the returned value is ignored, so application attributes won't be
propagated when `opCtx` is already present (e.g., after `keepBinary()` /
`withCalciteEngine()`).
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -1581,18 +1560,12 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- return new GridCacheProxyImpl<>(ctx, delegate,
- opCtx != null ? opCtx.withExpiryPolicy(plc) :
- new CacheOperationContext(
- false,
- false,
- false,
- plc,
- false,
- null,
- false,
- null,
- null));
+ if (opCtx != null)
+ opCtx.withExpiryPolicy(plc);
+ else
+ opCtx =
CacheOperationContext.builder().expiryPolicy(plc).build();
+
Review Comment:
`CacheOperationContext.withExpiryPolicy(...)` returns a new immutable
context, but the returned value is ignored. This means expiry policy isn't
actually applied when `opCtx` is already present.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -307,41 +297,30 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- return new GridCacheProxyImpl<>(ctx, delegate,
- opCtx != null ? opCtx.setApplicationAttributes(attrs) :
- new CacheOperationContext(
- false,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- attrs));
+ if (opCtx != null)
+ opCtx.withApplicationAttributes(attrs);
+ else
+ opCtx =
CacheOperationContext.builder().applicationAttributes(attrs).build();
+
+ return new GridCacheProxyImpl<>(ctx, delegate, opCtx);
}
finally {
gate.leave(prev);
}
}
/** {@inheritDoc} */
- @Override public <K1, V1> GridCacheProxyImpl<K1, V1> keepBinary() {
- if (opCtx != null && opCtx.isKeepBinary())
- return (GridCacheProxyImpl<K1, V1>)this;
+ @Override public GridCacheProxyImpl<K, V> keepBinary() {
+ if (opCtx != null) {
+ if (opCtx.isKeepBinary())
+ return this;
+ else
+ opCtx.withKeepBinary();
+ }
Review Comment:
`CacheOperationContext.withKeepBinary()` returns a new immutable context,
but the returned value is ignored here. As a result, `keepBinary()` may return
a proxy without the keep-binary flag when `opCtx` is already set.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -1604,17 +1577,37 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- return new GridCacheProxyImpl<>(ctx, delegate,
- new CacheOperationContext(
- false,
- false,
- false,
- null,
- true,
- null,
- false,
- null,
- null));
+ if (opCtx != null) {
+ if (opCtx.noRetries())
+ return this;
+ else
+ opCtx.withNoRetries();
+ }
+ else
+ opCtx =
CacheOperationContext.builder().noRetries(true).build();
+
+ return new GridCacheProxyImpl<>(ctx, delegate, opCtx);
+ }
+ finally {
+ gate.leave(prev);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override public IgniteInternalCache<K, V> withCalciteEngine() {
+ CacheOperationContext prev = gate.enter(opCtx);
+
+ try {
+ if (opCtx != null) {
+ if (opCtx.calciteEngine())
+ return this;
+ else
+ opCtx.withCalciteEngine();
+ }
Review Comment:
`CacheOperationContext.withCalciteEngine()` returns a new immutable context,
but the returned value is ignored here, so the Calcite-engine flag won't be
applied when `opCtx` already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -1604,17 +1577,37 @@ public IgniteInternalCache<K, V> delegate() {
CacheOperationContext prev = gate.enter(opCtx);
try {
- return new GridCacheProxyImpl<>(ctx, delegate,
- new CacheOperationContext(
- false,
- false,
- false,
- null,
- true,
- null,
- false,
- null,
- null));
+ if (opCtx != null) {
+ if (opCtx.noRetries())
+ return this;
+ else
+ opCtx.withNoRetries();
+ }
Review Comment:
`CacheOperationContext.withNoRetries()` returns a new immutable context, but
the returned value is ignored here, so no-retries mode isn't enabled when
`opCtx` already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -466,79 +466,65 @@ public void active(boolean active) {
}
/** {@inheritDoc} */
- @Override public final GridCacheProxyImpl<K, V> setSkipStore(boolean
skipStore) {
- CacheOperationContext opCtx = new CacheOperationContext(
- true,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ @Override public IgniteInternalCache<K, V> withSkipStore() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().skipStore(true).build();
+ else {
+ if (!opCtx.skipStore())
+ opCtx.withSkipStore();
+ }
return new GridCacheProxyImpl<>(ctx, this, opCtx);
}
/** {@inheritDoc} */
@Override public IgniteInternalCache<K, V> withSkipReadThrough() {
- CacheOperationContext opCtx = this.ctx.operationContextPerCall();
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
- if (opCtx == null) {
- opCtx = new CacheOperationContext(
- false,
- true,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().skipReadThrough(true).build();
+ else {
+ if (!opCtx.skipReadThrough())
+ opCtx.withSkipReadThrough();
+ }
Review Comment:
`CacheOperationContext.withSkipReadThrough()` returns a new immutable
context, but the result is ignored here, so `withSkipReadThrough()` becomes a
no-op when an operation context already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -466,79 +466,65 @@ public void active(boolean active) {
}
/** {@inheritDoc} */
- @Override public final GridCacheProxyImpl<K, V> setSkipStore(boolean
skipStore) {
- CacheOperationContext opCtx = new CacheOperationContext(
- true,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ @Override public IgniteInternalCache<K, V> withSkipStore() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().skipStore(true).build();
+ else {
+ if (!opCtx.skipStore())
+ opCtx.withSkipStore();
+ }
return new GridCacheProxyImpl<>(ctx, this, opCtx);
}
/** {@inheritDoc} */
@Override public IgniteInternalCache<K, V> withSkipReadThrough() {
- CacheOperationContext opCtx = this.ctx.operationContextPerCall();
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
- if (opCtx == null) {
- opCtx = new CacheOperationContext(
- false,
- true,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().skipReadThrough(true).build();
+ else {
+ if (!opCtx.skipReadThrough())
+ opCtx.withSkipReadThrough();
+ }
+
+ return new GridCacheProxyImpl<>(ctx, this, opCtx);
+ }
+
+ /** {@inheritDoc} */
+ @Override public IgniteInternalCache<K, V> withCalciteEngine() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().calciteEngine(true).build();
+ else {
+ if (!opCtx.calciteEngine())
+ opCtx.withCalciteEngine();
}
Review Comment:
`CacheOperationContext.withCalciteEngine()` returns a new immutable context,
but the result is ignored here. This prevents the Calcite-engine flag from
being applied when an operation context already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -466,79 +466,65 @@ public void active(boolean active) {
}
/** {@inheritDoc} */
- @Override public final GridCacheProxyImpl<K, V> setSkipStore(boolean
skipStore) {
- CacheOperationContext opCtx = new CacheOperationContext(
- true,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ @Override public IgniteInternalCache<K, V> withSkipStore() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().skipStore(true).build();
+ else {
+ if (!opCtx.skipStore())
+ opCtx.withSkipStore();
+ }
return new GridCacheProxyImpl<>(ctx, this, opCtx);
}
/** {@inheritDoc} */
@Override public IgniteInternalCache<K, V> withSkipReadThrough() {
- CacheOperationContext opCtx = this.ctx.operationContextPerCall();
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
- if (opCtx == null) {
- opCtx = new CacheOperationContext(
- false,
- true,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().skipReadThrough(true).build();
+ else {
+ if (!opCtx.skipReadThrough())
+ opCtx.withSkipReadThrough();
+ }
+
+ return new GridCacheProxyImpl<>(ctx, this, opCtx);
+ }
+
+ /** {@inheritDoc} */
+ @Override public IgniteInternalCache<K, V> withCalciteEngine() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().calciteEngine(true).build();
+ else {
+ if (!opCtx.calciteEngine())
+ opCtx.withCalciteEngine();
}
- else
- opCtx = opCtx.withSkipReadThrough();
- return new GridCacheProxyImpl<>(this.ctx, this, opCtx);
+ return new GridCacheProxyImpl<>(ctx, this, opCtx);
}
+
/** @return New internal cache instance based on this one, but with
application attributes. */
@Override public GridCacheProxyImpl<K, V>
withApplicationAttributes(Map<String, String> attrs) {
CacheOperationContext opCtx = ctx.operationContextPerCall();
- if (opCtx == null) {
- opCtx = new CacheOperationContext(
- false,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- new HashMap<>(attrs));
- }
+ if (opCtx == null)
+ opCtx =
CacheOperationContext.builder().applicationAttributes(attrs).build();
else
- opCtx = opCtx.withApplicationAttributes(attrs);
+ opCtx.withApplicationAttributes(attrs);
Review Comment:
`CacheOperationContext.withApplicationAttributes(...)` returns a new
immutable context, but the result is ignored, so attributes are not applied
when an operation context already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/datastructures/GridCacheQueueAdapter.java:
##########
@@ -433,17 +433,10 @@ protected GridCacheQueueAdapter(String queueName,
GridCacheQueueHeader hdr, Grid
if (opCtx != null && opCtx.isKeepBinary())
return (GridCacheQueueAdapter<V1>)this;
- opCtx = opCtx == null ? new CacheOperationContext(
- false,
- false,
- true,
- null,
- false,
- null,
- false,
- null,
- null)
- : opCtx.keepBinary();
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().keepBinary(true).build();
+ else
+ opCtx.withKeepBinary();
Review Comment:
`CacheOperationContext.withKeepBinary()` returns a new immutable context,
but the returned value is ignored, so the keep-binary flag is not actually
enabled when `opCtx` already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -550,32 +536,21 @@ public void active(boolean active) {
@Override public final GridCacheProxyImpl<K, V>
withExpiryPolicy(ExpiryPolicy plc) {
assert !CU.isUtilityCache(ctx.name());
- CacheOperationContext opCtx = new CacheOperationContext(
- false,
- false,
- false,
- plc,
- false,
- null,
- false,
- null,
- null);
+ CacheOperationContext opCtx =
CacheOperationContext.builder().expiryPolicy(plc).build();
return new GridCacheProxyImpl<>(ctx, this, opCtx);
}
/** {@inheritDoc} */
@Override public final IgniteInternalCache<K, V> withNoRetries() {
- CacheOperationContext opCtx = new CacheOperationContext(
- false,
- false,
- false,
- null,
- true,
- null,
- false,
- null,
- null);
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().noRetries(true).build();
+ else {
+ if (!opCtx.noRetries())
+ opCtx.withNoRetries();
+ }
Review Comment:
`CacheOperationContext.withNoRetries()` returns a new immutable context, but
the result is ignored here. This means `withNoRetries()` becomes a no-op when
an operation context already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -151,214 +128,291 @@ public CacheOperationContext keepBinary() {
return dataCenterId;
}
- /**
- * @return Skip store.
- */
- public boolean skipStore() {
- return skipStore;
+ /** Context with dataCenterId. */
+ public CacheOperationContext dataCenterId(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 readRepairStrategy(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(Collections.unmodifiableMap(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.
*/
- public Map<String, String> applicationAttributes() {
- return appAttrs;
+ public static Builder builder() {
+ return new Builder();
}
- /** {@inheritDoc} */
- @Override public String toString() {
- return S.toString(CacheOperationContext.class, this);
+ /** Cache operations context builder. */
+ public static class Builder {
+ /** Skip store. */
+ @GridToStringInclude
+ private boolean skipStore;
+
+ /** Skip read through. */
+ @GridToStringInclude
+ private boolean skipReadThrough;
+
+ /** No retries flag. */
+ @GridToStringInclude
+ private boolean noRetries;
+
+ /** Recovery flag. */
+ private boolean recovery;
+
+ /** Read-repair strategy. */
+ private ReadRepairStrategy readRepairStrategy;
+
+ /** Keep binary flag. */
+ private boolean keepBinary;
+
+ /** Expiry policy. */
+ private ExpiryPolicy expiryPlc;
+
+ /** Data center Id. */
+ private Byte dataCenterId;
+
+ /** Application attributes. */
+ private Map<String, String> appAttrs;
+
+ /** Calcite engine execution flag. */
+ private boolean calciteEngine;
+
+ /** */
+ Builder() {
+ // No context.
+ }
+
+ /** */
+ Builder(CacheOperationContext ctx) {
+ skipStore = ctx.skipStore;
+ skipReadThrough = ctx.skipReadThrough;
+ noRetries = ctx.noRetries;
+ recovery = ctx.recovery;
+ readRepairStrategy = ctx.readRepairStrategy;
+ keepBinary = ctx.keepBinary;
+ expiryPlc = ctx.expiryPlc;
+ dataCenterId = ctx.dataCenterId;
+ appAttrs = ctx.appAttrs;
+ calciteEngine = ctx.calciteEngineCall;
+ }
+
+ /**
+ * CacheOperationContext with keepBinary flag.
+ *
+ * @see IgniteInternalCache#keepBinary()
+ */
+ public Builder keepBinary(boolean keepBinary) {
+ this.keepBinary = keepBinary;
+ return this;
+ }
+
+ /**
+ * CacheOperationContext with skipStore flag.
+ *
+ * @see IgniteInternalCache#skipStore()
+ */
+ public Builder skipStore(boolean skipStore) {
+ this.skipStore = skipStore;
+ return this;
+ }
+
+ /**
+ * CacheOperationContext with attributes.
+ *
+ * @see IgniteInternalCache#withApplicationAttributes(Map)
+ */
+ public Builder applicationAttributes(Map<String, String> attrs) {
+ appAttrs = Collections.unmodifiableMap(attrs);
+ return this;
+ }
Review Comment:
`applicationAttributes(...)` stores an unmodifiable *view* of the provided
map, so later mutations of the original map can leak into the operation
context. This is a behavior change vs copying and can cause hard-to-debug
concurrency issues.
##########
modules/core/src/test/java/org/apache/ignite/util/TestStorageUtils.java:
##########
@@ -43,15 +44,15 @@ public class TestStorageUtils {
* @param breakData Break data.
*/
public static void corruptDataEntry(
- GridCacheContext<?, ?> ctx,
+ GridCacheContext ctx,
Object key,
boolean breakCntr,
boolean breakData
) throws IgniteCheckedException {
int partId = ctx.affinity().partition(key);
GridDhtLocalPartition locPart = ctx.topology().localPartition(partId);
- CacheEntry<Object, Object> e = ctx.cache().keepBinary().getEntry(key);
+ @Nullable CacheEntry<?, ?> e = ctx.cache().keepBinary().getEntry(key);
Review Comment:
`e` is annotated as nullable, but it is dereferenced immediately
(`e.getKey()`, `e.getValue()`). Either handle the missing-entry case explicitly
or assert it cannot happen here.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -466,79 +466,65 @@ public void active(boolean active) {
}
/** {@inheritDoc} */
- @Override public final GridCacheProxyImpl<K, V> setSkipStore(boolean
skipStore) {
- CacheOperationContext opCtx = new CacheOperationContext(
- true,
- false,
- false,
- null,
- false,
- null,
- false,
- null,
- null);
+ @Override public IgniteInternalCache<K, V> withSkipStore() {
+ CacheOperationContext opCtx = ctx.operationContextPerCall();
+
+ if (opCtx == null)
+ opCtx = CacheOperationContext.builder().skipStore(true).build();
+ else {
+ if (!opCtx.skipStore())
+ opCtx.withSkipStore();
+ }
Review Comment:
`CacheOperationContext.withSkipStore()` returns a new immutable context, but
the result is ignored here, so `withSkipStore()` becomes a no-op when an
operation context already exists.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -151,214 +128,291 @@ public CacheOperationContext keepBinary() {
return dataCenterId;
}
- /**
- * @return Skip store.
- */
- public boolean skipStore() {
- return skipStore;
+ /** Context with dataCenterId. */
+ public CacheOperationContext dataCenterId(Byte dataCenterId) {
+ return builder(this).dataCenterId(dataCenterId).build();
}
Review Comment:
`dataCenterId(Byte)` accepts a boxed type but delegates to
`Builder.dataCenterId(byte)`. Passing `null` will throw an NPE via
auto-unboxing, which is surprising for this API.
--
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]
