You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by chtyim <gi...@git.apache.org> on 2016/08/24 22:54:32 UTC
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location to manipulation...
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
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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(i) - '0';
+ if (digit < 0 || digit > 7) {
+ throw new IllegalArgumentException("Invalid permission " + permission +
+ ". Only digits between 0-7 is allowed.");
--- End diff --
fixed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill issue #3: (TWILL-188) Added methods to Location for manipulation of pe...
Posted by albertshau <gi...@git.apache.org>.
Github user albertshau commented on the issue:
https://github.com/apache/twill/pull/3
only minor comments about javadocs, otherwise LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
Github user chtyim closed the pull request at:
https://github.com/apache/twill/pull/3
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by albertshau <gi...@git.apache.org>.
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(i) - '0';
+ if (digit < 0 || digit > 7) {
+ throw new IllegalArgumentException("Invalid permission " + permission +
+ ". Only digits between 0-7 is allowed.");
--- End diff --
is -> are
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] twill pull request #3: (TWILL-188) Added methods to Location for manipulatio...
Posted by chtyim <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---