abashev commented on code in PR #12889:
URL: https://github.com/apache/ignite/pull/12889#discussion_r2966145697
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheEntryPredicateAdapter.java:
##########
@@ -144,27 +147,65 @@ public PredicateType type() {
/** */
public byte code() {
- assert type != null;
+ return code;
+ }
+ /** */
+ public void code(byte code) {
+ this.code = code;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void prepareMarshal(Marshaller marsh) throws
IgniteCheckedException {
switch (type) {
- case OTHER: return 0;
- case VALUE: return 1;
- case HAS_VALUE: return 2;
- case HAS_NO_VALUE: return 3;
- case ALWAYS_FALSE: return 4;
- }
+ case OTHER:
+ code = 0;
Review Comment:
Why not to store only enum or make a code a part for enum itself?
##########
modules/core/src/test/java/org/apache/ignite/internal/managers/communication/CacheEntryPredicateAdapterMessageTest.java:
##########
@@ -32,56 +35,88 @@ public class CacheEntryPredicateAdapterMessageTest {
/** */
@Test
public void testCacheEntryPredicateAdapterCode() {
- assertEquals(0, new CacheEntryPredicateAdapter().code());
- assertEquals(0, new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.OTHER).code());
- assertEquals(1, new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.VALUE).code());
- assertEquals(1, new
CacheEntryPredicateAdapter((CacheObject)null).code());
- assertEquals(2, new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.HAS_VALUE).code());
- assertEquals(3, new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.HAS_NO_VALUE).code());
- assertEquals(4, new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.ALWAYS_FALSE).code());
+ assertEquals(0, prepare(new CacheEntryPredicateAdapter()));
+ assertEquals(0, prepare(new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.OTHER)));
+ assertEquals(1, prepare(new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.VALUE)));
+ assertEquals(1, prepare(new
CacheEntryPredicateAdapter((CacheObject)null)));
+ assertEquals(2, prepare(new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.HAS_VALUE)));
+ assertEquals(3, prepare(new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.HAS_NO_VALUE)));
+ assertEquals(4, prepare(new
CacheEntryPredicateAdapter(CacheEntryPredicateAdapter.PredicateType.ALWAYS_FALSE)));
Review Comment:
assertCacheEntryPredicateAdapter(4, ALWAYS_FALSE)
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/SecurityAwareCustomMessageWrapper.java:
##########
@@ -95,37 +93,20 @@ public DiscoveryCustomMessage delegate() {
return ack == null ? null : new SecurityAwareCustomMessageWrapper(ack,
secSubjId);
}
- /** */
- public byte[] messageBytes() {
- if (delegate instanceof Message)
- return null;
-
- if (msgBytes != null)
- return msgBytes;
-
- try {
- return msgBytes = U.marshal(jdk(), delegate);
- }
- catch (IgniteCheckedException e) {
- throw new IgniteException(e);
- }
+ /** {@inheritDoc} */
+ @Override public short directType() {
+ return 501;
}
- /** */
- public void messageBytes(byte[] msgBytes) {
- if (F.isEmpty(msgBytes))
- return;
-
- try {
- delegate = U.unmarshal(jdk(), msgBytes, U.gridClassLoader());
- }
- catch (IgniteCheckedException e) {
- throw new RuntimeException(e);
- }
+ /** {@inheritDoc} */
+ @Override public void prepareMarshal(Marshaller marsh) throws
IgniteCheckedException {
+ if (!(delegate instanceof Message))
+ msgBytes = U.marshal(marsh, delegate);
}
/** {@inheritDoc} */
- @Override public short directType() {
- return 501;
+ @Override public void finishUnmarshal(Marshaller marsh, ClassLoader
clsLdr) throws IgniteCheckedException {
+ if (msgBytes != null)
+ delegate = U.unmarshal(marsh, msgBytes, U.gridClassLoader());
Review Comment:
not clsLdr?
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/FragmentMapping.java:
##########
@@ -177,4 +179,16 @@ public FragmentMapping explicitMapping(Set<Long> srcIds) {
@Override public MessageType type() {
return MessageType.FRAGMENT_MAPPING;
}
+
+ /** {@inheritDoc} */
+ @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx)
throws IgniteCheckedException {
+ for (ColocationGroup grp : colocationGrps)
Review Comment:
forEach?
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/IndexKeyTypeMessage.java:
##########
@@ -94,4 +96,14 @@ private static byte encode(@Nullable IndexKeyType keyType) {
@Override public short directType() {
return TYPE_CODE;
}
+
+ /** {@inheritDoc} */
+ @Override public void prepareMarshal(Marshaller marsh) throws
IgniteCheckedException {
+ code = encode(val);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void finishUnmarshal(Marshaller marsh, ClassLoader
clsLdr) throws IgniteCheckedException {
+ val = decode(code);
Review Comment:
After empty constructor this code will return `IndexKeyType.forCode(0)`
instead of null from original implementation
##########
modules/core/src/test/java/org/apache/ignite/internal/managers/communication/IndexKeyTypeMessageTest.java:
##########
@@ -33,156 +36,232 @@ public class IndexKeyTypeMessageTest {
/** */
@Test
public void testIndexKeyTypeCode() {
- assertEquals(Byte.MIN_VALUE, new IndexKeyTypeMessage(null).code());
- assertEquals(-1, new IndexKeyTypeMessage(IndexKeyType.UNKNOWN).code());
- assertEquals(0, new IndexKeyTypeMessage(IndexKeyType.NULL).code());
- assertEquals(1, new IndexKeyTypeMessage(IndexKeyType.BOOLEAN).code());
- assertEquals(2, new IndexKeyTypeMessage(IndexKeyType.BYTE).code());
- assertEquals(3, new IndexKeyTypeMessage(IndexKeyType.SHORT).code());
- assertEquals(4, new IndexKeyTypeMessage(IndexKeyType.INT).code());
- assertEquals(5, new IndexKeyTypeMessage(IndexKeyType.LONG).code());
- assertEquals(6, new IndexKeyTypeMessage(IndexKeyType.DECIMAL).code());
- assertEquals(7, new IndexKeyTypeMessage(IndexKeyType.DOUBLE).code());
- assertEquals(8, new IndexKeyTypeMessage(IndexKeyType.FLOAT).code());
- assertEquals(9, new IndexKeyTypeMessage(IndexKeyType.TIME).code());
- assertEquals(10, new IndexKeyTypeMessage(IndexKeyType.DATE).code());
- assertEquals(11, new
IndexKeyTypeMessage(IndexKeyType.TIMESTAMP).code());
- assertEquals(12, new IndexKeyTypeMessage(IndexKeyType.BYTES).code());
- assertEquals(13, new IndexKeyTypeMessage(IndexKeyType.STRING).code());
- assertEquals(14, new
IndexKeyTypeMessage(IndexKeyType.STRING_IGNORECASE).code());
- assertEquals(15, new IndexKeyTypeMessage(IndexKeyType.BLOB).code());
- assertEquals(16, new IndexKeyTypeMessage(IndexKeyType.CLOB).code());
- assertEquals(17, new IndexKeyTypeMessage(IndexKeyType.ARRAY).code());
- assertEquals(18, new
IndexKeyTypeMessage(IndexKeyType.RESULT_SET).code());
- assertEquals(19, new
IndexKeyTypeMessage(IndexKeyType.JAVA_OBJECT).code());
- assertEquals(20, new IndexKeyTypeMessage(IndexKeyType.UUID).code());
- assertEquals(21, new
IndexKeyTypeMessage(IndexKeyType.STRING_FIXED).code());
- assertEquals(22, new
IndexKeyTypeMessage(IndexKeyType.GEOMETRY).code());
- assertEquals(24, new
IndexKeyTypeMessage(IndexKeyType.TIMESTAMP_TZ).code());
- assertEquals(25, new IndexKeyTypeMessage(IndexKeyType.ENUM).code());
+ assertEquals(Byte.MIN_VALUE, prepare(new IndexKeyTypeMessage(null)));
+ assertEquals(-1, prepare(new
IndexKeyTypeMessage(IndexKeyType.UNKNOWN)));
Review Comment:
assertIndexKeyTypeMessage(-1, UNKNOWN)
##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageSerializerGenerator.java:
##########
@@ -933,19 +911,13 @@ private void returnFalseIfEnumReadFailed(VariableElement
field, String mapperDec
else
readOp = line("%s(%s, reader.readByte())", mapperDecodeCallStmnt,
enumValuesFieldName);
- String methodName = field.getAnnotation(Order.class).method();
-
- if (Objects.equals(methodName, "")) {
- if (type.equals(field.getEnclosingElement()))
- read.add(identedLine("msg.%s = %s;",
field.getSimpleName().toString(), readOp));
- else {
- // Field has to be requested from a super class object.
- read.add(identedLine("((%s)msg).%s = %s;",
- field.getEnclosingElement().getSimpleName(),
field.getSimpleName().toString(), readOp));
- }
+ if (type.equals(field.getEnclosingElement()))
+ read.add(identedLine("msg.%s = %s;",
field.getSimpleName().toString(), readOp));
+ else {
+ // Field has to be requested from a super class object.
+ read.add(identedLine("((%s)msg).%s = %s;",
+ field.getEnclosingElement().getSimpleName(),
field.getSimpleName().toString(), readOp));
Review Comment:
⬆️
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/FragmentMapping.java:
##########
@@ -177,4 +179,16 @@ public FragmentMapping explicitMapping(Set<Long> srcIds) {
@Override public MessageType type() {
return MessageType.FRAGMENT_MAPPING;
}
+
+ /** {@inheritDoc} */
+ @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx)
throws IgniteCheckedException {
+ for (ColocationGroup grp : colocationGrps)
+ grp.prepareMarshal(ctx);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void prepareUnmarshal(GridCacheSharedContext<?, ?> ctx)
throws IgniteCheckedException {
+ for (ColocationGroup grp : colocationGrps)
+ grp.prepareUnmarshal(ctx);
Review Comment:
forEach?
##########
modules/codegen/src/main/java/org/apache/ignite/internal/MessageSerializerGenerator.java:
##########
@@ -933,19 +911,13 @@ private void returnFalseIfEnumReadFailed(VariableElement
field, String mapperDec
else
readOp = line("%s(%s, reader.readByte())", mapperDecodeCallStmnt,
enumValuesFieldName);
- String methodName = field.getAnnotation(Order.class).method();
-
- if (Objects.equals(methodName, "")) {
- if (type.equals(field.getEnclosingElement()))
- read.add(identedLine("msg.%s = %s;",
field.getSimpleName().toString(), readOp));
- else {
- // Field has to be requested from a super class object.
- read.add(identedLine("((%s)msg).%s = %s;",
- field.getEnclosingElement().getSimpleName(),
field.getSimpleName().toString(), readOp));
- }
+ if (type.equals(field.getEnclosingElement()))
+ read.add(identedLine("msg.%s = %s;",
field.getSimpleName().toString(), readOp));
Review Comment:
Do we need toString for format %s?
--
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]
