Skip to content

Commit 339ebb0

Browse files
dmealingclaude
andcommitted
refactor(java): remove dead enum validation method, fix doc/cast cruft
Post-review cleanup of the enum @values validation-hardening change set. No behavior change; all 340 metadata tests pass. - EnumField: delete the orphaned static validateNodeAfterParse(MetaData, String). Its only caller (CanonicalJsonParser.processNode) was removed when validation moved to ValidationPhase, which now duplicates the logic. Drop the four imports it solely used (MetaData, MetaAttribute, MetaDataException, ErrorMessageConstants). Keep the still-referenced validateEnumValues helper. - EnumField: refresh class/registerTypes Javadoc that still described the validation hook as deferred; it now points at ValidationPhase. - ValidationPhase: reconcile contradictory ordering docs (class-level said 'after super resolution', inline comment said 'before') to one accurate framing — extends: super is resolved eagerly at parse, before this phase. - ValidationPhase: drop the needless @SuppressWarnings("unchecked") (raw MetaAttribute -> MetaAttribute<?> is a wildcard widening, not an unchecked cast — verified no warning on build) and the unreachable valuesAttr == null guard (getMetaAttr throws rather than returning null after hasMetaAttr). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5495f52 commit 339ebb0

2 files changed

Lines changed: 17 additions & 93 deletions

File tree

server/java/metadata/src/main/java/com/metaobjects/field/EnumField.java

Lines changed: 8 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,8 @@
77
package com.metaobjects.field;
88

99
import com.metaobjects.DataTypes;
10-
import com.metaobjects.MetaData;
11-
import com.metaobjects.MetaDataException;
12-
import com.metaobjects.attr.MetaAttribute;
1310
import com.metaobjects.attr.StringAttribute;
1411
import com.metaobjects.registry.MetaDataRegistry;
15-
import com.metaobjects.util.ErrorMessageConstants;
1612
import org.slf4j.Logger;
1713
import org.slf4j.LoggerFactory;
1814

