Copilot commented on code in PR #13215:
URL: https://github.com/apache/ignite/pull/13215#discussion_r3363769837
##########
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 = opCtx.withSkipStore();
Review Comment:
`opCtx` is a mutable field of the proxy; reassigning it here changes the
state of the current cache projection as a side effect of calling
`withSkipStore()`. Projection methods should not mutate the original proxy;
they should create a new `CacheOperationContext` variable and pass it to the
new proxy instance (apply the same pattern to other projection methods that now
assign to `opCtx`).
##########
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 = 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 = opCtx.withKeepBinary();
Review Comment:
`keepBinary()` currently mutates the proxy's `opCtx` field before returning
a new proxy. This can unintentionally enable keep-binary mode on the original
projection as a side effect. Use a local `newOpCtx` variable (and consider
updating other projection methods that now assign to `opCtx` similarly).
##########
modules/core/src/test/java/org/apache/ignite/util/TestStorageUtils.java:
##########
@@ -29,6 +29,9 @@
import
org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtLocalPartition;
import
org.apache.ignite.internal.processors.cache.persistence.IgniteCacheDatabaseSharedManager;
import org.apache.ignite.internal.processors.cache.version.GridCacheVersion;
+import org.jetbrains.annotations.Nullable;
+
+import static org.junit.Assert.assertNotNull;
Review Comment:
These imports are currently unused in this file. If the project is built
with unused-import checks (common in Ignite), this will fail the build; please
remove them or use them.
--
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]
