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

[BUG] - .proto -> typescript -> encode format and API doesn't allow for correct field ids and incorrectly assumes auto incrementation. #1234

Open
ShanonJackson opened this issue Aug 22, 2024 · 8 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@ShanonJackson
Copy link

Typia Version: "@latest 6.9.0"
Expected behavior: "Some way to add correct field ids when using proto serialization"
Actual behavior: "Typia uses invalid field ids when using proto serialization"

I have the following .proto file

message TalentGeneFilter {
    string field1 = 1;
    string field2 = 2;
    string field3 = 4; 
}

Please note the missing '3'

I convert this to a typescript type.

{field1: string, field2: string, field3: string}

I encode that typescript type.

typia.encode<type>(data);

Please note that in the binary output, field3 should be marked as using field 4 but that information is "lossy" lost when you convert the .proto -> typescript

This causes a bug for me because a downstream gRPC service expects correct field ids on correct fields (as per spec).

is there any way in typia I can do this?

{field1: string & typia.Field<1>, field2: string & typia.Field<2>, field3: string & & typia.Field<4>}

Thanks and hope this makes sense.

@samchon samchon self-assigned this Aug 22, 2024
@samchon samchon added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels Aug 22, 2024
@samchon
Copy link
Owner

samchon commented Aug 22, 2024

Good suggestion.

To support the Field (or something better name) tag, I've to change the current intersection tag system enormously. In current typia, it will actually try to validate the tag value when a object type case as a optional property. Due to I've afaid of such enourmous change, I had thought about this feature, but could not developed it for a log time.

By the way, I have to do it now because actual protocol buffer feature user has come.

Here is my additonal question for detailed specification:

  1. How about union type? It adjusts the Field tag too?
  2. Also, if user omits the Field tag to some properties, how will you do? Consider it as a compiler error? Or automatically assign the property field index by inference?
  3. How about think any better and proper name than Flag?
{
  id: string;
  etc: (string & tags.Field<3>) | (number & tags.Field<5>) | (boolean & tags.Field<9>;
  additional: Something;
}

@ShanonJackson
Copy link
Author

@samchon

Thanks for getting back to me so fast.

1: Union type is fine.
2: I think error is best because if we default to "auto incrementing" it can create bugs that are hard to track down, and also rely on users to specify fields in the same order as they are in the spec in typescript.
3: tags.Field<5> looks fine to me.

@samchon
Copy link
Owner

samchon commented Aug 22, 2024

How about another word Radix? The name field is too general.

@samchon
Copy link
Owner

samchon commented Aug 22, 2024

About this feature, I may try at next week's Saturday (2024-08-31).

@ShanonJackson
Copy link
Author

Field Numbers are what they are referred to in the protobuf spec see:

https://protobuf.dev/programming-guides/proto3/#assigning

That as why I like the syntax tags.Field<number> as they will be intuitive for people already familiar with protobuf. Alternatively tags.FieldNumber<5> but I feel this may be too verbose.

@samchon
Copy link
Owner

samchon commented Aug 22, 2024

About the naming, as I can starts this ticket at two weeks later, let's consider it for a while.

I'm considering those cases:

  • Field
  • FieldNumber
  • Radix
  • Sequence
  • ByteOrder
  • Order

@samchon
Copy link
Owner

samchon commented Sep 6, 2024

Adding custom tag on object is not easy, and LLM funtion calling schema is urgent for me due to my business duty.

I'm sorry but wait for a week more please.

samchon added a commit that referenced this issue Oct 14, 2024
Prepare #1234: refactor metadata schema for `protobuf` sequencing.
samchon added a commit that referenced this issue Oct 17, 2024
Develop #1234: `Sequence<N>` tag and its validator on `ProtobufFactory`
samchon added a commit that referenced this issue Oct 19, 2024
Close #1234: completed `Sequence<N>` implementation for Protocol Buffer.
@samchon
Copy link
Owner

samchon commented Oct 19, 2024

npm i typia@next

https://github.com/samchon/typia/blob/v7.0/test/src/structures/ObjectSequenceProtobuf.ts

@ShanonJackson You can use Sequence<N> tag in the next version.

The next update would be formally published after enhancing the LLM schema, maybe 2 weeks later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
No open projects
Status: To do
Development

No branches or pull requests

2 participants