You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/10/13 18:52:53 UTC
[GitHub] [ignite] nizhikov opened a new pull request #9490: IGNITE-14742 BinaryArrayWrapper introduced
nizhikov opened a new pull request #9490:
URL: https://github.com/apache/ignite/pull/9490
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754325089
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2004,7 +2006,10 @@ public static Object doReadOptimized(BinaryInputStream in, BinaryContext ctx, @N
return doReadTimeArray(in);
case GridBinaryMarshaller.OBJ_ARR:
- return doReadObjectArray(in, ctx, ldr, handles, detach, deserialize);
+ if (BinaryArray.useTypedArrays())
Review comment:
Makes sense. Fixed, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754345146
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1364,7 +1364,10 @@ float readFloat(int fieldId) throws BinaryObjectException {
@Nullable @Override public Object[] readObjectArray() throws BinaryObjectException {
switch (checkFlag(OBJ_ARR)) {
case NORMAL:
- return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useTypedArrays())
+ return BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
Review comment:
Fixed. Thanks
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1913,7 +1916,10 @@ private String fieldFlagName(byte flag) {
break;
case OBJ_ARR:
- obj = BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useTypedArrays())
+ obj = BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
Review comment:
Fixed. Thanks
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759255688
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1364,10 +1364,16 @@ float readFloat(int fieldId) throws BinaryObjectException {
@Nullable @Override public Object[] readObjectArray() throws BinaryObjectException {
switch (checkFlag(OBJ_ARR)) {
case NORMAL:
- return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useBinaryArrays())
+ return BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
Review comment:
Not fixed (there was comment about it)
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheObjectUtils.java
##########
@@ -201,8 +203,10 @@ public static Object unwrapBinary(
return unwrapKnownCollection(ctx, (Collection<Object>)o, keepBinary, cpy);
else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded(ctx, (Map<Object, Object>)o, keepBinary, cpy);
- else if (o instanceof Object[])
+ else if (o instanceof Object[] && !BinaryArray.useBinaryArrays())
Review comment:
Do we really need `!BinaryArray.useBinaryArrays()` condition here?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2060,10 +2065,50 @@ public static Object doReadOptimized(BinaryInputStream in, BinaryContext ctx, @N
handles.setHandle(arr, hPos);
+ for (int i = 0; i < len; i++) {
+ Object res = deserializeOrUnmarshal(in, ctx, ldr, handles, detach, deserialize);
+
+ if (deserialize && BinaryArray.useBinaryArrays() && res instanceof BinaryObject)
Review comment:
We already call `deserializeOrUnmarshal` with deserialize = true, why do we need to deserialize again?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r753151392
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
Review comment:
Fixed with `wasDeserialized` flag.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov merged pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #9490:
URL: https://github.com/apache/ignite/pull/9490
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757585015
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -317,7 +299,44 @@ private Object callServiceLocally(
return ((PlatformService)svc).invokeMethod(methodName(mtd), false, true, args, callAttrs);
}
else
- return mtd.invoke(svc, BinaryUtils.rawArrayInArgs(args, false));
+ return callServiceMethod(svc, mtd, args, callCtx);
+ }
+
+ /**
+ * @param svc Service to be called.
+ * @param mtd Method to call.
+ * @param args Method args.
+ * @param callCtx Service call context.
+ * @return Invocation result.
+ */
+ private static Object callServiceMethod(
+ Service svc,
+ Method mtd,
+ Object[] args,
+ @Nullable ServiceCallContext callCtx
+ ) throws InvocationTargetException, IllegalAccessException {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(callCtx);
+
+ try {
+ return mtd.invoke(svc, args);
+ }
+ finally {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(null);
+ }
+ }
+
+ /** */
+ private Object unmarshalResult(byte[] res) throws IgniteCheckedException {
+ Marshaller marsh = ctx.config().getMarshaller();
+
+ if (KEEP_BINARY.get() && BinaryArray.useBinaryArrays() && marsh instanceof BinaryMarshaller) {
Review comment:
fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757584129
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java
##########
@@ -710,7 +709,10 @@ private static boolean areMethodArgsCompatible(Class[] argTypes, Object[] args)
Object arg = args[i];
Class argType = wrap(argTypes[i]);
- if (arg != null && !argType.isAssignableFrom(arg.getClass()))
+ if (arg == null)
+ continue;
+
+ if (!argType.isAssignableFrom(arg.getClass()))
Review comment:
Reverted
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759135476
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
Review comment:
Fix with `deserialized` looks dangerous. What if user will try to use binary object after deserialization? For example:
```
Object[] val = new Object[] {new TestClass1()};
BinaryObject binVal = server.binary().toBinary(val);
binVal.deserialize();
srvCache.put(0, binVal);
```
In this case node will fail with the AssertionError.
How about storing an additional "deserialized array" in `BinaryArray`? And return it in `deserialize()` method if it already exists.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759371252
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheObjectUtils.java
##########
@@ -201,8 +203,10 @@ public static Object unwrapBinary(
return unwrapKnownCollection(ctx, (Collection<Object>)o, keepBinary, cpy);
else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded(ctx, (Map<Object, Object>)o, keepBinary, cpy);
- else if (o instanceof Object[])
+ else if (o instanceof Object[] && !BinaryArray.useBinaryArrays())
Review comment:
This method creates new object on every invocation:
1. It's not good from performance perspective.
2. There are tests that check same object(by reference) returned so without this condition tests fails.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754427508
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java
##########
@@ -857,10 +856,18 @@ public void testMetaData() throws Exception {
Collection<String> fields = meta.fieldNames();
- assertEquals(2, fields.size());
+ switch (fields.size()) {
Review comment:
Makes sense. Fixed, thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756335168
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/compute/PlatformCompute.java
##########
@@ -363,7 +364,7 @@ protected Object executeJavaTask(BinaryRawReaderEx reader, boolean async) throws
IgniteCompute compute0 = computeForTask(nodeIds);
- if (!keepBinary && arg instanceof BinaryObjectImpl)
+ if (!keepBinary && (arg instanceof BinaryObjectImpl || arg instanceof BinaryArray))
Review comment:
Actually, I don't know this :)
Anyway, looks like it's a matter for another investigation isn't it?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756233997
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2554,6 +2591,41 @@ public static void validateEnumValues(String typeName, @Nullable Map<String, Int
return mergedMap;
}
+ /**
+ * @param obj {@link BinaryArray} or {@code Object[]}.
+ * @return Objects array.
+ */
+ public static Object[] rawArrayFromBinary(Object obj) {
+ if (obj instanceof BinaryArray)
+ // We want raw data(no deserialization).
+ return ((BinaryArray)obj).array();
+ else
+ // This can happen even in BinaryArray.USE_TYPED_ARRAY = true.
+ // In case user pass special array type to arguments, String[], for example.
+ return (Object[])obj;
+ }
+
+ /** */
+ public static Object[] rawArrayInArgs(Object[] args, boolean keepBinary) {
+ if (args == null)
+ return args;
+
+ for (int i = 0; i < args.length; i++) {
+ if (args[i] instanceof BinaryArray) {
+ BinaryArray arr = (BinaryArray)args[i];
+
+ args[i] = keepBinary ? arr.array() : arr.deserialize();
Review comment:
Agree. Removed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755773728
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -923,6 +924,8 @@ else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded((Map<Object, Object>)o);
else if (o instanceof Object[])
return unwrapBinariesInArray((Object[])o);
+ else if (o instanceof BinaryArray)
+ return unwrapBinariesInArray(((BinaryArray)o).deserialize());
Review comment:
Removed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755808308
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -1063,12 +1066,17 @@ public static void initializeJavaObject(Object obj, String clsName, @Nullable Ma
throw new IgniteException("Java object/factory class field is not found [" +
"className=" + clsName + ", fieldName=" + fieldName + ']');
+ Object val = prop.getValue();
+
try {
- field.set(obj, prop.getValue());
+ field.set(
+ obj,
+ BinaryUtils.isObjectArray(field.getType()) ? BinaryUtils.rawArrayFromBinary(val) : val
Review comment:
Right now we have single test for this feature in .Net - [`ContinuousQueryJavaFilterTest#TestFilter`](https://github.com/apache/ignite/blob/master/modules/platforms/dotnet/Apache.Ignite.Core.Tests/Cache/Query/Continuous/ContinuousQueryJavaFilterTest.cs#L105) and it seems feature not documented.
The changes just make test pass.
Do you think something should be additionally changed?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754433533
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -198,10 +203,29 @@ else if (U.isToStringMethod(mtd))
else {
ctx.task().setThreadContext(TC_IO_POLICY, GridIoPolicy.SERVICE_POOL);
+ ServiceProxyCallable svcCallable;
Review comment:
> at least to thin client ServicesTest
Yes, will do.
.Net tests will be added as a result of [IGNITE-14299](https://issues.apache.org/jira/browse/IGNITE-14299).
Preliminary PR based on this PR available [here](https://github.com/nizhikov/ignite/pull/21).
You can estimate code coverage from it.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757584399
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -924,8 +923,10 @@ else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded((Map<Object, Object>)o);
else if (o instanceof Object[])
return unwrapBinariesInArray((Object[])o);
+/*
Review comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r750376368
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2066,6 +2071,38 @@ public static Object doReadOptimized(BinaryInputStream in, BinaryContext ctx, @N
return arr;
}
+ /**
+ * @param in Binary input stream.
+ * @param ctx Binary context.
+ * @param ldr Class loader.
+ * @param handles Holder for handles.
+ * @param detach Detach flag.
+ * @param deserialize Deep flag.
+ * @return Value.
+ * @throws BinaryObjectException In case of error.
+ */
+ public static BinaryArray doReadBinaryArray(BinaryInputStream in, BinaryContext ctx, ClassLoader ldr,
+ BinaryReaderHandlesHolder handles, boolean detach, boolean deserialize) {
+ int hPos = positionForHandle(in);
+
+ int compTypeId = in.readInt();
+ String compClsName = null;
+
+ if (compTypeId == GridBinaryMarshaller.UNREGISTERED_TYPE_ID)
+ compClsName = doReadClassName(in);
+
+ int len = in.readInt();
+
+ Object[] arr = new Object[len];
+
+ handles.setHandle(arr, hPos);
Review comment:
`BinaryArray` should be stored as a handle, otherwise during the next read of the same array component type will be lost.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinarySerializedFieldComparator.java
##########
@@ -288,7 +295,7 @@ else if (val1 instanceof double[])
* @return {@code True} if field is array.
*/
private static boolean isArray(@Nullable Object field) {
- return field != null && field.getClass().isArray();
+ return field != null && (field.getClass().isArray() || BinaryArray.class.isAssignableFrom(field.getClass()));
Review comment:
IMO `field instanceof BinaryArray` is more readable (and also a little bit faster), but it's up to you.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2004,7 +2006,10 @@ public static Object doReadOptimized(BinaryInputStream in, BinaryContext ctx, @N
return doReadTimeArray(in);
case GridBinaryMarshaller.OBJ_ARR:
- return doReadObjectArray(in, ctx, ldr, handles, detach, deserialize);
+ if (BinaryArray.useTypedArrays())
Review comment:
`if (BinaryArray.useTypedArrays() && !deserialize)` to avoid unnecessary wrapping when possible?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -923,6 +924,8 @@ else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded((Map<Object, Object>)o);
else if (o instanceof Object[])
return unwrapBinariesInArray((Object[])o);
+ else if (o instanceof BinaryArray)
+ return unwrapBinariesInArray(((BinaryArray)o).deserialize());
Review comment:
Do we really need this condition? BinaryArray.deserialize already unwrap binaries. And deserialize already called in the next line (since BinaryArray instanceof BinaryObject)
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1913,7 +1916,10 @@ private String fieldFlagName(byte flag) {
break;
case OBJ_ARR:
- obj = BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useTypedArrays())
+ obj = BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
Review comment:
Looks redundant (see previous comment).
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/compute/PlatformCompute.java
##########
@@ -363,7 +364,7 @@ protected Object executeJavaTask(BinaryRawReaderEx reader, boolean async) throws
IgniteCompute compute0 = computeForTask(nodeIds);
- if (!keepBinary && arg instanceof BinaryObjectImpl)
+ if (!keepBinary && (arg instanceof BinaryObjectImpl || arg instanceof BinaryArray))
Review comment:
Why only these two types? Binary enum is not possible here?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/service/ClientServiceInvokeRequest.java
##########
@@ -147,6 +148,9 @@ public ClientServiceInvokeRequest(BinaryReaderExImpl reader) {
IgniteServices services = grp.services();
+ if (BinaryArray.useTypedArrays())
+ GridServiceProxy.KEEP_BINARY.set(true);
Review comment:
Can we introduce some field in GridServiceProxy and pass it as a constructor parameter? I think there can be problems with thread local - if service invoke another service. For example, if a local java service was invoked by thin client and this service invoke another service hosted on another node - `KEEP_BINARY` flag will be `TRUE` and BinaryObject will be returned instead regular object to the first service.
Why KEEP BINARY is set only for useTypedArrays? I think it's useful for old implementation too.
Also, looks like all changes in services and GridTaskWorker are not related to problems solved by this ticket. All problems with the deserialization of service invocation results already should be resolved by introducing a binary array wrapper. Do I miss something?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
+
+ boolean keepBinary = BinaryObject.class.isAssignableFrom(compType);
+
+ for (int i = 0; i < arr.length; i++) {
+ Object obj = CacheObjectUtils.unwrapBinaryIfNeeded(null, arr[i], keepBinary, false, ldr);
+
+ if (!keepBinary && obj != null && BinaryObject.class.isAssignableFrom(obj.getClass()))
+ obj = ((BinaryObject)obj).deserialize(ldr);
+
+ res[i] = obj;
+ }
+
+ return (T)res;
+ }
+ finally {
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.TRUE);
+ }
+ }
+
+ /**
+ * @return Underlying array.
+ */
+ public Object[] array() {
+ return arr;
+ }
+
+ /**
+ * @return Component type ID.
+ */
+ public int componentTypeId() {
+ return compTypeId;
+ }
+
+ /**
+ * @return Component class name.
+ */
+ public String componentClassName() {
+ return compClsName;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryObject clone() throws CloneNotSupportedException {
+ return new BinaryArray(ctx, compTypeId, compClsName, arr.clone());
+ }
+
+ /** {@inheritDoc} */
+ @Override public int typeId() {
+ return GridBinaryMarshaller.OBJ_ARR;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void writeExternal(ObjectOutput out) throws IOException {
+ out.writeInt(compTypeId);
+ out.writeObject(compClsName);
+ out.writeObject(arr);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+ ctx = GridBinaryMarshaller.threadLocalContext();
+
+ compTypeId = in.readInt();
+ compClsName = (String)in.readObject();
+ arr = (Object[])in.readObject();
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryObjectBuilder toBuilder() throws BinaryObjectException {
+ throw new UnsupportedOperationException("Builder cannot be created for array wrapper.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public int enumOrdinal() throws BinaryObjectException {
+ throw new BinaryObjectException("Object is not enum.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public String enumName() throws BinaryObjectException {
+ throw new BinaryObjectException("Object is not enum.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public int size() {
+ return 0;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean isFlagSet(short flag) {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <F> F field(String fieldName) throws BinaryObjectException {
+ return null;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean hasField(String fieldName) {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public int hashCode() {
+ int result = 31 * Objects.hash(compTypeId);
+
+ result = 31 * result + IgniteUtils.hashCode(arr);
+
+ return result;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
+ if (this == o)
+ return true;
+
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ BinaryArray arr = (BinaryArray)o;
+
+ return compTypeId == arr.compTypeId
+ && Arrays.equals(this.arr, arr.arr);
+ }
+
+ /** {@inheritDoc} */
+ @Override public String toString() {
+ return S.toString(BinaryArray.class, this);
+ }
+
+ /** @return {@code True} if typed arrays should be used, {@code false} otherwise. */
+ public static boolean useTypedArrays() {
+ return USE_TYPED_ARRAYS;
+ }
+
+ /**
+ * Initialize {@link #USE_TYPED_ARRAYS} value with
+ * {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS} system property value.
+ *
+ * This method invoked using reflection in tests.
+ */
+ public static void initUseTypedArrays() {
+ USE_TYPED_ARRAYS = IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
Review comment:
I think the logic based on a static field that can be modified dynamically in runtime is fragile. Perhaps it could be moved to some non-static field of Ignite-instance-singleton object (for example `BinaryContext`, but I'm not sure it's available in all places where `useTypedArrays` is invoked). WDYT?
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryArraySelfTest.java
##########
@@ -0,0 +1,348 @@
+/*
+ * 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.binary;
+
+import java.util.Arrays;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.internal.binary.BinaryMarshallerSelfTest.TestClass1;
+import org.apache.ignite.internal.processors.platform.utils.PlatformUtils;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.junit.Test;
+
+/** */
+public class BinaryArraySelfTest extends AbstractTypedArrayTest {
+ /** */
+ private static Ignite server;
+
+ /** */
+ private static Ignite client;
+
+ /** */
+ private static IgniteCache<Object, Object> srvCache;
+
+ /** */
+ private static IgniteCache<Object, Object> cliCache;
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTestsStarted() throws Exception {
+ super.beforeTestsStarted();
+
+ server = startGrid(0);
+ client = startClientGrid(1);
+
+ srvCache = server.createCache("my-cache");
+ cliCache = client.getOrCreateCache("my-cache");
+ }
+
+ /** */
+ @Test
+ public void testArrayKey() {
+ doTestArrayKey(srvCache);
+ doTestArrayKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayValue() {
+ doTestArrayValue(srvCache);
+ doTestArrayValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInKey() {
+ doTestArrayFieldInKey(srvCache);
+ doTestArrayFieldInKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInValue() {
+ doTestArrayFieldInValue(srvCache);
+ doTestArrayFieldInValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBoxedPrimitivesArrays() {
+ doTestBoxedPrimitivesArrays(srvCache);
+ doTestBoxedPrimitivesArrays(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBinaryModeArray() {
+ doTestBinaryModeArray(srvCache);
+ doTestBinaryModeArray(cliCache);
+ }
+
+ /** */
+ private void doTestArrayKey(IgniteCache<Object, Object> c) {
+ TestClass1[] key1 = {new TestClass1(), new TestClass1()};
+ TestClass1[] key2 = {};
+ TestClass1[] key3 = new TestClass1[] {new TestClass1() {}};
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayFieldInKey(IgniteCache<Object, Object> c) {
+ TestClass key1 = new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()});
+ TestClass key2 = new TestClass(new TestClass1[] {});
+ TestClass key3 = new TestClass(new TestClass1[] {new TestClass1() {}});
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ c.put(2, new TestClass1[] {});
+ c.put(3, new TestClass1[] {new TestClass1() {}});
+
+ Object val1 = c.get(1);
+ Object val2 = c.get(2);
+ Object val3 = c.get(3);
+
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val1.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val2.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val3.getClass());
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestArrayFieldInValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()}));
+ c.put(2, new TestClass(new TestClass1[] {}));
+ c.put(3, new TestClass(new TestClass1[] {new TestClass1() {}}));
+
+ TestClass val1 = (TestClass)c.get(1);
+ TestClass val2 = (TestClass)c.get(2);
+ TestClass val3 = (TestClass)c.get(3);
+
+ assertEquals(2, val1.getArr().length);
+ assertEquals(0, val2.getArr().length);
+ assertEquals(1, val3.getArr().length);
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestBinaryModeArray(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ Object obj = c.withKeepBinary().get(1);
+
+ assertEquals(useTypedArrays ? BinaryArray.class : Object[].class, obj.getClass());
+
+ if (useTypedArrays)
+ assertEquals(TestClass1[].class, ((BinaryObject)obj).deserialize().getClass());
+
+ assertTrue(c.remove(1));
+ }
+
+ /** */
+ private void doTestBoxedPrimitivesArrays(IgniteCache<Object, Object> c) {
+ Object[] data = new Object[] {
+ new Byte[] {1, 2, 3},
+ new Short[] {1, 2, 3},
+ new Integer[] {1, 2, 3},
+ new Long[] {1L, 2L, 3L},
+ new Float[] {1f, 2f, 3f},
+ new Double[] {1d, 2d, 3d},
+ new Character[] {'a', 'b', 'c'},
+ new Boolean[] {true, false},
+ };
+
+ for (Object item : data) {
+ c.put(1, item);
+
+ Object item0 = c.get(1);
+
+ if (useTypedArrays)
+ assertTrue(c.replace(1, item, item));
+
+ assertTrue(c.remove(1));
+
+ if (useTypedArrays)
+ assertEquals(item.getClass(), item0.getClass());
+
+ assertTrue(Arrays.equals((Object[])item, (Object[])item0));
+
+ c.put(item, 1);
+
+ assertEquals(1, c.get(item));
+
+ if (useTypedArrays)
+ assertTrue(c.replace(item, 1, 2));
+
+ assertTrue(c.remove(item));
+ }
+ }
+
+ /** */
+ @Test
+ public void testArrayOfBinariesSerDe() {
+ BinaryObject[] arr = new BinaryObject[] {
+ server.binary().toBinary(new TestClass1()),
+ server.binary().toBinary(new TestClass1())
+ };
+
+ Object obj = server.binary().toBinary(arr);
+
+ Object deser;
+
+ if (useTypedArrays) {
+ assertTrue(obj instanceof BinaryArray);
+
+ deser = ((BinaryArray)obj).deserialize();
+ }
+ else {
+ assertTrue(obj instanceof Object[]);
+
+ deser = PlatformUtils.unwrapBinariesInArray((Object[])obj);
+ }
+
+ assertEquals(Object[].class, deser.getClass());
+
+ Object[] res = ((Object[])deser);
+
+ assertEquals(2, res.length);
+ assertTrue(res[0] instanceof TestClass1);
+ assertTrue(res[1] instanceof TestClass1);
+
Review comment:
redundant NL
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryArraySelfTest.java
##########
@@ -0,0 +1,348 @@
+/*
+ * 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.binary;
+
+import java.util.Arrays;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.internal.binary.BinaryMarshallerSelfTest.TestClass1;
+import org.apache.ignite.internal.processors.platform.utils.PlatformUtils;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.junit.Test;
+
+/** */
+public class BinaryArraySelfTest extends AbstractTypedArrayTest {
+ /** */
+ private static Ignite server;
+
+ /** */
+ private static Ignite client;
+
+ /** */
+ private static IgniteCache<Object, Object> srvCache;
+
+ /** */
+ private static IgniteCache<Object, Object> cliCache;
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTestsStarted() throws Exception {
+ super.beforeTestsStarted();
+
+ server = startGrid(0);
+ client = startClientGrid(1);
+
+ srvCache = server.createCache("my-cache");
+ cliCache = client.getOrCreateCache("my-cache");
+ }
+
+ /** */
+ @Test
+ public void testArrayKey() {
+ doTestArrayKey(srvCache);
+ doTestArrayKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayValue() {
+ doTestArrayValue(srvCache);
+ doTestArrayValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInKey() {
+ doTestArrayFieldInKey(srvCache);
+ doTestArrayFieldInKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInValue() {
+ doTestArrayFieldInValue(srvCache);
+ doTestArrayFieldInValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBoxedPrimitivesArrays() {
+ doTestBoxedPrimitivesArrays(srvCache);
+ doTestBoxedPrimitivesArrays(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBinaryModeArray() {
+ doTestBinaryModeArray(srvCache);
+ doTestBinaryModeArray(cliCache);
+ }
+
+ /** */
+ private void doTestArrayKey(IgniteCache<Object, Object> c) {
+ TestClass1[] key1 = {new TestClass1(), new TestClass1()};
+ TestClass1[] key2 = {};
+ TestClass1[] key3 = new TestClass1[] {new TestClass1() {}};
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayFieldInKey(IgniteCache<Object, Object> c) {
+ TestClass key1 = new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()});
+ TestClass key2 = new TestClass(new TestClass1[] {});
+ TestClass key3 = new TestClass(new TestClass1[] {new TestClass1() {}});
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ c.put(2, new TestClass1[] {});
+ c.put(3, new TestClass1[] {new TestClass1() {}});
+
+ Object val1 = c.get(1);
+ Object val2 = c.get(2);
+ Object val3 = c.get(3);
+
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val1.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val2.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val3.getClass());
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestArrayFieldInValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()}));
+ c.put(2, new TestClass(new TestClass1[] {}));
+ c.put(3, new TestClass(new TestClass1[] {new TestClass1() {}}));
+
+ TestClass val1 = (TestClass)c.get(1);
+ TestClass val2 = (TestClass)c.get(2);
+ TestClass val3 = (TestClass)c.get(3);
+
+ assertEquals(2, val1.getArr().length);
+ assertEquals(0, val2.getArr().length);
+ assertEquals(1, val3.getArr().length);
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestBinaryModeArray(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ Object obj = c.withKeepBinary().get(1);
+
+ assertEquals(useTypedArrays ? BinaryArray.class : Object[].class, obj.getClass());
+
+ if (useTypedArrays)
+ assertEquals(TestClass1[].class, ((BinaryObject)obj).deserialize().getClass());
+
+ assertTrue(c.remove(1));
+ }
+
+ /** */
+ private void doTestBoxedPrimitivesArrays(IgniteCache<Object, Object> c) {
+ Object[] data = new Object[] {
+ new Byte[] {1, 2, 3},
+ new Short[] {1, 2, 3},
+ new Integer[] {1, 2, 3},
+ new Long[] {1L, 2L, 3L},
+ new Float[] {1f, 2f, 3f},
+ new Double[] {1d, 2d, 3d},
+ new Character[] {'a', 'b', 'c'},
+ new Boolean[] {true, false},
+ };
+
+ for (Object item : data) {
+ c.put(1, item);
+
+ Object item0 = c.get(1);
+
+ if (useTypedArrays)
+ assertTrue(c.replace(1, item, item));
+
+ assertTrue(c.remove(1));
+
+ if (useTypedArrays)
+ assertEquals(item.getClass(), item0.getClass());
+
+ assertTrue(Arrays.equals((Object[])item, (Object[])item0));
+
+ c.put(item, 1);
+
+ assertEquals(1, c.get(item));
+
+ if (useTypedArrays)
+ assertTrue(c.replace(item, 1, 2));
+
+ assertTrue(c.remove(item));
+ }
+ }
+
+ /** */
+ @Test
+ public void testArrayOfBinariesSerDe() {
+ BinaryObject[] arr = new BinaryObject[] {
+ server.binary().toBinary(new TestClass1()),
+ server.binary().toBinary(new TestClass1())
+ };
+
+ Object obj = server.binary().toBinary(arr);
+
+ Object deser;
+
+ if (useTypedArrays) {
+ assertTrue(obj instanceof BinaryArray);
+
+ deser = ((BinaryArray)obj).deserialize();
+ }
+ else {
+ assertTrue(obj instanceof Object[]);
+
+ deser = PlatformUtils.unwrapBinariesInArray((Object[])obj);
+ }
+
+ assertEquals(Object[].class, deser.getClass());
Review comment:
Why for `BinaryObject[]` class `Object[]` is returned? Any compatibility issues?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
+
+ boolean keepBinary = BinaryObject.class.isAssignableFrom(compType);
+
+ for (int i = 0; i < arr.length; i++) {
+ Object obj = CacheObjectUtils.unwrapBinaryIfNeeded(null, arr[i], keepBinary, false, ldr);
+
+ if (!keepBinary && obj != null && BinaryObject.class.isAssignableFrom(obj.getClass()))
Review comment:
`obj != null && BinaryObject.class.isAssignableFrom(obj.getClass())` can be simplified to `obj instanceof BinaryObject`
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2554,6 +2591,41 @@ public static void validateEnumValues(String typeName, @Nullable Map<String, Int
return mergedMap;
}
+ /**
+ * @param obj {@link BinaryArray} or {@code Object[]}.
+ * @return Objects array.
+ */
+ public static Object[] rawArrayFromBinary(Object obj) {
+ if (obj instanceof BinaryArray)
+ // We want raw data(no deserialization).
+ return ((BinaryArray)obj).array();
+ else
+ // This can happen even in BinaryArray.USE_TYPED_ARRAY = true.
+ // In case user pass special array type to arguments, String[], for example.
+ return (Object[])obj;
+ }
+
+ /** */
+ public static Object[] rawArrayInArgs(Object[] args, boolean keepBinary) {
+ if (args == null)
+ return args;
+
+ for (int i = 0; i < args.length; i++) {
+ if (args[i] instanceof BinaryArray) {
+ BinaryArray arr = (BinaryArray)args[i];
+
+ args[i] = keepBinary ? arr.array() : arr.deserialize();
+ }
+ }
+
+ return args;
+ }
+
+ /** */
+ public static boolean isObjectArray(Class<?> cls) {
+ return Object[].class == cls || BinaryArray.class == cls;
Review comment:
`BinaryArray.class.isAssignableFrom(cls)` or `cls.isInstance(BinaryArray.class)` to consistency with other usages?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -486,7 +536,7 @@ private Object callService(Service srv, Method mtd) throws Exception {
throw new GridServiceMethodNotFoundException(svcName, mtdName, argTypes);
try {
- return mtd.invoke(srv, args);
+ return mtd.invoke(srv, BinaryUtils.rawArrayInArgs(args, false));
Review comment:
We can marshall result to `bytes[]` here and unmarshall on reduce node with or without deserialization instead of introducing the new class and making GridTaskWorker coupled with this class. WDYT?
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/dml/DmlUtils.java
##########
@@ -111,6 +112,18 @@ public static Object convert(Object val, GridH2RowDescriptor desc, Class<?> expC
// We have to convert arrays of reference types manually -
// see https://issues.apache.org/jira/browse/IGNITE-4327
// Still, we only can convert from Object[] to something more precise.
+ if (type == Value.ARRAY && val instanceof BinaryArray) {
+ val = ((BinaryArray)val).deserialize();
Review comment:
Looks suspicious. If we have `BinaryArray` here, then `val` should be binary, but here we deserialize it and binary objects will coexist with regular objects in the same context.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1364,7 +1364,10 @@ float readFloat(int fieldId) throws BinaryObjectException {
@Nullable @Override public Object[] readObjectArray() throws BinaryObjectException {
switch (checkFlag(OBJ_ARR)) {
case NORMAL:
- return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useTypedArrays())
+ return BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
Review comment:
Looks redundant. When we read array with the `deserialize` flag we have component type information and can create a corresponding array, so addition wrapper/unwrapper to/from `BinaryArray` is not required.
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectToStringTest.java
##########
@@ -85,6 +84,8 @@ public void testToStringInaccessibleOptimizedMarshallerClass() throws Exception
assertStringFormMatches(new TestContainer(new TestExternalizable(newExtInstance1())),
failedStrPattern("ExternalTestClass1"));
+
+ stopAllGrids();
Review comment:
It should be in the `finally` block, or in the `afterTest()` method.
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryObjectBuilderDefaultMappersSelfTest.java
##########
@@ -857,10 +856,18 @@ public void testMetaData() throws Exception {
Collection<String> fields = meta.fieldNames();
- assertEquals(2, fields.size());
+ switch (fields.size()) {
Review comment:
It's not trivial to understand why there can be different count of fields, let's use different class names for different test runs instead (for example `"org.test.MetaTest" + (useTypedArray ? "1" : "0")`) and revert this changes to make test more clear.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -198,10 +203,29 @@ else if (U.isToStringMethod(mtd))
else {
ctx.task().setThreadContext(TC_IO_POLICY, GridIoPolicy.SERVICE_POOL);
+ ServiceProxyCallable svcCallable;
Review comment:
What about services tests with enabled "use binary array" property? I think it should be added at least to thin client `ServicesTest`. Perhaps for .NET services tests too.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -1063,12 +1066,17 @@ public static void initializeJavaObject(Object obj, String clsName, @Nullable Ma
throw new IgniteException("Java object/factory class field is not found [" +
"className=" + clsName + ", fieldName=" + fieldName + ']');
+ Object val = prop.getValue();
+
try {
- field.set(obj, prop.getValue());
+ field.set(
+ obj,
+ BinaryUtils.isObjectArray(field.getType()) ? BinaryUtils.rawArrayFromBinary(val) : val
Review comment:
Looks suspiciously. If we have `BinaryArray` here, then the field value is not deserialized. Initializing java object field with not deserialized field value may lead to some hidden problems. Perhaps, props should be read with deserialization (for example, with `readObject()` method instead of `readObjectDetached()`).
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2554,6 +2591,41 @@ public static void validateEnumValues(String typeName, @Nullable Map<String, Int
return mergedMap;
}
+ /**
+ * @param obj {@link BinaryArray} or {@code Object[]}.
+ * @return Objects array.
+ */
+ public static Object[] rawArrayFromBinary(Object obj) {
+ if (obj instanceof BinaryArray)
+ // We want raw data(no deserialization).
+ return ((BinaryArray)obj).array();
+ else
+ // This can happen even in BinaryArray.USE_TYPED_ARRAY = true.
+ // In case user pass special array type to arguments, String[], for example.
+ return (Object[])obj;
+ }
+
+ /** */
+ public static Object[] rawArrayInArgs(Object[] args, boolean keepBinary) {
+ if (args == null)
+ return args;
+
+ for (int i = 0; i < args.length; i++) {
+ if (args[i] instanceof BinaryArray) {
+ BinaryArray arr = (BinaryArray)args[i];
+
+ args[i] = keepBinary ? arr.array() : arr.deserialize();
Review comment:
Why only `BinaryArray`'s are deserialized? What about regular binary objects? With such a behavior binary objects and regular objects will coexist in the same args array, I think it's error-prone.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/GridBinaryMarshaller.java
##########
@@ -153,6 +154,12 @@
/** Binary enum */
public static final byte BINARY_ENUM = 38;
+ /**
+ * {@link BinaryObject} interface type id.
+ * Used to store array component type in case of {@code BinaryObject[]}.
+ */
+ public static final byte BINARY_OBJ_INTERFACE = 39;
Review comment:
As far as I understand this type was introduced only to support BinaryArray[] in .NET? I see no test was modified in .NET, do we already have a test for this functionality? Why this type is not supported in java? I mean, if we `put(0, new BinaryObject[] {})` to cache and then `get(0)` from cache - result will be `Object[]`
##########
File path: modules/clients/src/test/java/org/apache/ignite/internal/processors/rest/JettyRestProcessorAbstractSelfTest.java
##########
@@ -1869,6 +1885,8 @@ public void testMetadataRemote() throws Exception {
assertResponseContainsError(content("nonExistingCacheName", GridRestCommand.CACHE_METADATA),
"Failed to request meta data. nonExistingCacheName is not found");
+
+ grid(1).destroyCache("partial");
Review comment:
Should be in `finally` block.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
Review comment:
This makes `BinaryArray` mutable. For example:
```
Object[] val = new Object[] {new TestClass1()};
BinaryArray binVal = grid(0).binary().toBinary(val);
assertTrue(binVal.array()[0] instanceof BinaryObject);
binVal.deserialize();
assertTrue(binVal.array()[0] instanceof BinaryObject);
```
This code will fail on the second `assertTrue`. After deserialization, `BinaryArray` contains some not binary types. Is it expected?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754424335
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryArray.java
##########
@@ -0,0 +1,260 @@
+/*
+ * 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.binary;
+
+import java.io.Externalizable;
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.Objects;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.CacheObjectUtils;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.tostring.GridToStringInclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_USE_TYPED_ARRAYS;
+
+/**
+ * Binary object representing array.
+ */
+public class BinaryArray implements BinaryObjectEx, Externalizable {
+ /** Default value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ public static final boolean DFLT_IGNITE_USE_TYPED_ARRAYS = false;
+
+ /** Value of {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS}. */
+ private static boolean USE_TYPED_ARRAYS =
+ IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
+
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Context. */
+ @GridDirectTransient
+ @GridToStringExclude
+ private BinaryContext ctx;
+
+ /** Type ID. */
+ private int compTypeId;
+
+ /** Type class name. */
+ @Nullable private String compClsName;
+
+ /** Values. */
+ @GridToStringInclude(sensitive = true)
+ private Object[] arr;
+
+ /**
+ * {@link Externalizable} support.
+ */
+ public BinaryArray() {
+ // No-op.
+ }
+
+ /**
+ * @param ctx Context.
+ * @param compTypeId Component type id.
+ * @param compClsName Component class name.
+ * @param arr Array.
+ */
+ public BinaryArray(BinaryContext ctx, int compTypeId, @Nullable String compClsName, Object[] arr) {
+ this.ctx = ctx;
+ this.compTypeId = compTypeId;
+ this.compClsName = compClsName;
+ this.arr = arr;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryType type() throws BinaryObjectException {
+ return BinaryUtils.typeProxy(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public @Nullable BinaryType rawType() throws BinaryObjectException {
+ return BinaryUtils.type(ctx, this);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize() throws BinaryObjectException {
+ return (T)deserialize(null);
+ }
+
+ /** {@inheritDoc} */
+ @Override public <T> T deserialize(ClassLoader ldr) throws BinaryObjectException {
+ ClassLoader resolveLdr = ldr == null ? ctx.configuration().getClassLoader() : ldr;
+
+ if (ldr != null)
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.FALSE);
+
+ try {
+ Class<?> compType = BinaryUtils.resolveClass(ctx, compTypeId, compClsName, resolveLdr, false);
+
+ Object[] res = Object.class == compType ? arr : (Object[])Array.newInstance(compType, arr.length);
+
+ boolean keepBinary = BinaryObject.class.isAssignableFrom(compType);
+
+ for (int i = 0; i < arr.length; i++) {
+ Object obj = CacheObjectUtils.unwrapBinaryIfNeeded(null, arr[i], keepBinary, false, ldr);
+
+ if (!keepBinary && obj != null && BinaryObject.class.isAssignableFrom(obj.getClass()))
+ obj = ((BinaryObject)obj).deserialize(ldr);
+
+ res[i] = obj;
+ }
+
+ return (T)res;
+ }
+ finally {
+ GridBinaryMarshaller.USE_CACHE.set(Boolean.TRUE);
+ }
+ }
+
+ /**
+ * @return Underlying array.
+ */
+ public Object[] array() {
+ return arr;
+ }
+
+ /**
+ * @return Component type ID.
+ */
+ public int componentTypeId() {
+ return compTypeId;
+ }
+
+ /**
+ * @return Component class name.
+ */
+ public String componentClassName() {
+ return compClsName;
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryObject clone() throws CloneNotSupportedException {
+ return new BinaryArray(ctx, compTypeId, compClsName, arr.clone());
+ }
+
+ /** {@inheritDoc} */
+ @Override public int typeId() {
+ return GridBinaryMarshaller.OBJ_ARR;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void writeExternal(ObjectOutput out) throws IOException {
+ out.writeInt(compTypeId);
+ out.writeObject(compClsName);
+ out.writeObject(arr);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
+ ctx = GridBinaryMarshaller.threadLocalContext();
+
+ compTypeId = in.readInt();
+ compClsName = (String)in.readObject();
+ arr = (Object[])in.readObject();
+ }
+
+ /** {@inheritDoc} */
+ @Override public BinaryObjectBuilder toBuilder() throws BinaryObjectException {
+ throw new UnsupportedOperationException("Builder cannot be created for array wrapper.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public int enumOrdinal() throws BinaryObjectException {
+ throw new BinaryObjectException("Object is not enum.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public String enumName() throws BinaryObjectException {
+ throw new BinaryObjectException("Object is not enum.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public int size() {
+ return 0;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean isFlagSet(short flag) {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <F> F field(String fieldName) throws BinaryObjectException {
+ return null;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean hasField(String fieldName) {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public int hashCode() {
+ int result = 31 * Objects.hash(compTypeId);
+
+ result = 31 * result + IgniteUtils.hashCode(arr);
+
+ return result;
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean equals(Object o) {
+ if (this == o)
+ return true;
+
+ if (o == null || getClass() != o.getClass())
+ return false;
+
+ BinaryArray arr = (BinaryArray)o;
+
+ return compTypeId == arr.compTypeId
+ && Arrays.equals(this.arr, arr.arr);
+ }
+
+ /** {@inheritDoc} */
+ @Override public String toString() {
+ return S.toString(BinaryArray.class, this);
+ }
+
+ /** @return {@code True} if typed arrays should be used, {@code false} otherwise. */
+ public static boolean useTypedArrays() {
+ return USE_TYPED_ARRAYS;
+ }
+
+ /**
+ * Initialize {@link #USE_TYPED_ARRAYS} value with
+ * {@link IgniteSystemProperties#IGNITE_USE_TYPED_ARRAYS} system property value.
+ *
+ * This method invoked using reflection in tests.
+ */
+ public static void initUseTypedArrays() {
+ USE_TYPED_ARRAYS = IgniteSystemProperties.getBoolean(IGNITE_USE_TYPED_ARRAYS, DFLT_IGNITE_USE_TYPED_ARRAYS);
Review comment:
> I think the logic based on a static field that can be modified dynamically in runtime is fragile
Yes, it's not a perfect solution, but can't come with something more robust.
I want to achieve two things here:
1. Performance - check should be as quick as possible.
2. Ability to change this value dynamically from tests.
`BinaryContext` not available from some contexts - `BinaryUtils`, `BinaryReader`, etc.
Do you have another options?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756327686
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/binary/BinaryArraySelfTest.java
##########
@@ -0,0 +1,348 @@
+/*
+ * 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.binary;
+
+import java.util.Arrays;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.internal.binary.BinaryMarshallerSelfTest.TestClass1;
+import org.apache.ignite.internal.processors.platform.utils.PlatformUtils;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.junit.Test;
+
+/** */
+public class BinaryArraySelfTest extends AbstractTypedArrayTest {
+ /** */
+ private static Ignite server;
+
+ /** */
+ private static Ignite client;
+
+ /** */
+ private static IgniteCache<Object, Object> srvCache;
+
+ /** */
+ private static IgniteCache<Object, Object> cliCache;
+
+ /** {@inheritDoc} */
+ @Override protected void beforeTestsStarted() throws Exception {
+ super.beforeTestsStarted();
+
+ server = startGrid(0);
+ client = startClientGrid(1);
+
+ srvCache = server.createCache("my-cache");
+ cliCache = client.getOrCreateCache("my-cache");
+ }
+
+ /** */
+ @Test
+ public void testArrayKey() {
+ doTestArrayKey(srvCache);
+ doTestArrayKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayValue() {
+ doTestArrayValue(srvCache);
+ doTestArrayValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInKey() {
+ doTestArrayFieldInKey(srvCache);
+ doTestArrayFieldInKey(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testArrayFieldInValue() {
+ doTestArrayFieldInValue(srvCache);
+ doTestArrayFieldInValue(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBoxedPrimitivesArrays() {
+ doTestBoxedPrimitivesArrays(srvCache);
+ doTestBoxedPrimitivesArrays(cliCache);
+ }
+
+ /** */
+ @Test
+ public void testBinaryModeArray() {
+ doTestBinaryModeArray(srvCache);
+ doTestBinaryModeArray(cliCache);
+ }
+
+ /** */
+ private void doTestArrayKey(IgniteCache<Object, Object> c) {
+ TestClass1[] key1 = {new TestClass1(), new TestClass1()};
+ TestClass1[] key2 = {};
+ TestClass1[] key3 = new TestClass1[] {new TestClass1() {}};
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayFieldInKey(IgniteCache<Object, Object> c) {
+ TestClass key1 = new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()});
+ TestClass key2 = new TestClass(new TestClass1[] {});
+ TestClass key3 = new TestClass(new TestClass1[] {new TestClass1() {}});
+
+ c.put(key1, 1);
+ c.put(key2, 2);
+ c.put(key3, 3);
+
+ assertEquals(1, c.get(key1));
+ assertEquals(2, c.get(key2));
+ assertEquals(3, c.get(key3));
+
+ assertTrue(c.replace(key1, 1, 2));
+ assertTrue(c.replace(key2, 2, 3));
+ assertTrue(c.replace(key3, 3, 4));
+
+ assertTrue(c.remove(key1));
+ assertTrue(c.remove(key2));
+ assertTrue(c.remove(key3));
+ }
+
+ /** */
+ private void doTestArrayValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ c.put(2, new TestClass1[] {});
+ c.put(3, new TestClass1[] {new TestClass1() {}});
+
+ Object val1 = c.get(1);
+ Object val2 = c.get(2);
+ Object val3 = c.get(3);
+
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val1.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val2.getClass());
+ assertEquals(useTypedArrays ? TestClass1[].class : Object[].class, val3.getClass());
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestArrayFieldInValue(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass(new TestClass1[] {new TestClass1(), new TestClass1()}));
+ c.put(2, new TestClass(new TestClass1[] {}));
+ c.put(3, new TestClass(new TestClass1[] {new TestClass1() {}}));
+
+ TestClass val1 = (TestClass)c.get(1);
+ TestClass val2 = (TestClass)c.get(2);
+ TestClass val3 = (TestClass)c.get(3);
+
+ assertEquals(2, val1.getArr().length);
+ assertEquals(0, val2.getArr().length);
+ assertEquals(1, val3.getArr().length);
+
+ if (useTypedArrays) {
+ assertTrue(c.replace(1, val1, val2));
+ assertTrue(c.replace(2, val2, val3));
+ assertTrue(c.replace(3, val3, val1));
+ }
+
+ assertTrue(c.remove(1));
+ assertTrue(c.remove(2));
+ assertTrue(c.remove(3));
+ }
+
+ /** */
+ private void doTestBinaryModeArray(IgniteCache<Object, Object> c) {
+ c.put(1, new TestClass1[] {new TestClass1(), new TestClass1()});
+ Object obj = c.withKeepBinary().get(1);
+
+ assertEquals(useTypedArrays ? BinaryArray.class : Object[].class, obj.getClass());
+
+ if (useTypedArrays)
+ assertEquals(TestClass1[].class, ((BinaryObject)obj).deserialize().getClass());
+
+ assertTrue(c.remove(1));
+ }
+
+ /** */
+ private void doTestBoxedPrimitivesArrays(IgniteCache<Object, Object> c) {
+ Object[] data = new Object[] {
+ new Byte[] {1, 2, 3},
+ new Short[] {1, 2, 3},
+ new Integer[] {1, 2, 3},
+ new Long[] {1L, 2L, 3L},
+ new Float[] {1f, 2f, 3f},
+ new Double[] {1d, 2d, 3d},
+ new Character[] {'a', 'b', 'c'},
+ new Boolean[] {true, false},
+ };
+
+ for (Object item : data) {
+ c.put(1, item);
+
+ Object item0 = c.get(1);
+
+ if (useTypedArrays)
+ assertTrue(c.replace(1, item, item));
+
+ assertTrue(c.remove(1));
+
+ if (useTypedArrays)
+ assertEquals(item.getClass(), item0.getClass());
+
+ assertTrue(Arrays.equals((Object[])item, (Object[])item0));
+
+ c.put(item, 1);
+
+ assertEquals(1, c.get(item));
+
+ if (useTypedArrays)
+ assertTrue(c.replace(item, 1, 2));
+
+ assertTrue(c.remove(item));
+ }
+ }
+
+ /** */
+ @Test
+ public void testArrayOfBinariesSerDe() {
+ BinaryObject[] arr = new BinaryObject[] {
+ server.binary().toBinary(new TestClass1()),
+ server.binary().toBinary(new TestClass1())
+ };
+
+ Object obj = server.binary().toBinary(arr);
+
+ Object deser;
+
+ if (useTypedArrays) {
+ assertTrue(obj instanceof BinaryArray);
+
+ deser = ((BinaryArray)obj).deserialize();
+ }
+ else {
+ assertTrue(obj instanceof Object[]);
+
+ deser = PlatformUtils.unwrapBinariesInArray((Object[])obj);
+ }
+
+ assertEquals(Object[].class, deser.getClass());
Review comment:
if `toBinary` will keep `BinaryObject` array component type then during deserialization on `get` we will got `ArrayStoreException` because component type of the array is `BinaryObject` but element is `TestClass1`.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755765778
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/service/ClientServiceInvokeRequest.java
##########
@@ -147,6 +148,9 @@ public ClientServiceInvokeRequest(BinaryReaderExImpl reader) {
IgniteServices services = grp.services();
+ if (BinaryArray.useTypedArrays())
+ GridServiceProxy.KEEP_BINARY.set(true);
Review comment:
`KEEP_BINARY=true` required when caller is a .Net or thin client.
Not related to the service itself.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756168877
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/client/service/ClientServiceInvokeRequest.java
##########
@@ -147,6 +148,9 @@ public ClientServiceInvokeRequest(BinaryReaderExImpl reader) {
IgniteServices services = grp.services();
+ if (BinaryArray.useTypedArrays())
+ GridServiceProxy.KEEP_BINARY.set(true);
Review comment:
> Why KEEP BINARY is set only for useTypedArrays? I think it's useful for old implementation too.
`KEEP_BINARY=true` now used for all cases when caller Is thin client or .net platform.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755776811
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -198,10 +203,29 @@ else if (U.isToStringMethod(mtd))
else {
ctx.task().setThreadContext(TC_IO_POLICY, GridIoPolicy.SERVICE_POOL);
+ ServiceProxyCallable svcCallable;
Review comment:
`ServicesTypedArrayTests` added
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757586794
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
##########
@@ -179,6 +180,14 @@ public void writeValue(BinaryWriterExImpl writer, Object val, boolean forceCol,
return;
}
+ if (val instanceof BinaryArray) {
+ BinaryArray val0 = (BinaryArray)val;
+
+ writeArray(writer, GridBinaryMarshaller.OBJ_ARR, val0.array(), val0.componentTypeId());
Review comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756788054
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2066,6 +2071,38 @@ public static Object doReadOptimized(BinaryInputStream in, BinaryContext ctx, @N
return arr;
}
+ /**
+ * @param in Binary input stream.
+ * @param ctx Binary context.
+ * @param ldr Class loader.
+ * @param handles Holder for handles.
+ * @param detach Detach flag.
+ * @param deserialize Deep flag.
+ * @return Value.
+ * @throws BinaryObjectException In case of error.
+ */
+ public static BinaryArray doReadBinaryArray(BinaryInputStream in, BinaryContext ctx, ClassLoader ldr,
+ BinaryReaderHandlesHolder handles, boolean detach, boolean deserialize) {
+ int hPos = positionForHandle(in);
+
+ int compTypeId = in.readInt();
+ String compClsName = null;
+
+ if (compTypeId == GridBinaryMarshaller.UNREGISTERED_TYPE_ID)
+ compClsName = doReadClassName(in);
+
+ int len = in.readInt();
+
+ Object[] arr = new Object[len];
+
+ handles.setHandle(arr, hPos);
Review comment:
Fixed. Thanks.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759292957
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1364,10 +1364,16 @@ float readFloat(int fieldId) throws BinaryObjectException {
@Nullable @Override public Object[] readObjectArray() throws BinaryObjectException {
switch (checkFlag(OBJ_ARR)) {
case NORMAL:
- return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useBinaryArrays())
+ return BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
+ return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
case HANDLE:
- return readHandleField();
+ if (BinaryArray.useBinaryArrays())
Review comment:
It's safer to use `instanceof BinaryArray` in case we use `doReadObjectArray` somewhere before.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r754430902
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2554,6 +2591,41 @@ public static void validateEnumValues(String typeName, @Nullable Map<String, Int
return mergedMap;
}
+ /**
+ * @param obj {@link BinaryArray} or {@code Object[]}.
+ * @return Objects array.
+ */
+ public static Object[] rawArrayFromBinary(Object obj) {
+ if (obj instanceof BinaryArray)
+ // We want raw data(no deserialization).
+ return ((BinaryArray)obj).array();
+ else
+ // This can happen even in BinaryArray.USE_TYPED_ARRAY = true.
+ // In case user pass special array type to arguments, String[], for example.
+ return (Object[])obj;
+ }
+
+ /** */
+ public static Object[] rawArrayInArgs(Object[] args, boolean keepBinary) {
+ if (args == null)
+ return args;
+
+ for (int i = 0; i < args.length; i++) {
+ if (args[i] instanceof BinaryArray) {
+ BinaryArray arr = (BinaryArray)args[i];
+
+ args[i] = keepBinary ? arr.array() : arr.deserialize();
+ }
+ }
+
+ return args;
+ }
+
+ /** */
+ public static boolean isObjectArray(Class<?> cls) {
+ return Object[].class == cls || BinaryArray.class == cls;
Review comment:
It seems there no more `isAssignableFrom`, `isInstance` calls, so I think we can keep this method as is.
What do you think?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755268360
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/dml/DmlUtils.java
##########
@@ -111,6 +112,18 @@ public static Object convert(Object val, GridH2RowDescriptor desc, Class<?> expC
// We have to convert arrays of reference types manually -
// see https://issues.apache.org/jira/browse/IGNITE-4327
// Still, we only can convert from Object[] to something more precise.
+ if (type == Value.ARRAY && val instanceof BinaryArray) {
+ val = ((BinaryArray)val).deserialize();
Review comment:
Fixed. Thanks!
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757866994
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -317,7 +299,44 @@ private Object callServiceLocally(
return ((PlatformService)svc).invokeMethod(methodName(mtd), false, true, args, callAttrs);
}
else
- return mtd.invoke(svc, BinaryUtils.rawArrayInArgs(args, false));
+ return callServiceMethod(svc, mtd, args, callCtx);
+ }
+
+ /**
+ * @param svc Service to be called.
+ * @param mtd Method to call.
+ * @param args Method args.
+ * @param callCtx Service call context.
+ * @return Invocation result.
+ */
+ private static Object callServiceMethod(
+ Service svc,
+ Method mtd,
+ Object[] args,
+ @Nullable ServiceCallContext callCtx
+ ) throws InvocationTargetException, IllegalAccessException {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(callCtx);
+
+ try {
+ return mtd.invoke(svc, args);
+ }
+ finally {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(null);
+ }
+ }
+
+ /** */
+ private Object unmarshalResult(byte[] res) throws IgniteCheckedException {
+ Marshaller marsh = ctx.config().getMarshaller();
+
+ if (KEEP_BINARY.get() && BinaryArray.useBinaryArrays() && marsh instanceof BinaryMarshaller) {
Review comment:
If we keep arrays in binary mode here then array component type will be lose.
For the case when component type known on the server node.
Because we read `Object[] {binaryobject}` from the stream instead of `UserObject[] {userObject}`.
This case checked in .Net - `ServicesTest#TestCallJavaServiceRemote`:
```
var bins = svc.testBinarizableArray(arr);
Assert.AreEqual(typeof(PlatformComputeBinarizable[]), bins.GetType());
Assert.AreEqual(new[] {11, 12, 13},bins.Select(x => x.Field));
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759125191
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -2554,6 +2591,41 @@ public static void validateEnumValues(String typeName, @Nullable Map<String, Int
return mergedMap;
}
+ /**
+ * @param obj {@link BinaryArray} or {@code Object[]}.
+ * @return Objects array.
+ */
+ public static Object[] rawArrayFromBinary(Object obj) {
+ if (obj instanceof BinaryArray)
+ // We want raw data(no deserialization).
+ return ((BinaryArray)obj).array();
+ else
+ // This can happen even in BinaryArray.USE_TYPED_ARRAY = true.
+ // In case user pass special array type to arguments, String[], for example.
+ return (Object[])obj;
+ }
+
+ /** */
+ public static Object[] rawArrayInArgs(Object[] args, boolean keepBinary) {
+ if (args == null)
+ return args;
+
+ for (int i = 0; i < args.length; i++) {
+ if (args[i] instanceof BinaryArray) {
+ BinaryArray arr = (BinaryArray)args[i];
+
+ args[i] = keepBinary ? arr.array() : arr.deserialize();
+ }
+ }
+
+ return args;
+ }
+
+ /** */
+ public static boolean isObjectArray(Class<?> cls) {
+ return Object[].class == cls || BinaryArray.class == cls;
Review comment:
There are several `val instanceof BinaryArray` calls, so any child class of BinaryArray will also fit this condition. For consistency, I think, it's better to keep the same semantic here (not only BinaryArray class itself but also any of its child). But we don't have a BinaryArray child now, so it's up to you, you can leave it as is.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r755776607
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -486,7 +536,7 @@ private Object callService(Service srv, Method mtd) throws Exception {
throw new GridServiceMethodNotFoundException(svcName, mtdName, argTypes);
try {
- return mtd.invoke(srv, args);
+ return mtd.invoke(srv, BinaryUtils.rawArrayInArgs(args, false));
Review comment:
Implemented. Changes in `GridTaskWorker` reverted.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r756278228
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/GridBinaryMarshaller.java
##########
@@ -153,6 +154,12 @@
/** Binary enum */
public static final byte BINARY_ENUM = 38;
+ /**
+ * {@link BinaryObject} interface type id.
+ * Used to store array component type in case of {@code BinaryObject[]}.
+ */
+ public static final byte BINARY_OBJ_INTERFACE = 39;
Review comment:
This tested in .Net - `ServicesTest#DoTestBinary`:
```
// Binary object array.
var binArr = arr.Select(Grid1.GetBinary().ToBinary<IBinaryObject>).ToArray();
var binObjs = binSvc.testBinaryObjectArray(binArr);
Assert.AreEqual(new[] {11, 12, 13}, binObjs.Select(x => x.GetField<int>("Field")));
```
Anyway, I reworked solution to exclude this interface from the patch.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r759350195
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryReaderExImpl.java
##########
@@ -1364,10 +1364,16 @@ float readFloat(int fieldId) throws BinaryObjectException {
@Nullable @Override public Object[] readObjectArray() throws BinaryObjectException {
switch (checkFlag(OBJ_ARR)) {
case NORMAL:
- return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
+ if (BinaryArray.useBinaryArrays())
+ return BinaryUtils.doReadBinaryArray(in, ctx, ldr, this, false, true).deserialize(ldr);
+ else
+ return BinaryUtils.doReadObjectArray(in, ctx, ldr, this, false, true);
case HANDLE:
- return readHandleField();
+ if (BinaryArray.useBinaryArrays())
Review comment:
Fixed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9490: IGNITE-14742 BinaryArray introduced
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9490:
URL: https://github.com/apache/ignite/pull/9490#discussion_r757567289
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/service/GridServiceProxy.java
##########
@@ -317,7 +299,44 @@ private Object callServiceLocally(
return ((PlatformService)svc).invokeMethod(methodName(mtd), false, true, args, callAttrs);
}
else
- return mtd.invoke(svc, BinaryUtils.rawArrayInArgs(args, false));
+ return callServiceMethod(svc, mtd, args, callCtx);
+ }
+
+ /**
+ * @param svc Service to be called.
+ * @param mtd Method to call.
+ * @param args Method args.
+ * @param callCtx Service call context.
+ * @return Invocation result.
+ */
+ private static Object callServiceMethod(
+ Service svc,
+ Method mtd,
+ Object[] args,
+ @Nullable ServiceCallContext callCtx
+ ) throws InvocationTargetException, IllegalAccessException {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(callCtx);
+
+ try {
+ return mtd.invoke(svc, args);
+ }
+ finally {
+ if (callCtx != null)
+ ServiceCallContextHolder.current(null);
+ }
+ }
+
+ /** */
+ private Object unmarshalResult(byte[] res) throws IgniteCheckedException {
+ Marshaller marsh = ctx.config().getMarshaller();
+
+ if (KEEP_BINARY.get() && BinaryArray.useBinaryArrays() && marsh instanceof BinaryMarshaller) {
Review comment:
Why we check `BinaryArray.useBinaryArrays()` here? Why not return marshalled (binary) data always if KEEP_BINARY=true?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/services/PlatformServices.java
##########
@@ -710,7 +709,10 @@ private static boolean areMethodArgsCompatible(Class[] argTypes, Object[] args)
Object arg = args[i];
Class argType = wrap(argTypes[i]);
- if (arg != null && !argType.isAssignableFrom(arg.getClass()))
+ if (arg == null)
+ continue;
+
+ if (!argType.isAssignableFrom(arg.getClass()))
Review comment:
Looks like redundant change
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/platform/utils/PlatformUtils.java
##########
@@ -924,8 +923,10 @@ else if (BinaryUtils.knownMap(o))
return unwrapBinariesIfNeeded((Map<Object, Object>)o);
else if (o instanceof Object[])
return unwrapBinariesInArray((Object[])o);
+/*
Review comment:
Comment should be removed
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/builder/BinaryBuilderSerializer.java
##########
@@ -179,6 +180,14 @@ public void writeValue(BinaryWriterExImpl writer, Object val, boolean forceCol,
return;
}
+ if (val instanceof BinaryArray) {
+ BinaryArray val0 = (BinaryArray)val;
+
+ writeArray(writer, GridBinaryMarshaller.OBJ_ARR, val0.array(), val0.componentTypeId());
Review comment:
```
if (val0.componentTypeId() == GridBinaryMarshaller.UNREGISTERED_TYPE_ID)
writeArray(writer, GridBinaryMarshaller.OBJ_ARR, val0.array(), val0.componentClassName());
else
writeArray(writer, GridBinaryMarshaller.OBJ_ARR, val0.array(), val0.componentTypeId());
```
?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org