alex-plekhanov commented on code in PR #13242:
URL: https://github.com/apache/ignite/pull/13242#discussion_r3441787868
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -108,38 +100,26 @@ public CacheOperationContext(
this.recovery = recovery;
this.readRepairStrategy = readRepairStrategy;
this.appAttrs = appAttrs;
+ this.handleBinaryInInterceptor = handleBinaryInInterceptor;
}
/**
- * @return Keep binary flag.
+ * Helper.
*/
- public boolean isKeepBinary() {
- return keepBinary;
+ public static CacheOperationContext instance() {
+ return INSTANCE;
}
/**
- * @return {@code True} if data center id is set otherwise {@code false}.
+ * @return keepBinary flag.
*/
- public boolean hasDataCenterId() {
- return dataCenterId != null;
+ public boolean isKeepBinary() {
+ return keepBinary;
}
- /**
- * See {@link IgniteInternalCache#keepBinary()}.
- *
- * @return New instance of CacheOperationContext with keep binary flag.
- */
- public CacheOperationContext keepBinary() {
- return new CacheOperationContext(
- skipStore,
- skipReadThrough,
- true,
- expiryPlc,
- noRetries,
- dataCenterId,
- recovery,
- readRepairStrategy,
- appAttrs);
+ /** Context with keepBinary flag. */
+ public CacheOperationContext withKeepBinary() {
+ return builder(this).keepBinary(true).build();
Review Comment:
Check for each method:
```
if (keepBinary)
return this;
```
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheOperationContext.java:
##########
@@ -151,214 +131,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 Whether to handle binary in interceptor. */
+ public boolean handleBinaryInInterceptor() {
+ return handleBinaryInInterceptor;
+ }
+
+ /** Context with {@link CacheOperationContext#handleBinaryInInterceptor}
flag. */
+ public CacheOperationContext withHandleBinaryInInterceptor() {
+ return builder(this).handleBinaryInInterceptor(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 from existing 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 for cache operations 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
Review Comment:
Why only these three fields marked as GridToStringInclude? AFAIK like this
annotation is redundant for booleans and we can remove it (and from
CacheOperationContext too). Let's check it.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##########
@@ -466,59 +466,53 @@ 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 = 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 = opCtx.withSkipReadThrough();
}
Review Comment:
Maybe replace this boilerplate code in cache adapters/cache proxies with
something like:
```
opCtx = CacheOperationContext.of(opCtx).withSkipReadThrough();
```
?
And implement `CacheOperationContext.of` like:
```
return opCtx == null ? INSTANCE : opCtx;
```
Count of allocated objects will be the same (one builder and one context
object). Perhaps builder can be marked as private after such a change.
--
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]
