Copilot commented on code in PR #12911:
URL: https://github.com/apache/ignite/pull/12911#discussion_r3009110470
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -504,6 +504,28 @@ public void active(boolean active) {
return new GridCacheProxyImpl<>(this.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 = this.ctx.operationContextPerCall();
+
+ if (opCtx == null) {
+ opCtx = new CacheOperationContext(
+ false,
+ false,
+ false,
+ null,
+ false,
+ null,
+ false,
+ null,
+ Collections.unmodifiableMap(attrs));
+ }
Review Comment:
When `opCtx == null`, this method stores
`Collections.unmodifiableMap(attrs)` directly in `CacheOperationContext`. This
is not a defensive copy (the backing `attrs` can still be mutated by the caller
later), which can lead to surprising/session-unsafe behavior. Consider copying
the map first (and then wrapping) or reusing
`CacheOperationContext#setApplicationAttributes` semantics, and validate
`attrs` is non-null to avoid accidental NPEs.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java:
##########
@@ -303,7 +303,7 @@ public IgniteInternalCache<K, V> delegate() {
}
/** @return New internal cache instance based on this one, but with
application attributes. */
- public GridCacheProxyImpl<K, V> withApplicationAttributes(Map<String,
String> attrs) {
+ @Override public GridCacheProxyImpl<K, V>
withApplicationAttributes(Map<String, String> attrs) {
CacheOperationContext prev = gate.enter(opCtx);
try {
Review Comment:
In `withApplicationAttributes`, the `opCtx == null` branch creates a new
`CacheOperationContext` with `skipReadThrough` set to `true` (second boolean
argument). This unintentionally changes read-through behavior when only
application attributes are requested, and it’s inconsistent with
`GridCacheAdapter.withApplicationAttributes` (which keeps
`skipReadThrough=false`). Please fix the constructor arguments so this
projection only adds attributes, and also make `attrs` a defensive copy to
avoid later external mutation affecting the context.
--
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]
