Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proto3-json-serializer Fails to Serialize Repeated Fields in Array #112

Open
7 tasks done
hardeep-qa opened this issue Jan 10, 2025 · 3 comments
Open
7 tasks done
Labels
size: m Pull request size is medium. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@hardeep-qa
Copy link

hardeep-qa commented Jan 10, 2025

Please make sure you have searched for information in the following guides.

Link to the code that reproduces this issue. A link to a public Github Repository or gist with a minimal reproduction.

N/A

A step-by-step description of how to reproduce the issue, based on the linked reproduction.

proto3-json-serializer version: 2.0.2
Node.js version: v20.13.1
protobufjs version: "^7.4.0"
Operating system: Windows

Description: The proto3-json-serializer is not serializing repeated fields in arrays properly. When attempting to serialize a message with repeated fields, the resulting JSON output is missing the array data.

1. Use the following JsonPayload:

{
  "hierarchyData": {
    "articleHierarchies": [
      {
        "hierarchyId": "Z1",
        "hierarchyNode": "1093895"
      },
      {
        "hierarchyId": "Z1",
        "hierarchyNode": "109389502"
      }
    ]
  }
}

2. Serialize the message using the provided logic:

import fs from 'fs';
import path from 'path';
import protobuf from 'protobufjs';
import * as serializer from 'proto3-json-serializer';

async function loadAllProtobufSchemas(): Promise<protobuf.Root> {
  const protoDirPath = 'data/services/protos';
  const protobufRoot = new protobuf.Root();
  const protoFiles = fs.readdirSync(protoDirPath).filter((file) => file.endsWith('.proto'));

  for (const protoFile of protoFiles) {
    await protobufRoot.load(path.join(protoDirPath, protoFile));
  }
  return protobufRoot;
}

export async function createProtobufMessage(messageTypeName: string, messagePayload: any): Promise<Uint8Array> {
  const protobufRoot = await loadAllProtobufSchemas();
  const protobufMessageType = protobufRoot.lookupType(messageTypeName);
  const protobufMessage = serializer.fromProto3JSON(protobufMessageType, messagePayload)!;
  console.log('>>1', JSON.stringify(protobufMessage, null, 2));
  const encodedMessage = protobufMessageType.encode(protobufMessage).finish();
  return encodedMessage;
}

3. Observe the logs after serialization:

{
  "hierarchyData": {}
}

4. Expecting the logs after serialization:

hierarchyData: {
      articleHierarchies: [
        {
          "hierarchyId": "Z1",
          "hierarchyNode": "1093895"
        },
        {
          "hierarchyId": "Z1",
          "hierarchyNode": "109389502"
        },
     ]

A clear and concise description of what the bug is, and what you expected to happen.

Bug Description: The proto3-json-serializer is not properly serializing repeated fields in arrays. When attempting to serialize a message with repeated fields, the resulting JSON output is missing the array data.

Expected Behavior: The serialized JSON should include the articleHierarchies array with its elements, as specified in the JsonPayload.

Additional Information: Please investigate why the repeated fields in arrays are not being serialized correctly and provide a fix or workaround.

A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. **

The proto3-json-serializer library is designed to serialize and deserialize protobuf.js objects according to the proto3 JSON specification. According to the documentation (https://cloud.google.com/nodejs/docs/reference/proto3-json-serializer/latest/overview), the library should handle repeated fields in arrays correctly.

Please find the example proto definitions in the comments section below.

@hardeep-qa
Copy link
Author

hardeep-qa commented Jan 10, 2025

Article.proto
image
image
image

ArticleHierarchy.proto
image

@sofisl
Copy link
Contributor

sofisl commented Jan 23, 2025

Has this been solved for you?

@sofisl sofisl added the needs more info This issue needs more information from the customer to proceed. label Jan 23, 2025
@sofisl sofisl reopened this Jan 23, 2025
@hardeep-qa
Copy link
Author

After some investigation, I found that the articleHierarchies field is case-sensitive. Using articleHierarchies (lowercase 'a') resulted in an empty object {}, while ArticleHierarchies (uppercase 'A') worked as expected. It appears the library isn't handling case sensitivity for this field correctly.

Here's a working example:

hierarchyData: {
     ArticleHierarchies: [
       {
         "hierarchyId": "Z1",
         "hierarchyNode": "1093895"
       },
       {
        "hierarchyId": "Z1",
        "hierarchyNode": "109389502"
        },
    ]
},

And here's a non-working example:

hierarchyData: {
     articleHierarchies: [
       {
         "hierarchyId": "Z1",
         "hierarchyNode": "1093895"
       },
       {
        "hierarchyId": "Z1",
        "hierarchyNode": "109389502"
        },
    ]
},

@github-actions github-actions bot removed the needs more info This issue needs more information from the customer to proceed. label Feb 1, 2025
@sofisl sofisl added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. size: m Pull request size is medium. labels Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants