NIFI-16049 Corrected if else logic in FlowFileUnpackagerV1

[dan-s1]

Summary

NIFI-16049

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
• Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using ./mvnw clean install -P contrib-check
  • JDK 21
  • JDK 25

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks @dan-s1, is this change something that can be easily validated with a unit test?

[dan-s1]

@exceptionfactory Let me see what I can do.

[dan-s1]

@exceptionfactory When writing a unit test, I came across an issue that I cannot write an XML file using the java.util.Properties storeToXML method to write attributes whose key or value are not strings. Furthermore in the FlowFilePackagerV1 code in method packageFlowFile it seems it only takes a map of attributes whose keys and values are only strings. In addition it seems the java.util.Properties loadFromXML used in FlowFileUnpackagerV1 will only load a Properties object whose keys and values are strings.
Hence I question, why there are these checks in FlowFileUnpackagerV1 in the first place? It would seem you cannot write anything but strings. Perhaps we should remove these checks entirely. I do have a unit test to test unpackaging in a normal scenario which I can contribute if you wish. Please advise.

[dan-s1]

@pvillard31 @exceptionfactory Per my earlier comments, I believe the way FlowFileUnpackagerV1 was trying to retrieve the attributes from the java.util.Properties object was incorrect. From reading the java.util.Properties javadocs, it seems once the java.util.Properties load its properties from an XML file, all its keys and values are strings and not any other object. Hence the previous logic which was checking the instance type was completely unnecessary. I have corrected this with my latest commit to use streams and the java.util.Properties stringPropertyNames and getProperty to build a map of the attributes. I have also added a unit test for testing the FlowFilePackagerV1 and the FlowFileUnpackagerV1.

[mosermw]

@pvillard31 @exceptionfactory Per my earlier comments, I believe the way FlowFileUnpackagerV1 was trying to retrieve the attributes from the java.util.Properties object was incorrect. From reading the java.util.Properties javadocs, it seems once the java.util.Properties load its properties from an XML file, all its keys and values are strings and not any other object. Hence the previous logic which was checking the instance type was completely unnecessary. I have corrected this with my latest commit to use streams and the java.util.Properties stringPropertyNames and getProperty to build a map of the attributes. I have also added a unit test for testing the FlowFilePackagerV1 and the FlowFileUnpackagerV1.

@dan-s1 I wouldn't worry about the FlowFileV1 format very much. It is extremely old and there are good reasons why it was replaced. Don't overthink it or waste time on it.