turcsanyip commented on a change in pull request #4370:
URL: https://github.com/apache/nifi/pull/4370#discussion_r450430244



##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -321,6 +328,15 @@ public void process(final InputStream in) throws 
IOException {
                                 
attributes.put(CoreAttributes.ABSOLUTE_PATH.key(), absPathString);
                                 attributes.put(CoreAttributes.MIME_TYPE.key(), 
OCTET_STREAM);
 
+                                attributes.put(FILE_INNER_PERMISSION, 
String.valueOf(tarEntry.getMode()));

Review comment:
       The permissions should be added in "rwx" format instead of an integer.

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -321,6 +328,15 @@ public void process(final InputStream in) throws 
IOException {
                                 
attributes.put(CoreAttributes.ABSOLUTE_PATH.key(), absPathString);
                                 attributes.put(CoreAttributes.MIME_TYPE.key(), 
OCTET_STREAM);
 
+                                attributes.put(FILE_INNER_PERMISSION, 
String.valueOf(tarEntry.getMode()));
+                                attributes.put(FILE_INNER_OWNER, 
String.valueOf(tarEntry.getUserName()));
+                                attributes.put(FILE_INNER_GROUP, 
String.valueOf(tarEntry.getGroupName()));
+
+                                String timePattern = "yyyy-MM-dd'T'HH:mm:ssZ";
+                                DateFormat df = new 
SimpleDateFormat(timePattern);

Review comment:
       Instead of `SimpleDateFormat`, `DateTimeFormatter` should be used in 
Java 8+ which is thread safe and can be defined as a constant.
   The built-in `DateTimeFormatter.ISO_OFFSET_DATE_TIME` instance seems to me 
the right format here.

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -110,6 +112,11 @@
 
     public static final String OCTET_STREAM = "application/octet-stream";
 
+    public static final String FILE_INNER_PERMISSION = "file.inner.permission";
+    public static final String FILE_INNER_OWNER = "file.inner.owner";
+    public static final String FILE_INNER_GROUP = "file.inner.group";
+    public static final String FILE_INNER_LAST_MODIFIED_TIME = 
"file.inner.lastModifiedTime";

Review comment:
       In my opinion the regular `file.*` attributes should be used and there's 
no need to introduce the "inner" ones.
   The properties of the source tar file will be preserved on the flowfile 
transferred to the `original` relationship and I don't think they are needed on 
the unpacked files.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to