mattcasters commented on code in PR #9078:
URL: https://github.com/apache/hop/pull/9078#discussion_r4348157923
##########
plugins/transforms/metainject/src/main/java/org/apache/hop/pipeline/transforms/metainject/MetaInject.java:
##########
@@ -622,7 +649,49 @@ static void collectInjectionKeyValue(
//
IValueMeta valueMeta = sourceRow.getRowMeta().getValueMeta(index);
Object valueData = valueMeta.getNativeDataType(sourceRow.getData()[index]);
- injectionKeyData.put(mapping.getTargetAttributeKey(), valueData);
+ injectionKeyData.put(resolveInjectionKey(mapping),
resolveInjectionValue(mapping, valueData));
+ }
+
+ /**
+ * Returns the injection key under which the mapped value must be stored.
The legacy {@code
+ * ALLOW_EMPTY_STREAM_ON_EXECUTION} target key, used by metadata-injection
mappings written before
+ * {@link ConditionStreamBehavior} existed, is translated to its modern
{@code
+ * CONDITION_STREAM_BEHAVIOR} counterpart. Any other key is returned
unchanged.
+ *
+ * @param mapping the mapping being injected
+ * @return the normalized injection (target attribute) key
+ */
+ private static String resolveInjectionKey(MetaInjectMapping mapping) {
+ if
(LEGACY_ALLOW_EMPTY_STREAM_INJECTION_KEY.equals(mapping.getTargetAttributeKey()))
{
Review Comment:
**[bug]** This rewrite runs only inside `collectInjectionKeyValue`. Other
lookups still see the raw `ALLOW_EMPTY_STREAM_ON_EXECUTION` key, so an existing
mapping is not actually kept.
- Opening the dialog calls `getUnavailableTargetKeys`, which asks
`BeanInjectionInfo` for that key. It is no longer a `@HopMetadataProperty`
injection key, so the mapping is reported as missing. `checkInvalidMapping`
then asks to remove it, and Yes + OK deletes a mapping that would have run. The
Inject Metadata tree also fails to show the source, because
`findMappingForTargetKey` compares only against `CONDITION_STREAM_BEHAVIOR`.
- A mapping stored with `target_detail=Y` never reaches this method.
`collectDataForOneMapping` calls `HopMetadataInjector.isTopLevelInjectionKey`
with the legacy key, gets false, logs `TargetKeyNotFound`, and leaves the
template on `ALL_NON_EMPTY`. That method already treats `target_detail=Y` on a
scalar key as a simple property (older pipelines do store scalar keys that way).
**Suggestion:** Resolve `ALLOW_EMPTY_STREAM_ON_EXECUTION` to
`CONDITION_STREAM_BEHAVIOR` before `lookupGroupKey`, `isTopLevelInjectionKey`,
and `getUnavailableTargetKeys`. In the tree, match a saved legacy key onto the
`CONDITION_STREAM_BEHAVIOR` row so the source stays visible and the dialog does
not offer to delete it.
##########
docs/hop-dev-manual/modules/ROOT/pages/metadata-serialization.adoc:
##########
@@ -272,6 +272,33 @@ This will serialize `Apache Hop` to:
----
====
+== Legacy and renamed keys
+
+When a property is renamed or a field changes type, existing serialized files
still carry the old key. To keep reading them you can mark a method with
`@OnUnknownDeserializationProperty`. The serializers invoke it once for every
property encountered during deserialization that does not map to any
`@HopMetadataProperty` field.
+
+This is the recommended way to translate a legacy or renamed key into a
current field. It works for both XML and JSON serialization.
+
+From `MetaInjectMeta.java`, where the legacy boolean
`allow_empty_stream_on_execution` was replaced by the `ConditionStreamBehavior`
enum:
+
+[source,java]
+----
+@OnUnknownDeserializationProperty
+public void onUnknownDeserializationProperty(UnknownProperty property) {
+ if ("allow_empty_stream_on_execution".equals(property.getName())) {
Review Comment:
**[suggestion]** This snippet is introduced as the code from
`MetaInjectMeta`, but it always assigns `conditionStreamBehavior` when
`allow_empty_stream_on_execution` is present. The class ignores that tag once
`conditionStreamBehaviorSet` is true, which is what lets a file that also
contains `condition_stream_behavior` still represent `always`. Copying this
example puts the legacy boolean back in charge.
**Suggestion:** Show the `conditionStreamBehaviorSet` guard from
`MetaInjectMeta.onUnknownDeserializationProperty`, including that the legacy
value is written only when the current tag was absent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
