Uploaded image for project: 'Apache Twill'
  1. Apache Twill
  2. TWILL-188

LocationFactory should have options to accept permission for create

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8.0
    • Component/s: None
    • Labels:
      None

      Description

      Currently LocationFactory and Location API doesn't have a way to specify the permission to use for creating the path. we need a way to specify the permission for create and other operations on Location

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user chtyim closed the pull request at:

          https://github.com/apache/twill/pull/3

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim closed the pull request at: https://github.com/apache/twill/pull/3
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174743

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.

          • * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
            */
            boolean createNew() throws IOException;

          /**
          + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          + * does not yet exist. The newly created file will have permission set based on the given permission settings.
          + *
          + * @param permission A permission string. It has to be either a three digits or a nine characters string.
          + * For the three digits string, it is similar to the UNIX permission numeric representation.
          — End diff –

          fixed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174743 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. * @throws IOException + * @throws IOException if error encountered during creationg of the file */ boolean createNew() throws IOException; /** + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name + * does not yet exist. The newly created file will have permission set based on the given permission settings. + * + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. — End diff – fixed.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174737

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true} if the file is successfully create, {@code false} otherwise.
            - * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
            */
            boolean createNew() throws IOException;

            /**
            + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
            + * does not yet exist. The newly created file will have permission set based on the given permission settings.
            + *
            + * @param permission A permission string. It has to be either a three digits or a nine characters string.
            + * For the three digits string, it is similar to the UNIX permission numeric representation.
            + * The first digit is the permission for owner, second digit is the permission for group and
            + * the third digit is the permission for all.
            + * For the nine characters string, it uses the format as specified by the
            + * {@link PosixFilePermissions#fromString(String)} method.
            + * @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.
            + * @throws IOException if error encountered during creationg of the file
            + */
            + boolean createNew(String permission) throws IOException;
            +
            + /**
            + * Returns the permissions of this

            {@link Location}

            . The permission string is a nine characters string as the format

              • End diff –

          fixed

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174737 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. - * @throws IOException + * @throws IOException if error encountered during creationg of the file */ boolean createNew() throws IOException; /** + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name + * does not yet exist. The newly created file will have permission set based on the given permission settings. + * + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. + * The first digit is the permission for owner, second digit is the permission for group and + * the third digit is the permission for all. + * For the nine characters string, it uses the format as specified by the + * {@link PosixFilePermissions#fromString(String)} method. + * @return {@code true} if the file is successfully create, {@code false} otherwise. + * @throws IOException if error encountered during creationg of the file + */ + boolean createNew(String permission) throws IOException; + + /** + * Returns the permissions of this {@link Location} . The permission string is a nine characters string as the format End diff – fixed
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174729

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -73,7 +105,12 @@

          • Creates an {@link OutputStream}

            for this location with the given permission. The actual permission supported

          • depends on implementation.
            *
          • * @param permission A POSIX permission string.
            + * @param permission A permission string. It has to be either a three digits or a nine characters string.
            + * For the three digits string, it is similar to the UNIX permission numeric representation.
              • End diff –

          fixed

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174729 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -73,7 +105,12 @@ Creates an {@link OutputStream} for this location with the given permission. The actual permission supported depends on implementation. * * @param permission A POSIX permission string. + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. End diff – fixed
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174588

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.

          • * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
              • End diff –

          fixed

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174588 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. * @throws IOException + * @throws IOException if error encountered during creationg of the file End diff – fixed
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174570

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java —
          @@ -255,4 +288,53 @@ public int hashCode() {
          public String toString()

          { return file.toString(); }

          +
          + /**
          + * Ensures the given

          {@link File}

          is a directory. If it doesn't exist, it will be created.
          + */
          + private void ensureDirectory(File dir) throws IOException {
          + // Check, create, check to resolve race conditions if there are concurrent creation of the directory.
          + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory())

          { + throw new IOException("Failed to create directory " + dir); + }

          + }
          +
          + /**
          + * Parses the given permission to a set of

          {@link PosixFilePermission}.
          + *
          + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)}
          + * methods.
          + * @return a new set of {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parsePermissions(String permission) {
          + Set<PosixFilePermission> permissions;
          + if (permission.length() == 3)

          { + permissions = parseNumericPermissions(permission); + }

          else if (permission.length() == 9)

          { + permissions = PosixFilePermissions.fromString(permission); + }

          else {
          + throw new IllegalArgumentException("Invalid permission " + permission +
          + ". Permission should either be a three digits or nine characters string.");
          — End diff –

          fixed

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174570 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java — @@ -255,4 +288,53 @@ public int hashCode() { public String toString() { return file.toString(); } + + /** + * Ensures the given {@link File} is a directory. If it doesn't exist, it will be created. + */ + private void ensureDirectory(File dir) throws IOException { + // Check, create, check to resolve race conditions if there are concurrent creation of the directory. + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory()) { + throw new IOException("Failed to create directory " + dir); + } + } + + /** + * Parses the given permission to a set of {@link PosixFilePermission}. + * + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)} + * methods. + * @return a new set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parsePermissions(String permission) { + Set<PosixFilePermission> permissions; + if (permission.length() == 3) { + permissions = parseNumericPermissions(permission); + } else if (permission.length() == 9) { + permissions = PosixFilePermissions.fromString(permission); + } else { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); — End diff – fixed
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76174543

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java —
          @@ -255,4 +288,53 @@ public int hashCode() {
          public String toString()

          { return file.toString(); }

          +
          + /**
          + * Ensures the given

          {@link File}

          is a directory. If it doesn't exist, it will be created.
          + */
          + private void ensureDirectory(File dir) throws IOException {
          + // Check, create, check to resolve race conditions if there are concurrent creation of the directory.
          + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory())

          { + throw new IOException("Failed to create directory " + dir); + }

          + }
          +
          + /**
          + * Parses the given permission to a set of

          {@link PosixFilePermission}.
          + *
          + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)}
          + * methods.
          + * @return a new set of {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parsePermissions(String permission) {
          + Set<PosixFilePermission> permissions;
          + if (permission.length() == 3)

          { + permissions = parseNumericPermissions(permission); + }

          else if (permission.length() == 9)

          { + permissions = PosixFilePermissions.fromString(permission); + }

          else

          { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); + }

          +
          + return permissions;
          + }
          +
          + /**
          + * Parses a three digits UNIX numeric permission representation to a set of

          {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parseNumericPermissions(String permission) {
          + String posixPermission = "";
          + for (int i = 0; i < 3; i++) {
          + int digit = permission.charAt - '0';
          + if (digit < 0 || digit > 7) {
          + throw new IllegalArgumentException("Invalid permission " + permission +
          + ". Only digits between 0-7 is allowed.");
          — End diff –

          fixed

          Show
          githubbot ASF GitHub Bot added a comment - Github user chtyim commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76174543 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java — @@ -255,4 +288,53 @@ public int hashCode() { public String toString() { return file.toString(); } + + /** + * Ensures the given {@link File} is a directory. If it doesn't exist, it will be created. + */ + private void ensureDirectory(File dir) throws IOException { + // Check, create, check to resolve race conditions if there are concurrent creation of the directory. + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory()) { + throw new IOException("Failed to create directory " + dir); + } + } + + /** + * Parses the given permission to a set of {@link PosixFilePermission}. + * + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)} + * methods. + * @return a new set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parsePermissions(String permission) { + Set<PosixFilePermission> permissions; + if (permission.length() == 3) { + permissions = parseNumericPermissions(permission); + } else if (permission.length() == 9) { + permissions = PosixFilePermissions.fromString(permission); + } else { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); + } + + return permissions; + } + + /** + * Parses a three digits UNIX numeric permission representation to a set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parseNumericPermissions(String permission) { + String posixPermission = ""; + for (int i = 0; i < 3; i++) { + int digit = permission.charAt - '0'; + if (digit < 0 || digit > 7) { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Only digits between 0-7 is allowed."); — End diff – fixed
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user albertshau commented on the issue:

          https://github.com/apache/twill/pull/3

          only minor comments about javadocs, otherwise LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on the issue: https://github.com/apache/twill/pull/3 only minor comments about javadocs, otherwise LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164643

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -73,7 +105,12 @@

          • Creates an {@link OutputStream}

            for this location with the given permission. The actual permission supported

          • depends on implementation.
            *
          • * @param permission A POSIX permission string.
            + * @param permission A permission string. It has to be either a three digits or a nine characters string.
            + * For the three digits string, it is similar to the UNIX permission numeric representation.
              • End diff –

          three digit, nine character

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164643 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -73,7 +105,12 @@ Creates an {@link OutputStream} for this location with the given permission. The actual permission supported depends on implementation. * * @param permission A POSIX permission string. + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. End diff – three digit, nine character
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164629

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true} if the file is successfully create, {@code false} otherwise.
            - * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
            */
            boolean createNew() throws IOException;

            /**
            + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
            + * does not yet exist. The newly created file will have permission set based on the given permission settings.
            + *
            + * @param permission A permission string. It has to be either a three digits or a nine characters string.
            + * For the three digits string, it is similar to the UNIX permission numeric representation.
            + * The first digit is the permission for owner, second digit is the permission for group and
            + * the third digit is the permission for all.
            + * For the nine characters string, it uses the format as specified by the
            + * {@link PosixFilePermissions#fromString(String)} method.
            + * @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.
            + * @throws IOException if error encountered during creationg of the file
            + */
            + boolean createNew(String permission) throws IOException;
            +
            + /**
            + * Returns the permissions of this

            {@link Location}

            . The permission string is a nine characters string as the format

              • End diff –

          nine characters -> nine character

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164629 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. - * @throws IOException + * @throws IOException if error encountered during creationg of the file */ boolean createNew() throws IOException; /** + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name + * does not yet exist. The newly created file will have permission set based on the given permission settings. + * + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. + * The first digit is the permission for owner, second digit is the permission for group and + * the third digit is the permission for all. + * For the nine characters string, it uses the format as specified by the + * {@link PosixFilePermissions#fromString(String)} method. + * @return {@code true} if the file is successfully create, {@code false} otherwise. + * @throws IOException if error encountered during creationg of the file + */ + boolean createNew(String permission) throws IOException; + + /** + * Returns the permissions of this {@link Location} . The permission string is a nine characters string as the format End diff – nine characters -> nine character
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164602

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.

          • * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
            */
            boolean createNew() throws IOException;

          /**
          + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          + * does not yet exist. The newly created file will have permission set based on the given permission settings.
          + *
          + * @param permission A permission string. It has to be either a three digits or a nine characters string.
          + * For the three digits string, it is similar to the UNIX permission numeric representation.
          — End diff –

          three digits -> three digit

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164602 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. * @throws IOException + * @throws IOException if error encountered during creationg of the file */ boolean createNew() throws IOException; /** + * Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name + * does not yet exist. The newly created file will have permission set based on the given permission settings. + * + * @param permission A permission string. It has to be either a three digits or a nine characters string. + * For the three digits string, it is similar to the UNIX permission numeric representation. — End diff – three digits -> three digit
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164566

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java —
          @@ -53,11 +54,42 @@

          • Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name
          • does not yet exist.
          • @return {@code true}

            if the file is successfully create,

            {@code false}

            otherwise.

          • * @throws IOException
            + * @throws IOException if error encountered during creationg of the file
              • End diff –

          typo: creation

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164566 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/Location.java — @@ -53,11 +54,42 @@ Atomically creates a new, empty file named by this abstract pathname if and only if a file with this name does not yet exist. @return {@code true} if the file is successfully create, {@code false} otherwise. * @throws IOException + * @throws IOException if error encountered during creationg of the file End diff – typo: creation
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164526

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java —
          @@ -255,4 +288,53 @@ public int hashCode() {
          public String toString()

          { return file.toString(); }

          +
          + /**
          + * Ensures the given

          {@link File}

          is a directory. If it doesn't exist, it will be created.
          + */
          + private void ensureDirectory(File dir) throws IOException {
          + // Check, create, check to resolve race conditions if there are concurrent creation of the directory.
          + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory())

          { + throw new IOException("Failed to create directory " + dir); + }

          + }
          +
          + /**
          + * Parses the given permission to a set of

          {@link PosixFilePermission}.
          + *
          + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)}
          + * methods.
          + * @return a new set of {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parsePermissions(String permission) {
          + Set<PosixFilePermission> permissions;
          + if (permission.length() == 3)

          { + permissions = parseNumericPermissions(permission); + }

          else if (permission.length() == 9)

          { + permissions = PosixFilePermissions.fromString(permission); + }

          else {
          + throw new IllegalArgumentException("Invalid permission " + permission +
          + ". Permission should either be a three digits or nine characters string.");
          — End diff –

          digits -> digit

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164526 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java — @@ -255,4 +288,53 @@ public int hashCode() { public String toString() { return file.toString(); } + + /** + * Ensures the given {@link File} is a directory. If it doesn't exist, it will be created. + */ + private void ensureDirectory(File dir) throws IOException { + // Check, create, check to resolve race conditions if there are concurrent creation of the directory. + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory()) { + throw new IOException("Failed to create directory " + dir); + } + } + + /** + * Parses the given permission to a set of {@link PosixFilePermission}. + * + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)} + * methods. + * @return a new set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parsePermissions(String permission) { + Set<PosixFilePermission> permissions; + if (permission.length() == 3) { + permissions = parseNumericPermissions(permission); + } else if (permission.length() == 9) { + permissions = PosixFilePermissions.fromString(permission); + } else { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); — End diff – digits -> digit
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/twill/pull/3#discussion_r76164518

          — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java —
          @@ -255,4 +288,53 @@ public int hashCode() {
          public String toString()

          { return file.toString(); }

          +
          + /**
          + * Ensures the given

          {@link File}

          is a directory. If it doesn't exist, it will be created.
          + */
          + private void ensureDirectory(File dir) throws IOException {
          + // Check, create, check to resolve race conditions if there are concurrent creation of the directory.
          + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory())

          { + throw new IOException("Failed to create directory " + dir); + }

          + }
          +
          + /**
          + * Parses the given permission to a set of

          {@link PosixFilePermission}.
          + *
          + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)}
          + * methods.
          + * @return a new set of {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parsePermissions(String permission) {
          + Set<PosixFilePermission> permissions;
          + if (permission.length() == 3)

          { + permissions = parseNumericPermissions(permission); + }

          else if (permission.length() == 9)

          { + permissions = PosixFilePermissions.fromString(permission); + }

          else

          { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); + }

          +
          + return permissions;
          + }
          +
          + /**
          + * Parses a three digits UNIX numeric permission representation to a set of

          {@link PosixFilePermission}

          .
          + */
          + private Set<PosixFilePermission> parseNumericPermissions(String permission) {
          + String posixPermission = "";
          + for (int i = 0; i < 3; i++) {
          + int digit = permission.charAt - '0';
          + if (digit < 0 || digit > 7) {
          + throw new IllegalArgumentException("Invalid permission " + permission +
          + ". Only digits between 0-7 is allowed.");
          — End diff –

          is -> are

          Show
          githubbot ASF GitHub Bot added a comment - Github user albertshau commented on a diff in the pull request: https://github.com/apache/twill/pull/3#discussion_r76164518 — Diff: twill-common/src/main/java/org/apache/twill/filesystem/LocalLocation.java — @@ -255,4 +288,53 @@ public int hashCode() { public String toString() { return file.toString(); } + + /** + * Ensures the given {@link File} is a directory. If it doesn't exist, it will be created. + */ + private void ensureDirectory(File dir) throws IOException { + // Check, create, check to resolve race conditions if there are concurrent creation of the directory. + if (!dir.isDirectory() && !dir.mkdirs() && !dir.isDirectory()) { + throw new IOException("Failed to create directory " + dir); + } + } + + /** + * Parses the given permission to a set of {@link PosixFilePermission}. + * + * @param permission the permission as passed to the {@link #createNew(String)} or {@link #getOutputStream(String)} + * methods. + * @return a new set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parsePermissions(String permission) { + Set<PosixFilePermission> permissions; + if (permission.length() == 3) { + permissions = parseNumericPermissions(permission); + } else if (permission.length() == 9) { + permissions = PosixFilePermissions.fromString(permission); + } else { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Permission should either be a three digits or nine characters string."); + } + + return permissions; + } + + /** + * Parses a three digits UNIX numeric permission representation to a set of {@link PosixFilePermission} . + */ + private Set<PosixFilePermission> parseNumericPermissions(String permission) { + String posixPermission = ""; + for (int i = 0; i < 3; i++) { + int digit = permission.charAt - '0'; + if (digit < 0 || digit > 7) { + throw new IllegalArgumentException("Invalid permission " + permission + + ". Only digits between 0-7 is allowed."); — End diff – is -> are
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user chtyim opened a pull request:

          https://github.com/apache/twill/pull/3

          (TWILL-188) Added methods to Location to manipulation of permission

          • Three methods are added
            void createNew(String permission)
            String getPermissions()
            void setPermissions(String permission)

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/chtyim/twill twill-188

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/twill/pull/3.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #3



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user chtyim opened a pull request: https://github.com/apache/twill/pull/3 ( TWILL-188 ) Added methods to Location to manipulation of permission Three methods are added void createNew(String permission) String getPermissions() void setPermissions(String permission) You can merge this pull request into a Git repository by running: $ git pull https://github.com/chtyim/twill twill-188 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/twill/pull/3.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3
          Hide
          chtyim Terence Yim added a comment -

          Currently there is a Location.getOutputStream(String permission) method for creation with permission. We can add a new createNew(String permission) method so that one can create the file atomically with the permission set.

          Show
          chtyim Terence Yim added a comment - Currently there is a Location.getOutputStream(String permission) method for creation with permission. We can add a new createNew(String permission) method so that one can create the file atomically with the permission set.

            People

            • Assignee:
              chtyim Terence Yim
              Reporter:
              gsps1 Shankar Selvam
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development