Skip to content

Commit 6735b37

Browse files
authored
feat: make generate enum use id for wire format (#3576)
## Why? ## What does this PR do? make generate enum use id for wire format - java - python - c++ - swift - javascript ## Related issues ## AI Contribution Checklist - [ ] Substantial AI assistance was used in this PR: `yes` / `no` - [ ] If `yes`, I included a completed [AI Contribution Checklist](https://github.com/apache/fory/blob/main/AI_POLICY.md#9-contributor-checklist-for-ai-assisted-prs) in this PR description and the required `AI Usage Disclosure`. - [ ] If `yes`, my PR description includes the required `ai_review` summary and screenshot evidence of the final clean AI review results from both fresh reviewers on the current PR diff or current HEAD after the latest code changes. ## Does this PR introduce any user-facing change? - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark
1 parent 4a3c71c commit 6735b37

16 files changed

Lines changed: 496 additions & 45 deletions

File tree

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
name: Publish JavaScript
19+
run-name: "JavaScript Release: ${{ github.ref_name }}"
20+
21+
on:
22+
push:
23+
tags: ['v*']
24+
25+
permissions:
26+
contents: read
27+
id-token: write
28+
29+
concurrency:
30+
group: release-javascript-${{ github.ref }}
31+
cancel-in-progress: false
32+
33+
jobs:
34+
publish-npm:
35+
runs-on: ubuntu-latest
36+
if: github.repository == 'apache/fory'
37+
steps:
38+
- uses: actions/checkout@v5
39+
40+
- uses: actions/setup-node@v4
41+
with:
42+
node-version: '20'
43+
registry-url: 'https://registry.npmjs.org'
44+
45+
- name: Upgrade npm for trusted publishing
46+
run: npm i -g npm@latest
47+
48+
- name: Verify Node/npm versions
49+
shell: bash
50+
run: |
51+
set -euo pipefail
52+
echo "Node: $(node --version)"
53+
echo "npm: $(npm --version)"
54+
REQUIRED="11.5.1"
55+
CURRENT="$(npm --version)"
56+
if [[ "$(printf '%s\n' "$REQUIRED" "$CURRENT" | sort -V | head -n1)" != "$REQUIRED" ]]; then
57+
echo "npm version must be >= $REQUIRED for trusted publishing. current=$CURRENT"
58+
exit 1
59+
fi
60+
61+
- name: Bump javascript version
62+
shell: bash
63+
run: |
64+
set -euo pipefail
65+
VERSION="${{ github.ref_name }}"
66+
VERSION="${VERSION#v}"
67+
python ci/release.py bump_version -l javascript -version "$VERSION"
68+
69+
- name: Install dependencies
70+
shell: bash
71+
working-directory: javascript
72+
run: npm install
73+
74+
- name: Build packages
75+
shell: bash
76+
working-directory: javascript
77+
run: npm run build
78+
79+
- name: Prepare npm for trusted publishing
80+
shell: bash
81+
run: |
82+
set -euo pipefail
83+
npm config set registry https://registry.npmjs.org/
84+
npm config delete //registry.npmjs.org/:_authToken || true
85+
npm config get registry
86+
87+
- name: Verify repository metadata for provenance
88+
shell: bash
89+
run: |
90+
set -euo pipefail
91+
node <<'EOF'
92+
const fs = require("node:fs");
93+
94+
const expected = "git@github.com:apache/fory.git";
95+
const files = [
96+
"javascript/packages/core/package.json",
97+
"javascript/packages/hps/package.json"
98+
];
99+
100+
for (const file of files) {
101+
const json = JSON.parse(fs.readFileSync(file, "utf8"));
102+
const actual = json?.repository ?? "";
103+
if (actual !== expected) {
104+
console.error(
105+
`Repository URL mismatch in ${file}: expected "${expected}", got "${actual}"`
106+
);
107+
process.exit(1);
108+
}
109+
}
110+
111+
console.log("repository metadata verified for all publishable packages");
112+
EOF
113+
114+
- name: Publish @apache-fory/hps
115+
shell: bash
116+
working-directory: javascript/packages/hps
117+
run: |
118+
set -euo pipefail
119+
TAG="latest"
120+
if echo "${{ github.ref_name }}" | grep -qE '[-]'; then
121+
TAG="next"
122+
fi
123+
npm publish --access public --tag "$TAG" --provenance
124+
env:
125+
NODE_AUTH_TOKEN: ""
126+
NPM_TOKEN: ""
127+
128+
- name: Publish @apache-fory/core
129+
shell: bash
130+
working-directory: javascript/packages/core
131+
run: |
132+
set -euo pipefail
133+
TAG="latest"
134+
if echo "${{ github.ref_name }}" | grep -qE '[-]'; then
135+
TAG="next"
136+
fi
137+
npm publish --access public --tag "$TAG" --provenance
138+
env:
139+
NODE_AUTH_TOKEN: ""
140+
NPM_TOKEN: ""

ci/release.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ def bump_version(**kwargs):
177177
bump_python_version(new_version)
178178
elif lang == "javascript":
179179
_bump_version(
180-
"javascript/packages/fory",
180+
"javascript/packages/core",
181181
"package.json",
182182
_normalize_js_version(new_version),
183183
_update_js_version,

compiler/fory_compiler/generators/java.py

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -366,18 +366,14 @@ def generate_enum_file(self, enum: Enum) -> GeneratedFile:
366366
lines.append(f"package {java_package};")
367367
lines.append("")
368368

369-
# Enum declaration
370-
lines.append(f"public enum {enum.name} {{")
371-
372-
# Enum values (strip prefix for scoped enums)
373-
for i, value in enumerate(enum.values):
374-
comma = "," if i < len(enum.values) - 1 else ";"
375-
stripped_name = self.strip_enum_prefix(enum.name, value.name)
376-
lines.append(f" {stripped_name}{comma}")
377-
378-
lines.append("}")
369+
lines.append("import org.apache.fory.annotation.ForyEnumId;")
379370
lines.append("")
380371

372+
# Enum declaration
373+
lines.extend(
374+
self.generate_enum_declaration(enum, f"public enum {enum.name} {{")
375+
)
376+
381377
# Build file path
382378
path = self.get_java_package_path()
383379
if path:
@@ -531,7 +527,7 @@ def generate_outer_class_file(self, outer_classname: str) -> GeneratedFile:
531527
for message in self.schema.messages:
532528
self.collect_message_imports(message, imports)
533529
for enum in self.schema.enums:
534-
pass # Enums don't need special imports
530+
self.collect_enum_imports(imports)
535531
for union in self.schema.unions:
536532
self.collect_union_imports(union, imports)
537533

@@ -604,12 +600,19 @@ def collect_message_imports(self, message: Message, imports: Set[str]):
604600
if self.has_array_field_recursive(message):
605601
imports.add("java.util.Arrays")
606602

603+
for nested_enum in message.nested_enums:
604+
self.collect_enum_imports(imports)
605+
607606
# Collect imports from nested messages
608607
for nested_msg in message.nested_messages:
609608
self.collect_message_imports(nested_msg, imports)
610609
for nested_union in message.nested_unions:
611610
self.collect_union_imports(nested_union, imports)
612611

612+
def collect_enum_imports(self, imports: Set[str]):
613+
"""Collect imports required by generated Java enums."""
614+
imports.add("org.apache.fory.annotation.ForyEnumId")
615+
613616
def collect_union_imports(self, union: Union, imports: Set[str]):
614617
"""Collect imports for a union and its cases."""
615618
imports.add("org.apache.fory.type.union.Union")
@@ -635,15 +638,30 @@ def has_array_field_recursive(self, message: Message) -> bool:
635638

636639
def generate_nested_enum(self, enum: Enum) -> List[str]:
637640
"""Generate a nested enum as a static inner class."""
638-
lines = []
639-
lines.append(f"public static enum {enum.name} {{")
641+
return self.generate_enum_declaration(
642+
enum, f"public static enum {enum.name} {{"
643+
)
644+
645+
def generate_enum_declaration(self, enum: Enum, header: str) -> List[str]:
646+
"""Generate a Java enum declaration with stable Fory enum ids."""
647+
lines = [header]
640648

641-
# Enum values (strip prefix for scoped enums)
642649
for i, value in enumerate(enum.values):
643650
comma = "," if i < len(enum.values) - 1 else ";"
644651
stripped_name = self.strip_enum_prefix(enum.name, value.name)
645-
lines.append(f" {stripped_name}{comma}")
652+
lines.append(f" {stripped_name}({value.value}){comma}")
646653

654+
lines.append("")
655+
lines.append(" private final int id;")
656+
lines.append("")
657+
lines.append(f" {enum.name}(int id) {{")
658+
lines.append(" this.id = id;")
659+
lines.append(" }")
660+
lines.append("")
661+
lines.append(" @ForyEnumId")
662+
lines.append(" public int getId() {")
663+
lines.append(" return id;")
664+
lines.append(" }")
647665
lines.append("}")
648666
lines.append("")
649667
return lines

compiler/fory_compiler/tests/test_generated_code.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,32 @@ def test_java_repeated_float16_generation_uses_float16_list():
543543
assert "private Float16List vals;" in java_output
544544

545545

546+
def test_java_enum_generation_uses_fory_enum_ids():
547+
schema = parse_fdl(
548+
dedent(
549+
"""
550+
package gen;
551+
552+
enum Status {
553+
UNKNOWN = 4096;
554+
OK = 8192;
555+
}
556+
"""
557+
)
558+
)
559+
java_output = render_files(generate_files(schema, JavaGenerator))
560+
assert "import org.apache.fory.annotation.ForyEnumId;" in java_output
561+
assert "public enum Status {" in java_output
562+
assert "UNKNOWN(4096)," in java_output
563+
assert "OK(8192);" in java_output
564+
assert "private final int id;" in java_output
565+
assert "Status(int id) {" in java_output
566+
assert "this.id = id;" in java_output
567+
assert "@ForyEnumId" in java_output
568+
assert "public int getId() {" in java_output
569+
assert "return id;" in java_output
570+
571+
546572
def test_go_bfloat16_generation():
547573
idl = dedent(
548574
"""

compiler/fory_compiler/tests/test_package_options.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,9 +494,15 @@ def test_outer_classname_generates_single_file(self):
494494
assert "myapp/DescriptorProtos.java" == outer_file.path
495495

496496
# Check content
497+
assert "import org.apache.fory.annotation.ForyEnumId;" in outer_file.content
497498
assert "public final class DescriptorProtos" in outer_file.content
498499
assert "private DescriptorProtos()" in outer_file.content
499500
assert "public static enum Status" in outer_file.content
501+
assert "UNKNOWN(0)," in outer_file.content
502+
assert "ACTIVE(1);" in outer_file.content
503+
assert "private final int id;" in outer_file.content
504+
assert "@ForyEnumId" in outer_file.content
505+
assert "public int getId()" in outer_file.content
500506
assert "public static class User" in outer_file.content
501507
assert "public static class Order" in outer_file.content
502508

@@ -606,6 +612,12 @@ def test_without_outer_classname_generates_separate_files(self):
606612
assert "Status.java" in file_names
607613
assert "User.java" in file_names
608614
assert "MyappForyRegistration.java" in file_names
615+
status_file = next(f for f in files if f.path.endswith("Status.java"))
616+
assert "import org.apache.fory.annotation.ForyEnumId;" in status_file.content
617+
assert "UNKNOWN(0);" in status_file.content
618+
assert "private final int id;" in status_file.content
619+
assert "@ForyEnumId" in status_file.content
620+
assert "public int getId()" in status_file.content
609621

610622

611623
class TestJavaMultipleFiles:

cpp/fory/serialization/enum_serializer.h

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,35 @@ namespace serialization {
3737

3838
/// Serializer specialization for enum types.
3939
///
40-
/// Writes the enum ordinal (underlying integral value) to match the xlang
41-
/// specification for value-based enums.
40+
/// Writes enum values using xlang-compatible uint32 wire ids. Registered enums
41+
/// with contiguous zero-based values keep ordinal encoding; enums with sparse
42+
/// non-negative declared values preserve those declared values on the wire.
4243
template <typename E>
4344
struct Serializer<E, std::enable_if_t<std::is_enum_v<E>>> {
4445
static constexpr TypeId type_id = TypeId::ENUM;
4546

4647
using Metadata = meta::EnumMetadata<E>;
4748
using OrdinalType = typename Metadata::OrdinalType;
49+
using EnumInfo = meta::EnumInfo<E>;
50+
51+
static constexpr bool supports_explicit_wire_values() {
52+
if constexpr (!EnumInfo::defined) {
53+
return true;
54+
}
55+
for (std::size_t i = 0; i < EnumInfo::size; ++i) {
56+
const auto raw = static_cast<OrdinalType>(EnumInfo::values[i]);
57+
if constexpr (std::is_signed_v<OrdinalType>) {
58+
if (raw < 0) {
59+
return false;
60+
}
61+
}
62+
using Unsigned = std::make_unsigned_t<OrdinalType>;
63+
if (static_cast<Unsigned>(raw) != static_cast<Unsigned>(i)) {
64+
return true;
65+
}
66+
}
67+
return false;
68+
}
4869

4970
static inline void write_type_info(WriteContext &ctx) {
5071
// Use compile-time type lookup for faster enum type info writing
@@ -72,6 +93,24 @@ struct Serializer<E, std::enable_if_t<std::is_enum_v<E>>> {
7293
}
7394

7495
static inline void write_data(E value, WriteContext &ctx) {
96+
if constexpr (supports_explicit_wire_values()) {
97+
if constexpr (EnumInfo::defined) {
98+
if (!EnumInfo::contains(value)) {
99+
ctx.set_error(Error::unknown_enum("Unknown enum value"));
100+
return;
101+
}
102+
}
103+
const auto raw_value = static_cast<OrdinalType>(value);
104+
if constexpr (std::is_signed_v<OrdinalType>) {
105+
if (raw_value < 0) {
106+
ctx.set_error(
107+
Error::unknown_enum("Negative enum values are not supported"));
108+
return;
109+
}
110+
}
111+
ctx.write_var_uint32(static_cast<uint32_t>(raw_value));
112+
return;
113+
}
75114
OrdinalType ordinal{};
76115
if (!Metadata::to_ordinal(value, &ordinal)) {
77116
ctx.set_error(Error::unknown_enum("Unknown enum value"));
@@ -111,6 +150,25 @@ struct Serializer<E, std::enable_if_t<std::is_enum_v<E>>> {
111150
if (FORY_PREDICT_FALSE(ctx.has_error())) {
112151
return E{};
113152
}
153+
if constexpr (supports_explicit_wire_values()) {
154+
if (raw_ordinal >
155+
static_cast<uint32_t>(std::numeric_limits<OrdinalType>::max())) {
156+
ctx.set_error(Error::unknown_enum(
157+
"Invalid enum value: " +
158+
std::to_string(static_cast<unsigned long long>(raw_ordinal))));
159+
return E{};
160+
}
161+
const auto value = static_cast<E>(static_cast<OrdinalType>(raw_ordinal));
162+
if constexpr (EnumInfo::defined) {
163+
if (!EnumInfo::contains(value)) {
164+
ctx.set_error(Error::unknown_enum(
165+
"Invalid enum value: " +
166+
std::to_string(static_cast<unsigned long long>(raw_ordinal))));
167+
return E{};
168+
}
169+
}
170+
return value;
171+
}
114172
OrdinalType ordinal = static_cast<OrdinalType>(raw_ordinal);
115173
E value{};
116174
if (!Metadata::from_ordinal(ordinal, &value)) {

0 commit comments

Comments
 (0)