IGNITE-28972 Generation must fail Message to byte[] marshalling by nizhikov · Pull Request #13461 · apache/ignite

nizhikov · 2026-08-10T19:40:01Z

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)
The pull request has been checked by the Teamcity Bot and
the green visa attached to the JIRA ticket (see tab PR Check at TC.Bot - Instance 1 or TC.Bot - Instance 2)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

anton-vinogradov

Thanks for picking this up, the gap is real.

One blocker: the check is applied before MarshalledKind is computed, so it keys off the field type while the rule actually depends on the companion wire field. ELEMENTS (an X[] companion) and MAP are written by the communication protocol, not by a marshaller, so Message elements there are correct and must stay allowed. As written, Set<SomeMessage> with a SomeMessage[] companion is rejected even though generation emits writeObjectArray(..., MessageCollectionItemType.MSG) and produces no marshaller blob at all. The current tests do not catch it: testMarshalledCollectionMessage uses GridCacheVersion, which is not a Message, and the symmetric map case is allowed only because map form is excluded by hand.

Computing the kind first and checking only BLOB and ELEMENT_BLOBS fixes the false positive, lets the same walk cover nested generics, and keeps all 49 MessageProcessorTest tests green including the four new ones (verified locally, the error text can stay as is).

The rest are smaller notes inline.

anton-vinogradov · 2026-08-10T20:09:54Z

        if (ann == null)
            return null;

+        ensureCorrectlyAnnotated(field, ann);


The check runs before the kind is computed, so it judges the field type instead of the companion and rejects a working pattern (Set + GridTopicMessage[] companion) that the protocol writes itself. Please move the check after the kind is known and apply it only to BLOB and ELEMENT_BLOBS.

Vladsz83 · 2026-08-11T09:57:54Z

+public class MarshalledOnMessageCollection implements Message {
+    /** */
+    @Marshalled("msgBytes")
+    Collection<TestMessage> msgColl;


Do we need to check also List and List in the same way?

Generator works for all Collections, don't think we want to check all of the Collection implementations in tests.

Vladsz83 · 2026-08-11T09:59:10Z

+/** */
+public class MarshalledOnMessageMap implements Message {
+    /** */
+    @Marshalled("bytes")


Do we need to check also @Marshalled with keys() and values()?

Checked in TestMarshalledMapMessage

There are Collection<GridCacheVersion> and Collection<GridCacheVersion> mapVals; Expected: byte[] arrays

Vladsz83 · 2026-08-11T13:11:35Z

+/** */
+public class MarshalledOnMessageMap implements Message {
+    /** */
+    @Marshalled("bytes")


There are Collection<GridCacheVersion> and Collection<GridCacheVersion> mapVals; Expected: byte[] arrays

Vladsz83 · 2026-08-11T13:28:36Z

+
+    /** Test that {@code @Marshalled} annotation on raw {@link Map} fail generation. */
+    @Test
+    public void testRawMapFailGeneration() {


Join with testRawCollectionFailGeneration? + The same, do we need "TestMessage.java"

Vladsz83 · 2026-08-11T13:44:32Z

-            return MarshalledKind.MAP;
+            res = MarshalledKind.MAP;
+        else {
+            TypeMirror wire = requireEnclosed(enclosed, ann.value(), "@Marshalled").asType();


Just a notise. Annotation here look as something important. Important to search. But is onle for the error.

anton-vinogradov · 2026-08-11T14:40:04Z

+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** */
+public class MarshalledOnMessageArray implements Message {


Let's call such messages as IncorrectXXX

anton-vinogradov · 2026-08-11T14:56:57Z

+public class MarshalledOnMessageCollection2 implements Message {
+    /** */
+    @Marshalled("msgBytes")
+    Collection<TestMessage> msgColl;


Should we check wildcards as well, like Collection<? extends TestMessage>?

anton-vinogradov · 2026-08-11T14:58:23Z

+        if (foundMessageToBytesTransformation(field.asType(), field, ann)) {
+            env.getMessager().printMessage(Diagnostic.Kind.ERROR,
+                "Message must be written by dedicated message serializers. " +
+                    "Remove @" + Marshalled.class.getSimpleName() + " annotation and remove companion field.", field);


And set an @order annotation?

github-actions · 2026-08-11T15:36:40Z

Possible compatibility issues. Please, check rolling upgrade cases

This PR modifies protected classes (with Order annotation).
Changes to these classes can break rolling upgrade compatibility.

Affected files:

modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java

IGNITE-28972 Generation must fail Message to byte[] marshalling

4a1ed7c

github-actions Bot added the compatibility label Aug 10, 2026

anton-vinogradov reviewed Aug 10, 2026

View reviewed changes

nizhikov added 6 commits August 10, 2026 23:19

Merge branch 'master' into IGNITE-28972

a8cb318

IGNITE-28972 Generation must fail Message to byte[] marshalling

3504596

IGNITE-28972 Generation must fail Message to byte[] marshalling

0d4b5d4

IGNITE-28972 Generation must fail Message to byte[] marshalling

69a5031

IGNITE-28972 Generation must fail Message to byte[] marshalling

bb138d3

Merge branch 'master' into IGNITE-28972

384ee6f

Vladsz83 reviewed Aug 11, 2026

View reviewed changes

nizhikov added 3 commits August 11, 2026 13:34

IGNITE-28972 Generation must fail Message to byte[] marshalling

41f3013

IGNITE-28972 Generation must fail Message to byte[] marshalling

f589a77

IGNITE-28972 Generation must fail Message to byte[] marshalling

307ada9

Vladsz83 reviewed Aug 11, 2026

View reviewed changes

Comment thread modules/core/src/test/java/org/apache/ignite/internal/codegen/MessageProcessorTest.java

Vladsz83 reviewed Aug 11, 2026

View reviewed changes

IGNITE-28972 Generation must fail Message to byte[] marshalling

cd32f5f

Vladsz83 approved these changes Aug 11, 2026

View reviewed changes

IGNITE-28972 Generation must fail Message to byte[] marshalling

14f3767

anton-vinogradov requested changes Aug 11, 2026

View reviewed changes

IGNITE-28972 Generation must fail Message to byte[] marshalling

12d749e

Uh oh!

Conversation

nizhikov commented Aug 10, 2026

The Contribution Checklist

Notes

Uh oh!

anton-vinogradov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 11, 2026

Possible compatibility issues. Please, check rolling upgrade cases

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants