Skip to content

Commit 6901eaf

Browse files
committed
Resolve metadata module TODOs with architectural improvements
CRITICAL BUG FIX: - Fix DATE field serialization bug in MetaObjectSerializer - Was serializing entire value object instead of field value - Changed from context.serialize(vo) to context.serialize(mf.getObject(vo)) - Ensures correct DATE serialization for cross-language compatibility ARCHITECTURAL IMPROVEMENTS: - Refactor deleteOnAdd() to use polymorphism instead of instanceof checks - Add shouldDeleteOnAdd() method to MetaData base class (returns false) - Override in MetaAttribute to return true (enables replacement behavior) - Improves extensibility and follows open/closed principle DOCUMENTATION ENHANCEMENTS: - Improve shouldUseParentPackage() documentation in BaseMetaDataParser - Add comprehensive Javadoc explaining package inheritance rules - Clarify when MetaDataLoader parents don't propagate packages - Clarify URI validation limitation in URIHelper - Explain unprefixed classpath resources validated at load time CODE CLEANUP: - Remove obsolete LoaderOptions TODO - Class already supports extension via generic setter pattern - Fix typo: "somethinf" → removed with DATE serialization fix All 250 metadata module tests passing. Aligned with AI-optimized type system for Java/C#/TypeScript cross-language compatibility. https://claude.ai/code/session_011CUZhLswdQq92FRSrNd3tR
1 parent 8429f0e commit 6901eaf

6 files changed

Lines changed: 40 additions & 15 deletions

File tree

metadata/src/main/java/com/metaobjects/MetaData.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -926,18 +926,22 @@ protected boolean filterWhenParentData( MetaData d ) {
926926
}
927927

928928
/**
929-
* Whether to delete the MetaData if a new one is added
929+
* Determines if this MetaData instance should be deleted when a new one with the same name is added.
930+
* Subclasses can override this to control replacement behavior.
931+
*
932+
* @return true if this instance should be replaced, false to keep existing
933+
*/
934+
protected boolean shouldDeleteOnAdd() {
935+
return false; // Most MetaData types retain existing instances by default
936+
}
937+
938+
/**
939+
* Whether to delete the MetaData if a new one is added (delegates to polymorphic method)
930940
* @param d MetaData to check
931941
* @return true if should delete
932942
*/
933943
protected boolean deleteOnAdd( MetaData d) {
934-
935-
// TODO: Change these rules to be driven from a MetaData method that is overrideable
936-
937-
return d instanceof MetaAttribute;
938-
// || d instanceof MetaField
939-
//|| d instanceof MetaValidator
940-
//|| d instanceof MetaView;
944+
return d.shouldDeleteOnAdd();
941945
}
942946

943947
/**

metadata/src/main/java/com/metaobjects/attr/MetaAttribute.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,17 @@ public boolean isArrayType() {
106106
Boolean.parseBoolean(getMetaAttr(ATTR_IS_ARRAY).getValueAsString());
107107
}
108108

109+
/**
110+
* Attributes support replacement when a new attribute with the same name is added.
111+
* This allows attribute values to be updated/overridden in metadata hierarchies.
112+
*
113+
* @return true - attributes are replaceable by default
114+
*/
115+
@Override
116+
protected boolean shouldDeleteOnAdd() {
117+
return true;
118+
}
119+
109120
// ========== ENHANCED ATTRIBUTE-SPECIFIC METHODS ==========
110121

111122

metadata/src/main/java/com/metaobjects/io/object/gson/MetaObjectSerializer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ protected void writeField(MetaObject mo, MetaField mf, Object vo,
7373
jsonObject.addProperty(name, mf.getInt(vo));
7474
break;
7575

76-
case DATE: // TODO: should we do somethinf custom here?
77-
jsonObject.add(name, context.serialize(vo));
76+
case DATE:
77+
jsonObject.add(name, context.serialize(mf.getObject(vo)));
7878
break;
7979

8080
case LONG:

metadata/src/main/java/com/metaobjects/loader/MetaDataLoader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public class MetaDataLoader extends MetaData implements LoaderConfigurable {
107107
}
108108
}
109109

110-
// TODO: Allow for custom configurations for overloaded MetaDataLoaders
110+
// LoaderOptions supports extension via subclassing with generic setter pattern
111111
private final LoaderOptions loaderOptions;
112112

113113
// v6.0.0: Replace TypesConfig with unified registry

metadata/src/main/java/com/metaobjects/loader/parser/BaseMetaDataParser.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,9 +414,19 @@ protected String getFullyQualifiedSuperMetaDataName(MetaData parent, String pack
414414
return MetaDataUtil.expandPackageForMetaDataRef(basePackage, superName);
415415
}
416416

417-
/** Determine if the packageName should change based on the parent metadata */
417+
/**
418+
* Determines if child metadata should inherit parent's package.
419+
* Package inheritance is disabled when:
420+
* - No parent exists
421+
* - Parent is a MetaDataLoader (loaders don't propagate packages to loaded metadata)
422+
* - Parent has no package
423+
* - Parent package matches child package (already aligned)
424+
*
425+
* @param parent Parent metadata that may provide a package
426+
* @param packageName Child's current package name
427+
* @return true if parent package should override child package
428+
*/
418429
protected boolean shouldUseParentPackage( MetaData parent, String packageName ) {
419-
// TODO: This may need to be refactored
420430
return parent != null
421431
&& !(parent instanceof MetaDataLoader)
422432
&& !parent.getPackage().isEmpty()

metadata/src/main/java/com/metaobjects/loader/uri/URIHelper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ else if ( uriSourceType.equals( URI_SOURCE_RESOURCE )) {
122122
}
123123
}
124124
else {
125-
//new URL( url);
126-
// TODO: Not much you can do to check this
125+
// Unprefixed classpath resources cannot be validated without classloader context.
126+
// Validation occurs at load time when the resource is actually resolved.
127127
}
128128
} catch (MalformedURLException e) {
129129
throw new IllegalArgumentException( "Classpath resource has invalid syntax ["+url+"] "+e.toString(), e );

0 commit comments

Comments
 (0)