This is an automated email from the ASF dual-hosted git repository.
apkhmv pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new b1c2969dd0 IGNITE-23093 Support nested objects automatic conversion in
Compute API (#4556)
b1c2969dd0 is described below
commit b1c2969dd0d309b7e8c2b06b49ec016521330c71
Author: Maksim Myskov <maxim.mys...@gmail.com>
AuthorDate: Wed Oct 16 16:13:15 2024 +0300
IGNITE-23093 Support nested objects automatic conversion in Compute API
(#4556)
---
.../internal/client/proto/pojo/PojoConverter.java | 46 ++++++++++++++++++----
.../proto/ClientComputeJobPackerUnpackerTest.java | 25 ++++--------
.../internal/client/proto/pojo/ChildPojo.java | 23 -----------
.../internal/client/proto/pojo/ParentPojo.java | 23 -----------
.../ignite/internal/client/proto/pojo/Pojo.java | 35 +++++++++++++++-
.../client/proto/pojo/PojoConverterTest.java | 29 +++++---------
.../apache/ignite/internal/runner/app/Jobs.java | 21 +++++++++-
.../ItThinClientPojoComputeMarshallingTest.java | 17 ++++++++
8 files changed, 126 insertions(+), 93 deletions(-)
diff --git
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/pojo/PojoConverter.java
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/pojo/PojoConverter.java
index 121e612805..5589edc432 100644
---
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/pojo/PojoConverter.java
+++
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/pojo/PojoConverter.java
@@ -20,13 +20,21 @@ package org.apache.ignite.internal.client.proto.pojo;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.ignite.sql.ColumnType;
import org.apache.ignite.table.Tuple;
import org.apache.ignite.table.mapper.Mapper;
import org.apache.ignite.table.mapper.PojoMapper;
/** Converts POJO to Tuple and back. */
public class PojoConverter {
+ private static final Set<Class<?>> NATIVE_TYPES =
Arrays.stream(ColumnType.values())
+ .map(ColumnType::javaClass)
+ .collect(Collectors.toUnmodifiableSet());
/**
* Converts POJO to Tuple. Supports public and private non-static fields.
@@ -38,11 +46,6 @@ public class PojoConverter {
public static Tuple toTuple(Object obj) throws PojoConversionException {
Class<?> clazz = obj.getClass();
- // TODO https://issues.apache.org/jira/browse/IGNITE-23092
- if (clazz.getSuperclass() != Object.class) {
- throw new PojoConversionException("Can't convert subclasses");
- }
-
PojoMapper<?> mapper;
try {
mapper = (PojoMapper<?>) Mapper.of(clazz);
@@ -61,7 +64,7 @@ public class PojoConverter {
// TODO https://issues.apache.org/jira/browse/IGNITE-23261
MethodHandles.Lookup lookup =
MethodHandles.privateLookupIn(clazz, MethodHandles.lookup());
VarHandle varHandle = lookup.unreflectVarHandle(field);
- tuple.set(columnName, varHandle.get(obj));
+ tuple.set(columnName, convertToTupleIfNeeded(field,
varHandle.get(obj)));
} catch (IllegalAccessException e) {
throw new PojoConversionException("Cannot access field `" +
fieldName + "`", e);
} catch (NoSuchFieldException e) {
@@ -92,7 +95,7 @@ public class PojoConverter {
// TODO https://issues.apache.org/jira/browse/IGNITE-23261
MethodHandles.Lookup lookup =
MethodHandles.privateLookupIn(clazz, MethodHandles.lookup());
VarHandle varHandle = lookup.unreflectVarHandle(field);
- varHandle.set(obj, value);
+ varHandle.set(obj, convertFromNestedTupleIfNeeded(field,
value));
} catch (UnsupportedOperationException e) {
throw new PojoConversionException("Field for the column `" +
fieldName + "` is final", e);
} catch (ClassCastException e) {
@@ -104,6 +107,32 @@ public class PojoConverter {
}
}
+ private static Object convertToTupleIfNeeded(Field field, Object obj) {
+ if (isNativeType(field.getType())) {
+ return obj;
+ }
+ if (obj == null) {
+ return null;
+ }
+ return toTuple(obj);
+ }
+
+ private static Object convertFromNestedTupleIfNeeded(Field field, Object
value) throws IllegalAccessException {
+ if (isNativeType(field.getType())) {
+ return value;
+ }
+ if (value == null) {
+ return null;
+ }
+ try {
+ Object obj =
field.getType().getDeclaredConstructor().newInstance();
+ fromTuple(obj, (Tuple) value);
+ return obj;
+ } catch (NoSuchMethodException | InvocationTargetException |
InstantiationException e) {
+ throw new PojoConversionException("Field for the column `" +
field.getName() + "` cannot be converted", e);
+ }
+ }
+
private static Field getField(Class<?> clazz, String fieldName) {
try {
return clazz.getDeclaredField(fieldName);
@@ -112,4 +141,7 @@ public class PojoConverter {
}
}
+ private static boolean isNativeType(Class<?> clazz) {
+ return NATIVE_TYPES.contains(clazz) || clazz.isPrimitive();
+ }
}
diff --git
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/ClientComputeJobPackerUnpackerTest.java
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/ClientComputeJobPackerUnpackerTest.java
index 3e446d33fc..428aece0be 100644
---
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/ClientComputeJobPackerUnpackerTest.java
+++
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/ClientComputeJobPackerUnpackerTest.java
@@ -41,7 +41,6 @@ import
org.apache.ignite.internal.client.proto.pojo.StaticFieldPojo;
import org.apache.ignite.marshalling.Marshaller;
import org.apache.ignite.marshalling.MarshallingException;
import org.apache.ignite.marshalling.UnmarshallingException;
-import org.apache.ignite.marshalling.UnsupportedObjectTypeMarshallingException;
import org.apache.ignite.table.Tuple;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -76,11 +75,7 @@ class ClientComputeJobPackerUnpackerTest {
}
private static List<Object> pojo() {
- return List.of(new Pojo(
- true, (byte) 4, (short) 8, 15, 16L, 23.0f, 42.0d,
"TEST_STRING", UUID.randomUUID(), new byte[]{1, 2, 3},
- LocalTime.now(), LocalDate.now(), LocalDateTime.now(),
Instant.now(),
- Period.of(1, 2, 3), Duration.of(1, ChronoUnit.DAYS)
- ));
+ return List.of(Pojo.generateTestPojo());
}
private ClientMessagePacker messagePacker;
@@ -192,26 +187,20 @@ class ClientComputeJobPackerUnpackerTest {
}
}
- /** Pojo with unsupported type. */
- public static class UnsupportedType {
- // Unsupported type
- Object obj = new Object();
- }
-
@Test
- void packUnsupportedType() {
+ void packInvalidPojoStatic() {
assertThrows(
- UnsupportedObjectTypeMarshallingException.class,
- () -> packJobResult(new UnsupportedType(), null,
messagePacker),
- "Tuple field is of unsupported type: class java.lang.Object"
+ MarshallingException.class,
+ () -> packJobResult(new StaticFieldPojo(), null,
messagePacker),
+ "Can't pack object"
);
}
@Test
- void packInvalidPojo() {
+ void packInvalidPojoEmpty() {
assertThrows(
MarshallingException.class,
- () -> packJobResult(new StaticFieldPojo(), null,
messagePacker),
+ () -> packJobResult(new Object(), null, messagePacker),
"Can't pack object"
);
}
diff --git
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ChildPojo.java
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ChildPojo.java
deleted file mode 100644
index 484136f5c3..0000000000
---
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ChildPojo.java
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.ignite.internal.client.proto.pojo;
-
-/** Child POJO. */
-public class ChildPojo extends ParentPojo {
- public int childField;
-}
diff --git
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ParentPojo.java
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ParentPojo.java
deleted file mode 100644
index b0b977e2e1..0000000000
---
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/ParentPojo.java
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.ignite.internal.client.proto.pojo;
-
-/** Parent POJO. */
-public class ParentPojo {
- public int parentField;
-}
diff --git
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/Pojo.java
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/Pojo.java
index 4e0bd4a5ae..6cb93afb65 100644
---
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/Pojo.java
+++
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/Pojo.java
@@ -23,6 +23,7 @@ import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.Period;
+import java.time.temporal.ChronoUnit;
import java.util.Arrays;
import java.util.Objects;
import java.util.UUID;
@@ -47,12 +48,14 @@ public class Pojo {
private Instant instant;
private Period period;
private Duration duration;
+ private Pojo childPojo;
public Pojo() {
}
/** Constructor. */
public Pojo(
+ boolean nested,
boolean bool, byte b, short shortField, int intField, long
longField, float floatField, double doubleField, String string,
UUID uuid, byte[] byteArray, LocalTime localTime, LocalDate
localDate, LocalDateTime localDateTime,
Instant instant, Period period, Duration duration
@@ -73,6 +76,10 @@ public class Pojo {
this.instant = instant;
this.period = period;
this.duration = duration;
+ if (nested) {
+ this.childPojo = new Pojo(false, bool, b, shortField, intField,
longField, floatField, doubleField, string, uuid,
+ byteArray, localTime, localDate, localDateTime, instant,
period, duration);
+ }
}
@Override
@@ -92,7 +99,8 @@ public class Pojo {
&& Objects.equals(uuid, pojo.uuid) && Arrays.equals(byteArray,
pojo.byteArray)
&& Objects.equals(localTime, pojo.localTime) &&
Objects.equals(localDate, pojo.localDate)
&& Objects.equals(localDateTime, pojo.localDateTime) &&
Objects.equals(instant, pojo.instant)
- && Objects.equals(period, pojo.period) &&
Objects.equals(duration, pojo.duration);
+ && Objects.equals(period, pojo.period) &&
Objects.equals(duration, pojo.duration)
+ && Objects.equals(childPojo, pojo.childPojo);
}
@Override
@@ -113,6 +121,7 @@ public class Pojo {
result = 31 * result + Objects.hashCode(instant);
result = 31 * result + Objects.hashCode(period);
result = 31 * result + Objects.hashCode(duration);
+ result = 31 * result + Objects.hashCode(childPojo);
return result;
}
@@ -121,6 +130,28 @@ public class Pojo {
return "Pojo{" + "bool=" + bool + ", b=" + byteField + ", s=" +
shortField + ", i=" + intField + ", l=" + longField
+ ", f=" + floatField + ", d=" + doubleField + ", str='" +
string + '\'' + ", uuid=" + uuid
+ ", byteArray=" + Arrays.toString(byteArray) + ", localTime="
+ localTime + ", localDate=" + localDate
- + ", localDateTime=" + localDateTime + ", instant=" + instant
+ ", period=" + period + ", duration=" + duration + '}';
+ + ", localDateTime=" + localDateTime + ", instant=" + instant
+ ", period=" + period + ", duration=" + duration
+ + ", childPojo=" + childPojo
+ + '}';
+ }
+
+ /**
+ * Creates instance filled with test data.
+ */
+ public static Pojo generateTestPojo() {
+ return generateTestPojo(false);
+ }
+
+ /**
+ * Creates instance filled with test data.
+ *
+ * @param nested {@code true} if childPojo field needs to be filled
+ */
+ public static Pojo generateTestPojo(boolean nested) {
+ return new Pojo(nested,
+ true, (byte) 4, (short) 8, 15, 16L, 23.0f, 42.0d,
"TEST_STRING", UUID.randomUUID(), new byte[]{1, 2, 3},
+ LocalTime.now(), LocalDate.now(), LocalDateTime.now(),
Instant.now(),
+ Period.of(1, 2, 3), Duration.of(1, ChronoUnit.DAYS)
+ );
}
}
diff --git
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/PojoConverterTest.java
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/PojoConverterTest.java
index 3b256e3d5e..d555d98052 100644
---
a/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/PojoConverterTest.java
+++
b/modules/client-common/src/test/java/org/apache/ignite/internal/client/proto/pojo/PojoConverterTest.java
@@ -23,14 +23,6 @@ import static
org.apache.ignite.internal.testframework.IgniteTestUtils.assertThr
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
-import java.time.Duration;
-import java.time.Instant;
-import java.time.LocalDate;
-import java.time.LocalDateTime;
-import java.time.LocalTime;
-import java.time.Period;
-import java.time.temporal.ChronoUnit;
-import java.util.UUID;
import org.apache.ignite.table.Tuple;
import org.junit.jupiter.api.Test;
@@ -38,24 +30,23 @@ import org.junit.jupiter.api.Test;
class PojoConverterTest {
@Test
void allTypes() {
- Pojo src = new Pojo(
- true, (byte) 4, (short) 8, 15, 16L, 23.0f, 42.0d,
"TEST_STRING", UUID.randomUUID(), new byte[]{1, 2, 3},
- LocalTime.now(), LocalDate.now(), LocalDateTime.now(),
Instant.now(),
- Period.of(1, 2, 3), Duration.of(1, ChronoUnit.DAYS)
- );
+ Pojo src = Pojo.generateTestPojo(true);
Tuple tuple = toTuple(src);
Pojo dst = new Pojo();
fromTuple(dst, tuple);
assertThat(dst, is(src));
}
+ /*
+ * Nested Pojo is null in this case.
+ */
@Test
- void nestedPojo() {
- assertThrows(
- PojoConversionException.class,
- () -> toTuple(new ChildPojo()),
- "Can't convert subclasses"
- );
+ void allTypesWithoutNested() {
+ Pojo src = Pojo.generateTestPojo(false);
+ Tuple tuple = toTuple(src);
+ Pojo dst = new Pojo();
+ fromTuple(dst, tuple);
+ assertThat(dst, is(src));
}
@Test
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/Jobs.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/Jobs.java
index 2ad5631055..6fa5803b77 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/Jobs.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/Jobs.java
@@ -192,8 +192,17 @@ public class Jobs {
public static class PojoJob implements ComputeJob<PojoArg, PojoResult> {
@Override
public CompletableFuture<PojoResult> executeAsync(JobExecutionContext
context, @Nullable PojoArg arg) {
+ return completedFuture(new PojoResult().setLongValue(getSum(arg)));
+ }
+
+ private static long getSum(@Nullable PojoArg arg) {
var numberFromStr = Integer.parseInt(arg.strValue);
- return completedFuture(new PojoResult().setLongValue(arg.intValue
+ numberFromStr));
+ var sum = arg.intValue + numberFromStr;
+ if (arg.childPojo != null) {
+ return sum + getSum(arg.getChildPojo());
+ } else {
+ return sum;
+ }
}
}
@@ -201,6 +210,7 @@ public class Jobs {
public static class PojoArg {
String strValue;
int intValue;
+ PojoArg childPojo;
public PojoArg() {
}
@@ -223,6 +233,15 @@ public class Jobs {
return this;
}
+ public PojoArg getChildPojo() {
+ return childPojo;
+ }
+
+ public PojoArg setChildPojo(PojoArg value) {
+ this.childPojo = value;
+ return this;
+ }
+
@Override
public boolean equals(Object obj) {
if (this == obj) {
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientPojoComputeMarshallingTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientPojoComputeMarshallingTest.java
index 7f4b6ca72c..902d0b0d6c 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientPojoComputeMarshallingTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientPojoComputeMarshallingTest.java
@@ -59,6 +59,23 @@ public class ItThinClientPojoComputeMarshallingTest extends
ItAbstractThinClient
assertThat(result.getLongValue(), is(3L));
}
+ @ParameterizedTest
+ @ValueSource(ints = {0, 1})
+ void childPojoJob(int targetNodeIdx) {
+ // Given target node.
+ var targetNode = node(targetNodeIdx);
+
+ // When run job with provided pojo result class.
+ PojoResult result = client().compute().execute(
+ JobTarget.node(targetNode),
+
JobDescriptor.builder(PojoJob.class).resultClass(PojoResult.class).build(),
+ new PojoArg().setStrValue("1").setChildPojo(new
PojoArg().setStrValue("1"))
+ );
+
+ // Then the job returns the expected result.
+ assertThat(result.getLongValue(), is(2L));
+ }
+
@ParameterizedTest
@ValueSource(ints = {0, 1})
void pojoJobWithDifferentClass(int targetNodeIdx) {
