[slachiewicz]

Jira: THRIFT-735

A required union field passed validate() when the union object was present but had no field set, for example after an unknown member was skipped on read. The Java generator now also checks isSet() on required fields whose true type is a union, so such a struct fails with TProtocolException like an unset required field. Writing an unset union was already refused (TUnion.write()); only validation was missing.

This makes validate() stricter for that one case, so it is worth a line in the release notes.

Verified: gradle test --tests '*TestRequiredUnionValidation' → master compiler 2 of 5 fail, this branch 5 of 5 pass; TestTUnion, TestTDeserializer and the recursion-limit tests pass.

• Did you create an Apache Jira ticket?
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-735: describe my issue"?
• Did you squash your changes to a single commit?
• Did you do your best to avoid breaking changes? Only a required union with no field set is newly rejected.

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the required-union validation change.

No open issues 🎉

What changed in this PR

Updates Java code generation so required union fields fail validation when the union object exists but has no member set. This also covers typedef aliases while preserving validation behavior for optional unions.

Changes:

• Adds isSet() checks for required union fields in generated Java validators.
• Adds five regression tests covering populated, missing, unset, aliased, and optional unions.
• Registers the new Thrift test schema with Gradle.

File	Description
lib/​java/​src/​test/​resources/​JavaRequiredUnion.thrift	Defines required, aliased, and optional union test fields.
lib/​java/​src/​test/​java/​org/​apache/​thrift/​TestRequiredUnionValidation.java	Tests required and optional union validation behavior.
lib/​java/​gradle/​generateTestThrift.gradle	Registers the new schema for Java test generation.
compiler/​cpp/​src/​thrift/​generate/​t_java_generator.cc	Generates checks that reject unset required unions.

_{Review effort: Balanced}

[Jens-G]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Two suggestions, below the bar for an issue but verified by reading:

• No test exercises the beans branch of the change (if (!isSetX() || !x.isSet())), although the commit message names both styles. JavaRequiredUnion.thrift is compiled only by generateJava with the default options, and the one beans compile, JavaBeansTest.thrift, has no union.

thrift/lib/java/gradle/generateTestThrift.gradle

Lines 93 to 95 in 79556a5

	thriftCompile(it, 'FuzzTest.thrift')
	thriftCompile(it, 'JavaRequiredUnion.thrift')
	}

• The new check also runs in read(), which ends with struct.validate(), not only when an application calls validate(). Take a struct whose required union holds a member that the reader's IDL does not know. The union reader skips the unknown field and leaves setField_ unset. Before, the struct was accepted; generated code now throws TProtocolException from read(). That follows from THRIFT-735, but it is a visible change for a reader that is older than its writer, so a test through read() and a line in the release notes may be worth it.

thrift/compiler/cpp/src/thrift/generate/t_java_generator.cc

Lines 2232 to 2246 in 79556a5

	bool is_union = req_type->is_struct() && ((t_struct*)req_type)->is_union();
	string union_unset
	= is_union ? " || !" + make_valid_java_identifier((*f_iter)->get_name()) + ".isSet()"
	: "";
	if (bean_style_) {
	out << indent() << "if (!" << generate_isset_check(*f_iter) << union_unset << ") {" << '\n'
	<< indent()
	<< " throw new org.apache.thrift.protocol.TProtocolException(\"Required field '"
	<< (*f_iter)->get_name() << "' is unset! Struct:\" + toString());" << '\n'
	<< indent() << "}" << '\n'
	<< '\n';
	} else {
	if (type_can_be_null((*f_iter)->get_type())) {
	indent(out) << "if (" << (*f_iter)->get_name() << " == null" << union_unset << ") {"
	<< '\n';

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.