[GitHub] nifi pull request #2056: NIFI-4262 - MergeContent - option to add merged uui...

[markap14 <gi...@git.apache.org>]

Github user markap14 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/2056#discussion_r160995049
  
    --- Diff: nifi-nar-bundles/nifi-extension-utils/nifi-processor-utils/src/main/java/org/apache/nifi/processor/util/bin/Bin.java ---
    @@ -173,4 +174,19 @@ public long getBinAge() {
             final long ageInNanos = System.nanoTime() - creationMomentEpochNs;
             return TimeUnit.MILLISECONDS.convert(ageInNanos, TimeUnit.NANOSECONDS);
         }
    +
    +    /**
    +     * @return the UUID of the flow file generated by the merge of this bin's content
    +     */
    +    public String getUuid() {
    +        return uuid;
    +    }
    +
    +    /**
    +     * Set the UUID of the flow file generated by the merge of this bin's content
    +     * @param uuid UUID to set
    +     */
    +    public void setUuid(String uuid) {
    --- End diff --
    
    After reviewing this again, I think we've got 2 approaches that I would recommend. My preferred approach would be to update the `boolean processBin(Bin, ProcessContext)` method to return a `BinProcessingResult` object, where `BinProcessingResult` would be a new class that has the boolean of whether or not the bin was committed and also has a Map<String, String> of attributes that should be added to the 'original' FlowFiles. This is more flexible and allows us to make other modifications more cleanly in the future. This is not considered part of our API but rather is just a convenience class, so we can change it in any minor version of NiFi.
    
    The second option would be to keep as-is, but at least just check if the bin uuid is null. If so, just ignore it instead of trying to add it to the FlowFiles. This option, though, I feel is a little less clean and requires that implementations know that this 'magical' UUID exists on the bin. It's also less flexible for future updates.


---