You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "nizhikov (via GitHub)" <gi...@apache.org> on 2026/01/12 14:03:59 UTC
[PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
nizhikov opened a new pull request, #12626:
URL: https://github.com/apache/ignite/pull/12626
…nary-impl
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685394855
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean skipHdrCheck,
+ boolean forUnmarshal);
+
+ /**
+ * @param ctx Context.
+ * @param failIfUnregistered Flag to fail while writing object of unregistered type.
+ * @param typeId Type id.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, boolean failIfUnregistered, int typeId);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out, BinaryWriterSchemaHolder schema);
+
+ /**
+ * Create accessor for the field.
+ *
+ * @param field Field.
+ * @param id FIeld ID.
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685436954
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java:
##########
@@ -179,6 +180,20 @@ TreeSet.class, new BinaryTreeSetWriteReplacer()
/** FNV1 hash prime. */
private static final int FNV1_PRIME = 0x01000193;
+ /** */
+ public static final BinariesFactory binariesFactory;
+
+ static {
+ Iterator<BinariesFactory> factories = CommonUtils.loadService(BinariesFactory.class).iterator();
+
+ A.ensure(
+ factories.hasNext(),
+ "Implementation for BinariesFactory service not found. Please add ignite-binary-impl to classpath"
Review Comment:
Done
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685464669
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -781,7 +781,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
break;
case BINARY_ENUM:
- writer.writeBinaryEnum((BinaryEnumObjectImpl)obj);
+ writer.writeBinaryEnum((BinaryObjectEx)obj);
Review Comment:
We want to expose only binary-api in writer.
And BinaryEnumObjectImpl is not public api and will be moved to binary-impl module.
> it can broke smth if assertions are disabled
`BinaryObject#ordinal` throws for non enum.
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12626:
URL: https://github.com/apache/ignite/pull/12626#issuecomment-3740424594
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) **Quality Gate failed**
Failed conditions
 [34 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685448669
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -781,7 +781,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
break;
case BINARY_ENUM:
- writer.writeBinaryEnum((BinaryEnumObjectImpl)obj);
+ writer.writeBinaryEnum((BinaryObjectEx)obj);
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12626:
URL: https://github.com/apache/ignite/pull/12626#issuecomment-3740395134
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) **Quality Gate failed**
Failed conditions
 [34 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2682537917
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -365,12 +365,12 @@ else if (useOptMarshaller)
if (!ids.add(fieldId))
throw new BinaryObjectException("Duplicate field ID: " + name);
- BinaryFieldAccessor fieldInfo = BinaryFieldAccessor.create(f, fieldId);
+ BinaryFieldAccessor fieldInfo = BinaryUtils.binariesFactory.create(f, fieldId);
fields0.put(name, fieldInfo);
if (metaDataEnabled)
- stableFieldsMeta.put(name, new BinaryFieldMetadata(fieldInfo));
+ stableFieldsMeta.put(name, new BinaryFieldMetadata(fieldInfo.mode.typeId(), fieldInfo.id));
Review Comment:
```suggestion
stableFieldsMeta.put(name, new BinaryFieldMetadata(fieldInfo.mode().typeId(), fieldInfo.id));
```
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean skipHdrCheck,
+ boolean forUnmarshal);
+
+ /**
+ * @param ctx Context.
+ * @param failIfUnregistered Flag to fail while writing object of unregistered type.
+ * @param typeId Type id.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, boolean failIfUnregistered, int typeId);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out, BinaryWriterSchemaHolder schema);
Review Comment:
missed "schema" param
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685448669
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -781,7 +781,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
break;
case BINARY_ENUM:
- writer.writeBinaryEnum((BinaryEnumObjectImpl)obj);
+ writer.writeBinaryEnum((BinaryObjectEx)obj);
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2684993543
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryWriterEx.java:
##########
@@ -131,6 +132,11 @@ public interface BinaryWriterEx extends BinaryWriter, BinaryRawWriter, ObjectOut
*/
public int schemaId();
+ /**
+ * @param schemaId Schema ID.
+ */
+ public void schemaId(int schemaId);
Review Comment:
i understand why you need it but just for defence from potential problems probably it need to be appended some additional assert check for schemaId change invariants ?
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685391534
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean skipHdrCheck,
+ boolean forUnmarshal);
+
+ /**
+ * @param ctx Context.
+ * @param failIfUnregistered Flag to fail while writing object of unregistered type.
+ * @param typeId Type id.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, boolean failIfUnregistered, int typeId);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out, BinaryWriterSchemaHolder schema);
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov merged PR #12626:
URL: https://github.com/apache/ignite/pull/12626
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685443819
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryReaderEx.java:
##########
@@ -85,4 +85,11 @@ public interface BinaryReaderEx extends BinaryReader, BinaryRawReader, BinaryRea
* @return Offset.
*/
public boolean findFieldByName(String name);
+
+ /**
+ * Get or create object schema.
+ *
+ * @return Schema.
+ */
+ public BinarySchema getOrCreateSchema();
Review Comment:
Done
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12626:
URL: https://github.com/apache/ignite/pull/12626#issuecomment-3738761387
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) **Quality Gate failed**
Failed conditions
 [50 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12626:
URL: https://github.com/apache/ignite/pull/12626#issuecomment-3743451009
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) **Quality Gate failed**
Failed conditions
 [34 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2684967526
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java:
##########
@@ -179,6 +180,20 @@ TreeSet.class, new BinaryTreeSetWriteReplacer()
/** FNV1 hash prime. */
private static final int FNV1_PRIME = 0x01000193;
+ /** */
+ public static final BinariesFactory binariesFactory;
+
+ static {
+ Iterator<BinariesFactory> factories = CommonUtils.loadService(BinariesFactory.class).iterator();
+
+ A.ensure(
+ factories.hasNext(),
+ "Implementation for BinariesFactory service not found. Please add ignite-binary-impl to classpath"
Review Comment:
```suggestion
"Implementation for " + BinariesFactory.class.getSimpleName() + " service not found. Please add ignite-binary-impl to classpath"
```
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java:
##########
@@ -18,911 +18,45 @@
package org.apache.ignite.internal.binary;
import java.lang.reflect.Field;
-import java.math.BigDecimal;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Map;
-import java.util.UUID;
-import org.apache.ignite.binary.BinaryObjectException;
-import org.apache.ignite.internal.UnregisteredBinaryTypeException;
-import org.apache.ignite.internal.UnregisteredClassException;
-import org.apache.ignite.internal.util.CommonUtils;
-import org.apache.ignite.internal.util.GridUnsafe;
-import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.S;
/**
* Field accessor to speedup access.
Review Comment:
I suppose - after accessors are removed this class is not an accessor at all but pojo for holding some params and probably can be renamed, what do you think ?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean skipHdrCheck,
+ boolean forUnmarshal);
+
+ /**
+ * @param ctx Context.
+ * @param failIfUnregistered Flag to fail while writing object of unregistered type.
+ * @param typeId Type id.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, boolean failIfUnregistered, int typeId);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out);
+
+ /**
+ * @param ctx Context.
+ * @param out Output stream.
+ * @return Writer instance.
+ */
+ public BinaryWriterEx writer(BinaryContext ctx, BinaryOutputStream out, BinaryWriterSchemaHolder schema);
+
+ /**
+ * Create accessor for the field.
+ *
+ * @param field Field.
+ * @param id FIeld ID.
Review Comment:
typo
##########
modules/binary/impl/src/main/java/org/apache/ignite/internal/binary/BinaryWriterExImpl.java:
##########
@@ -1586,4 +1567,298 @@ private void clearState(boolean clearHandler) {
if (clearHandler)
this.handles = null;
}
+
+ /** {@inheritDoc} */
+ @Override public void writeField(Object obj, BinaryFieldAccessor fld) throws BinaryObjectException {
+ writeFieldIdNoSchemaUpdate(fld.id);
+
+ switch (fld.mode) {
+ case P_BYTE:
+ writeByteFieldPrimitive(GridUnsafe.getByteField(obj, fld.offset));
+
+ break;
+
+ case P_BOOLEAN:
+ writeBooleanFieldPrimitive(GridUnsafe.getBooleanField(obj, fld.offset));
+
+ break;
+
+ case P_SHORT:
+ writeShortFieldPrimitive(GridUnsafe.getShortField(obj, fld.offset));
+
+ break;
+
+ case P_CHAR:
+ writeCharFieldPrimitive(GridUnsafe.getCharField(obj, fld.offset));
+
+ break;
+
+ case P_INT:
+ writeIntFieldPrimitive(GridUnsafe.getIntField(obj, fld.offset));
+
+ break;
+
+ case P_LONG:
+ writeLongFieldPrimitive(GridUnsafe.getLongField(obj, fld.offset));
+
+ break;
+
+ case P_FLOAT:
+ writeFloatFieldPrimitive(GridUnsafe.getFloatField(obj, fld.offset));
+
+ break;
+
+ case P_DOUBLE:
+ writeDoubleFieldPrimitive(GridUnsafe.getDoubleField(obj, fld.offset));
+
+ break;
+
+ case BYTE:
+ case BOOLEAN:
+ case SHORT:
+ case CHAR:
+ case INT:
+ case LONG:
+ case FLOAT:
+ case DOUBLE:
+ case DECIMAL:
+ case STRING:
+ case UUID:
+ case DATE:
+ case TIMESTAMP:
+ case TIME:
+ case BYTE_ARR:
+ case SHORT_ARR:
+ case INT_ARR:
+ case LONG_ARR:
+ case FLOAT_ARR:
+ case DOUBLE_ARR:
+ case CHAR_ARR:
+ case BOOLEAN_ARR:
+ case DECIMAL_ARR:
+ case STRING_ARR:
+ case UUID_ARR:
+ case DATE_ARR:
+ case TIMESTAMP_ARR:
+ case TIME_ARR:
+ case ENUM_ARR:
+ case OBJECT_ARR:
+ case BINARY_OBJ:
+ case BINARY:
+ default:
+ assert obj != null;
+
+ Object val;
+
+ try {
+ val = fld.field.get(obj);
+ }
+ catch (IllegalAccessException e) {
+ throw new BinaryObjectException("Failed to get value for field: " + fld.field, e);
+ }
+
+ switch (mode(fld, val)) {
+ case BYTE:
+ writeByteField((Byte)val);
+
+ break;
+
+ case SHORT:
+ writeShortField((Short)val);
+
+ break;
+
+ case INT:
+ writeIntField((Integer)val);
+
+ break;
+
+ case LONG:
+ writeLongField((Long)val);
+
+ break;
+
+ case FLOAT:
+ writeFloatField((Float)val);
+
+ break;
+
+ case DOUBLE:
+ writeDoubleField((Double)val);
+
+ break;
+
+ case CHAR:
+ writeCharField((Character)val);
+
+ break;
+
+ case BOOLEAN:
+ writeBooleanField((Boolean)val);
+
+ break;
+
+ case DECIMAL:
+ writeDecimal((BigDecimal)val);
+
+ break;
+
+ case STRING:
+ writeString((String)val);
+
+ break;
+
+ case UUID:
+ writeUuid((UUID)val);
+
+ break;
+
+ case DATE:
+ writeDate((Date)val);
+
+ break;
+
+ case TIMESTAMP:
+ writeTimestamp((Timestamp)val);
+
+ break;
+
+ case TIME:
+ writeTime((Time)val);
+
+ break;
+
+ case BYTE_ARR:
+ writeByteArray((byte[])val);
+
+ break;
+
+ case SHORT_ARR:
+ writeShortArray((short[])val);
+
+ break;
+
+ case INT_ARR:
+ writeIntArray((int[])val);
+
+ break;
+
+ case LONG_ARR:
+ writeLongArray((long[])val);
+
+ break;
+
+ case FLOAT_ARR:
+ writeFloatArray((float[])val);
+
+ break;
+
+ case DOUBLE_ARR:
+ writeDoubleArray((double[])val);
+
+ break;
+
+ case CHAR_ARR:
+ writeCharArray((char[])val);
+
+ break;
+
+ case BOOLEAN_ARR:
+ writeBooleanArray((boolean[])val);
+
+ break;
+
+ case DECIMAL_ARR:
+ writeDecimalArray((BigDecimal[])val);
+
+ break;
+
+ case STRING_ARR:
+ writeStringArray((String[])val);
+
+ break;
+
+ case UUID_ARR:
+ writeUuidArray((UUID[])val);
+
+ break;
+
+ case DATE_ARR:
+ writeDateArray((Date[])val);
+
+ break;
+
+ case TIMESTAMP_ARR:
+ writeTimestampArray((Timestamp[])val);
+
+ break;
+
+ case TIME_ARR:
+ writeTimeArray((Time[])val);
+
+ break;
+
+ case OBJECT_ARR:
+ writeObjectArray((Object[])val);
+
+ break;
+
+ case COL:
+ writeCollection((Collection<?>)val);
+
+ break;
+
+ case MAP:
+ writeMap((Map<?, ?>)val);
+
+ break;
+
+ case BINARY_OBJ:
+ writeBinaryObject((BinaryObjectImpl)val);
+
+ break;
+
+ case ENUM:
+ writeEnum((Enum<?>)val);
+
+ break;
+
+ case BINARY_ENUM:
+ writeBinaryEnum((BinaryEnumObjectImpl)val);
+
+ break;
+
+ case ENUM_ARR:
+ doWriteEnumArray((Object[])val);
+
+ break;
+
+ case BINARY:
+ case OBJECT:
+ case PROXY:
+ writeObject(val);
+
+ break;
+
+ case CLASS:
+ writeClass((Class)val);
+
+ break;
+
+ default:
+ assert false : "Invalid mode: " + fld.mode;
Review Comment:
probably AssertionError need to be thrown from here ? It looks like unexpected behavior, wdyt ?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryClassDescriptor.java:
##########
@@ -781,7 +781,7 @@ void write(Object obj, BinaryWriterExImpl writer) throws BinaryObjectException {
break;
case BINARY_ENUM:
- writer.writeBinaryEnum((BinaryEnumObjectImpl)obj);
+ writer.writeBinaryEnum((BinaryObjectEx)obj);
Review Comment:
whats the purpose in such a change ? writeBinaryEnum implementation still invalidate that "val instanceof BinaryEnumObjectImpl" also it can broke smth if assertions are disabled
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java:
##########
@@ -18,911 +18,45 @@
package org.apache.ignite.internal.binary;
import java.lang.reflect.Field;
-import java.math.BigDecimal;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Map;
-import java.util.UUID;
-import org.apache.ignite.binary.BinaryObjectException;
-import org.apache.ignite.internal.UnregisteredBinaryTypeException;
-import org.apache.ignite.internal.UnregisteredClassException;
-import org.apache.ignite.internal.util.CommonUtils;
-import org.apache.ignite.internal.util.GridUnsafe;
-import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.S;
/**
* Field accessor to speedup access.
*/
-abstract class BinaryFieldAccessor {
+class BinaryFieldAccessor {
/** Field ID. */
- protected final int id;
+ final int id;
/** Field name */
- protected final String name;
+ final String name;
/** Mode. */
- protected final BinaryWriteMode mode;
+ final BinaryWriteMode mode;
- /**
- * Create accessor for the field.
- *
- * @param field Field.
- * @param id FIeld ID.
- * @return Accessor.
- */
- public static BinaryFieldAccessor create(Field field, int id) {
- BinaryWriteMode mode = BinaryUtils.mode(field.getType());
-
- switch (mode) {
- case P_BYTE:
- return new BytePrimitiveAccessor(field, id);
-
- case P_BOOLEAN:
- return new BooleanPrimitiveAccessor(field, id);
-
- case P_SHORT:
- return new ShortPrimitiveAccessor(field, id);
-
- case P_CHAR:
- return new CharPrimitiveAccessor(field, id);
-
- case P_INT:
- return new IntPrimitiveAccessor(field, id);
-
- case P_LONG:
- return new LongPrimitiveAccessor(field, id);
+ /** Offset. Used for primitive fields, only. */
+ final long offset;
- case P_FLOAT:
- return new FloatPrimitiveAccessor(field, id);
+ /** Target field. */
+ final Field field;
- case P_DOUBLE:
- return new DoublePrimitiveAccessor(field, id);
-
- case BYTE:
- case BOOLEAN:
- case SHORT:
- case CHAR:
- case INT:
- case LONG:
- case FLOAT:
- case DOUBLE:
- case DECIMAL:
- case STRING:
- case UUID:
- case DATE:
- case TIMESTAMP:
- case TIME:
- case BYTE_ARR:
- case SHORT_ARR:
- case INT_ARR:
- case LONG_ARR:
- case FLOAT_ARR:
- case DOUBLE_ARR:
- case CHAR_ARR:
- case BOOLEAN_ARR:
- case DECIMAL_ARR:
- case STRING_ARR:
- case UUID_ARR:
- case DATE_ARR:
- case TIMESTAMP_ARR:
- case TIME_ARR:
- case ENUM_ARR:
- case OBJECT_ARR:
- case BINARY_OBJ:
- case BINARY:
- return new DefaultFinalClassAccessor(field, id, mode, false);
-
- default:
- return new DefaultFinalClassAccessor(field, id, mode, !CommonUtils.isFinal(field.getType()));
- }
- }
+ /** Dynamic accessor flag. */
+ final boolean dynamic;
/**
* Protected constructor.
*
* @param id Field ID.
* @param mode Mode;
*/
- protected BinaryFieldAccessor(Field field, int id, BinaryWriteMode mode) {
+ protected BinaryFieldAccessor(Field field, int id, BinaryWriteMode mode, long offset, boolean dynamic) {
Review Comment:
plz fix java doc, append missing params
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryReaderEx.java:
##########
@@ -85,4 +85,11 @@ public interface BinaryReaderEx extends BinaryReader, BinaryRawReader, BinaryRea
* @return Offset.
*/
public boolean findFieldByName(String name);
+
+ /**
+ * Get or create object schema.
+ *
+ * @return Schema.
+ */
+ public BinarySchema getOrCreateSchema();
Review Comment:
BinarySchema has package private visibility is it ok to have such approach in interface ? probably it need to be changed with "public" ? wdyt ?
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryWriterEx.java:
##########
@@ -131,6 +132,11 @@ public interface BinaryWriterEx extends BinaryWriter, BinaryRawWriter, ObjectOut
*/
public int schemaId();
+ /**
+ * @param schemaId Schema ID.
+ */
+ public void schemaId(int schemaId);
Review Comment:
i understand why you need it but just for defence from potential problems probably it need to be appended sode additional assert check for schemaId change invariants ?
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685656803
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldDescriptor.java:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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.lang.reflect.Field;
+
+/**
+ * Field descriptor.
+ */
+class BinaryFieldDescriptor {
+ /** Field ID. */
+ final int id;
+
+ /** Field name */
Review Comment:
Comment without "." at the end, do we need to fill a codestyle issue for such a check if it still not present ?
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685447358
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryFieldAccessor.java:
##########
@@ -18,911 +18,45 @@
package org.apache.ignite.internal.binary;
import java.lang.reflect.Field;
-import java.math.BigDecimal;
-import java.sql.Time;
-import java.sql.Timestamp;
-import java.util.Collection;
-import java.util.Date;
-import java.util.Map;
-import java.util.UUID;
-import org.apache.ignite.binary.BinaryObjectException;
-import org.apache.ignite.internal.UnregisteredBinaryTypeException;
-import org.apache.ignite.internal.UnregisteredClassException;
-import org.apache.ignite.internal.util.CommonUtils;
-import org.apache.ignite.internal.util.GridUnsafe;
-import org.apache.ignite.internal.util.typedef.F;
-import org.apache.ignite.internal.util.typedef.internal.S;
/**
* Field accessor to speedup access.
Review Comment:
Renamed to BinaryFieldDescriptor
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2682496041
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
Review Comment:
the same as above
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2682493384
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
Review Comment:
"is needed" (as above) or "is need" let\`s align this java doc ?
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12626:
URL: https://github.com/apache/ignite/pull/12626#issuecomment-3738869299
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) **Quality Gate failed**
Failed conditions
 [50 New Code Smells](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626) (required ≤ 1)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12626)
##
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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
Re: [PR] IGNITE-27540 Move BinaryReader and BinaryWriter implementations to binary-impl [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #12626:
URL: https://github.com/apache/ignite/pull/12626#discussion_r2685387583
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
Review Comment:
Done
##########
modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinariesFactory.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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.lang.reflect.Field;
+import org.apache.ignite.internal.binary.streams.BinaryInputStream;
+import org.apache.ignite.internal.binary.streams.BinaryOutputStream;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Factory for binaries classes creation.
+ */
+public interface BinariesFactory {
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param forUnmarshal {@code True} if reader is needed to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx, BinaryInputStream in, ClassLoader ldr, boolean forUnmarshal);
+
+ /**
+ * Creates reader instance.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
+ */
+ public BinaryReaderEx reader(BinaryContext ctx,
+ BinaryInputStream in,
+ ClassLoader ldr,
+ @Nullable BinaryReaderHandles hnds,
+ boolean forUnmarshal);
+
+ /**
+ * Constructor.
+ *
+ * @param ctx Context.
+ * @param in Input stream.
+ * @param ldr Class loader.
+ * @param hnds Context.
+ * @param skipHdrCheck Whether to skip header check.
+ * @param forUnmarshal {@code True} if reader is need to unmarshal object.
Review Comment:
Done
--
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