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



##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/FileInfo.java
##########
@@ -206,6 +207,32 @@ public Builder permissions(String permissions) {
             return this;
         }
 
+        public Builder permissions(int fileModeOctal) {
+            if(fileModeOctal > 0777 || fileModeOctal < 00) {
+                throw new RuntimeException("Invalid mode numeral");
+            }
+            final int NUM_SYSTEM = 8;
+            StringBuilder sb = new StringBuilder();
+            while(fileModeOctal > 0) {
+                int digit = fileModeOctal % NUM_SYSTEM;
+                final String[] perms = new String[]{"x", "w", "r"};
+                for (final String perm : perms) {
+                    if ((digit & 1) == 1) {
+                        sb.insert(0, perm);
+                    } else {
+                        sb.insert(0, "-");
+                    }
+                    digit >>= 1;
+                }
+                fileModeOctal /= NUM_SYSTEM;
+            }
+            if (sb.length() < 9) {
+                IntStream.range(0, 9 - sb.length()).forEach(i -> sb.insert(0, 
'-'));
+            }
+            this.permissions = sb.toString();
+            return this;
+        }

Review comment:
       I think this algorithm could be simplified a lot, similar to this (not 
tested thoroughly):
   ```
           char[] PERM = "xwrxwrxwr".toCharArray();
   
           StringBuilder sb = new StringBuilder();
           for (char p : PERM) {
               sb.append((fileModeOctal & 1) == 1 ? p : '-');
               fileModeOctal >>= 1;
           }
           this.permissions = sb.reverse().toString();
   ```

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -321,6 +337,16 @@ public void process(final InputStream in) throws 
IOException {
                                 
attributes.put(CoreAttributes.ABSOLUTE_PATH.key(), absPathString);
                                 attributes.put(CoreAttributes.MIME_TYPE.key(), 
OCTET_STREAM);
 
+                                {
+                                    FileInfo info = new 
FileInfo.Builder().permissions(tarEntry.getMode()).build();
+                                    attributes.put(FILE_PERMISSIONS_ATTRIBUTE, 
info.getPermissions());
+                                }

Review comment:
       Why are these lines in an anonymous code block?

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -110,6 +118,12 @@
 
     public static final String OCTET_STREAM = "application/octet-stream";
 
+    public static final String FILE_LAST_MODIFY_TIME_ATTRIBUTE = 
"file.lastModifiedTime";

Review comment:
       Typo: MODIFIED

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/UnpackContent.java
##########
@@ -298,6 +312,8 @@ public TarUnpacker(Pattern fileFilter) {
         public void unpack(final ProcessSession session, final FlowFile 
source, final List<FlowFile> unpacked) {
             final String fragmentId = UUID.randomUUID().toString();
             session.read(source, new InputStreamCallback() {
+                private final DateTimeFormatter dateTimeFormatter = 
DateTimeFormatter.ofPattern(FILE_MODIFY_DATE_ATTR_FORMAT).withZone(ZoneId.systemDefault());

Review comment:
       You can define the DateTimeFormatter instance in a constant as it is 
thread-safe. There's no need the create a new one for every FlowFile.

##########
File path: 
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/FileInfo.java
##########
@@ -206,6 +207,32 @@ public Builder permissions(String permissions) {
             return this;
         }
 
+        public Builder permissions(int fileModeOctal) {

Review comment:
       I would consider to define a separate utility method for this conversion 
instead of putting it into the builder.
   Creating a FileInfo object just to convert the permissions seems to me a bit 
strange.
   Furthermore, one might need the other way conversion (rwx => octal) in the 
future.




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to