@@ -32,9 +28,9 @@
3228
* member must match {@code ^[A-Za-z_][A-Za-z0-9_]*$}. These constraints mirror the
3329
* cross-language validation contract shared with TS and C# (conformance error code
3430
* {@code ERR_BAD_ATTR_VALUE}). The static {@link #validateEnumValues(Object)} method
35-
* enforces this contract; call it after loading to validate the content. A
36-
* load-time-enforcement hook is deferred (see the comment in
37-
* {@link #registerTypes(MetaDataRegistry)}).</p>
31+
* is the lower-level content helper; the loader's
32+
* {@link com.metaobjects.loader.ValidationPhase} invokes it in a post-load pass to
33+
* enforce this contract.</p>
3834
*
3935
* @version 6.0
4036
*/
@@ -81,10 +77,11 @@ public EnumField(String name) {
8177
* {@code attr.stringarray} type was removed; the pattern mirrors {@code identity.primary
8278
* @fields}). Per-element content validation (non-empty, identifier-safe members,
8379
* no duplicates — equivalent to cross-language {@code ERR_BAD_ATTR_VALUE}) is
84-
* available via the static {@link #validateEnumValues(Object)} method. Wiring it
85-
* as a load-time constraint is deferred because the constraint enforcer fires at
86-
* node-creation time (before {@code @values} is parsed), so enforcement would
87-
* require a dedicated post-parse validation pass.</p>
80+
* available via the static {@link #validateEnumValues(Object)} method. It is not
81+
* wired as a node-creation constraint because the constraint enforcer fires before
82+
* {@code @values} is parsed; instead the loader's
83+
* {@link com.metaobjects.loader.ValidationPhase} runs it as a dedicated post-load
84+
* validation pass.</p>
8885
*
8986
* @param registry The MetaDataRegistry to register with
9087
*/
@@ -113,78 +110,6 @@ public static void registerTypes(MetaDataRegistry registry) {
113110
}
114111
}
115112

116-
// -----------------------------------------------------------------------
117-
// Post-parse validation hook — called by CanonicalJsonParser after the
118-
// full field.enum node (attributes + children) has been built.
119-
// -----------------------------------------------------------------------
120-
121-
/**
122-
* Post-parse validation for a freshly-built {@code field.enum} node.
123-
*
124-
* <p>Enforces the cross-language {@code @values} contract at load time:</p>
125-
* <ol>
126-
* <li>Missing {@code @values} → throws with {@code ERR_MISSING_REQUIRED_ATTR}</li>
127-
* <li>Empty / non-identifier member / duplicate → throws with {@code ERR_BAD_ATTR_VALUE}</li>
128-
* </ol>
129-
*
130-
* <p>Called from {@code CanonicalJsonParser.processNode()} after the node's
131-
* attributes and children have been processed, so {@code @values} is already
132-
* set on the node. Mirrors the TS and C# loader validation that fires at the
133-
* equivalent point in those pipelines.</p>
134-
*
135-
* @param enumNode the {@code field.enum} node to validate; ignored if not
136-
* type=field / subtype=enum
137-
* @param filename the source filename — included in error messages
138-
* @throws MetaDataException with code {@code ERR_MISSING_REQUIRED_ATTR} if
139-
* {@code @values} is absent, or {@code ERR_BAD_ATTR_VALUE} if the
140-
* members fail the identifier / uniqueness contract
141-
*/
142-
public static void validateNodeAfterParse(MetaData enumNode, String filename) {
143-
if (enumNode == null) return;
144-
if (!TYPE_FIELD.equals(enumNode.getType()) || !SUBTYPE_ENUM.equals(enumNode.getSubType())) return;
145-
146-
// --- Content check (own @values only) ---
147-
// Validate OWN @values regardless of whether the node is abstract or concrete.
148-
// A node that merely inherits @values from a super has nothing to content-validate here.
149-
// This matches the TS (attr-schema-validate.ts) and C# (ValidationPasses.ValidateEnumValues)
150-
// own-only contracts.
151-
if (enumNode.hasMetaAttr(ATTR_VALUES, false)) {
152-
MetaAttribute<?> valuesAttr;
153-
try {
154-
@SuppressWarnings("unchecked")
155-
MetaAttribute<?> attr = (MetaAttribute<?>) enumNode.getMetaAttr(ATTR_VALUES, false);
156-
valuesAttr = attr;
157-
} catch (Exception e) {
158-
// hasMetaAttr(false) returned true above, so this should not occur in practice.
159-
throw new MetaDataException(
160-
ErrorMessageConstants.ERR_MISSING_REQUIRED_ATTR + ": field.enum '" + enumNode.getName()
161-
+ "' could not read own @values attribute in file [" + filename + "]", e);
162-
}
163-
if (!validateEnumValues(valuesAttr.getValue())) {
164-
throw new MetaDataException(
165-
ErrorMessageConstants.ERR_BAD_ATTR_VALUE + ": field.enum '" + enumNode.getName()
166-
+ "' @values must be a non-empty list of identifier-safe, unique members"
167-
+ " (e.g. [\"DRAFT\",\"PUBLISHED\"]) in file [" + filename + "]");
168-
}
169-
// Own @values present and valid — no need for the required check below.
170-
return;
171-
}
172-
173-
// --- Required check ---
174-
// The node has no own @values. It is valid ONLY if it has a super reference
175-
// (inheriting @values from the super, which is validated on its own node).
176-
// getSuperData() is non-null iff an "extends" was given AND the super was found
177-
// (if extends was given but not found, BaseMetaDataParser already threw).
178-
// This check is therefore load-order-independent — no dependency on getSuperData()
179-
// resolution state beyond the guarantee that createOrOverlayMetaData provides.
180-
if (enumNode.getSuperData() == null) {
181-
throw new MetaDataException(
182-
ErrorMessageConstants.ERR_MISSING_REQUIRED_ATTR + ": field.enum '" + enumNode.getName()
183-
+ "' is missing required @values attribute in file [" + filename + "]");
184-
}
185-
// Has a super — inherits @values from the super, which is validated on its own node. OK.
186-
}
187-
188113
// -----------------------------------------------------------------------
189114
// @values validation — cross-language contract
190115
// -----------------------------------------------------------------------

server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@
2626
* <p>In this phase only enum {@code @values} content validation is wired here. Other
2727
* validation passes will be migrated incrementally.</p>
2828
*
29-
* <p>Ordering: this phase runs <em>after</em> {@code extends:} super resolution, so
30-
* {@link MetaData#getSuperData()} is already set. The own-only validation contract
31-
* (validate the node's own attributes, not inherited ones) means we do not need
32-
* effective/resolved attribute access.</p>
29+
* <p>Ordering: {@code extends:} super resolution happens eagerly at parse time, so by
30+
* the time this phase runs {@link MetaData#getSuperData()} is already set. The own-only
31+
* validation contract (validate the node's own attributes, not inherited ones) means we
32+
* do not need effective/resolved attribute access.</p>
3333
*
3434
* @since 6.1.0
3535
*/
@@ -70,9 +70,9 @@ public static void run(MetaRoot root) {
7070
// A concrete field.enum with no own @values AND no super reference is flagged as
7171
// missing a required attribute → ERR_MISSING_REQUIRED_ATTR.
7272
//
73-
// This pass is safe to run before super resolution because it relies only on
74-
// getSuperData() for the required-check exemption, and getSuperData() is set by
75-
// the parser when a valid "extends" is found (before the validation phase runs).
73+
// This pass relies only on getSuperData() for the required-check exemption, and
74+
// getSuperData() is set eagerly by the parser when a valid "extends" is found
75+
// (at parse time, before this validation phase runs).
7676
// =========================================================================
7777

7878
/**
@@ -116,9 +116,8 @@ private static void validateEnumNode(MetaData node) {
116116

117117
// --- Own @values content check ---
118118
if (node.hasMetaAttr(EnumField.ATTR_VALUES, false)) {
119-
@SuppressWarnings("unchecked")
120-
MetaAttribute<?> valuesAttr = (MetaAttribute<?>) node.getMetaAttr(EnumField.ATTR_VALUES, false);
121-
if (valuesAttr == null || !EnumField.validateEnumValues(valuesAttr.getValue())) {
119+
MetaAttribute<?> valuesAttr = node.getMetaAttr(EnumField.ATTR_VALUES, false);
120+
if (!EnumField.validateEnumValues(valuesAttr.getValue())) {
122121
throw new MetaDataException(
123122
ErrorMessageConstants.ERR_BAD_ATTR_VALUE
124123
+ ": field.enum '" + node.getName()

0 commit comments

Comments
 (0)