A couple attribute-related changes:

- Throw UOE instead of IAE when attempting to set an attribute that can't be set at all during file creation. This matches the behavior of the real file system implementations.
- Make the owner:owner attribute not settable during file creation, which also matches the actual behavior.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=117965277

8 files changed, 16 insertions, 20 deletions

diff --git a/jimfs/src/main/java/com/google/common/jimfs/AttributeProvider.java b/jimfs/src/main/java/com/google/common/jimfs/AttributeProvider.java
index c509467..cf248b1 100644
--- a/jimfs/src/main/java/com/google/common/jimfs/AttributeProvider.java
+++ b/jimfs/src/main/java/com/google/common/jimfs/AttributeProvider.java
@@ -138,9 +138,13 @@ public abstract class AttributeProvider {
   // exception helpers
 
   /**
-   * Throws an illegal argument exception indicating that the given attribute cannot be set.
+   * Throws a runtime exception indicating that the given attribute cannot be set.
    */
-  protected static IllegalArgumentException unsettable(String view, String attribute) {
+  protected static RuntimeException unsettable(String view, String attribute, boolean create) {
+    // This matches the behavior of the real file system implementations: if the attempt to set the
+    // attribute is being made during file creation, throw UOE even though the attribute is one
+    // that cannot be set under any circumstances
+    checkNotCreate(view, attribute, create);
     throw new IllegalArgumentException("cannot set attribute '" + view + ":" + attribute + "'");
   }
 
@@ -177,7 +181,7 @@ public abstract class AttributeProvider {
     Object expected =
         expectedTypes.length == 1 ? expectedTypes[0] : "one of " + Arrays.toString(expectedTypes);
     throw new IllegalArgumentException(
-        "invalid type " + value.getClass() 
+        "invalid type " + value.getClass()
             + " for attribute '" + view + ":" + attribute
             + "': expected " + expected);
   }
diff --git a/jimfs/src/main/java/com/google/common/jimfs/BasicAttributeProvider.java b/jimfs/src/main/java/com/google/common/jimfs/BasicAttributeProvider.java
index b1e110c..e8c2c8a 100644
--- a/jimfs/src/main/java/com/google/common/jimfs/BasicAttributeProvider.java
+++ b/jimfs/src/main/java/com/google/common/jimfs/BasicAttributeProvider.java
@@ -105,7 +105,7 @@ final class BasicAttributeProvider extends AttributeProvider {
       case "isRegularFile":
       case "isSymbolicLink":
       case "isOther":
-        throw unsettable(view, attribute);
+        throw unsettable(view, attribute, create);
       default:
     }
   }
diff --git a/jimfs/src/main/java/com/google/common/jimfs/OwnerAttributeProvider.java b/jimfs/src/main/java/com/google/common/jimfs/OwnerAttributeProvider.java
index 1c89468..24b9fe3 100644
--- a/jimfs/src/main/java/com/google/common/jimfs/OwnerAttributeProvider.java
+++ b/jimfs/src/main/java/com/google/common/jimfs/OwnerAttributeProvider.java
@@ -79,6 +79,7 @@ final class OwnerAttributeProvider extends AttributeProvider {
   @Override
   public void set(File file, String view, String attribute, Object value, boolean create) {
     if (attribute.equals("owner")) {
+      checkNotCreate(view, attribute, create);
       UserPrincipal user = checkType(view, attribute, value, UserPrincipal.class);
       // TODO(cgdecker): Do we really need to do this? Any reason not to allow any UserPrincipal?
       if (!(user instanceof UserLookupService.JimfsUserPrincipal)) {
diff --git a/jimfs/src/main/java/com/google/common/jimfs/UnixAttributeProvider.java b/jimfs/src/main/java/com/google/common/jimfs/UnixAttributeProvider.java
index 552b161..6d444b0 100644
--- a/jimfs/src/main/java/com/google/common/jimfs/UnixAttributeProvider.java
+++ b/jimfs/src/main/java/com/google/common/jimfs/UnixAttributeProvider.java
@@ -128,7 +128,7 @@ final class UnixAttributeProvider extends AttributeProvider {
 
   @Override
   public void set(File file, String view, String attribute, Object value, boolean create) {
-    throw unsettable(view, attribute);
+    throw unsettable(view, attribute, create);
   }
 
   @SuppressWarnings("OctalInteger")
@@ -138,7 +138,7 @@ final class UnixAttributeProvider extends AttributeProvider {
       checkNotNull(permission);
       switch (permission) {
         case OWNER_READ:
-          result |= 0400;
+          result |= 0400; // note: octal numbers
           break;
         case OWNER_WRITE:
           result |= 0200;
diff --git a/jimfs/src/test/java/com/google/common/jimfs/AttributeServiceTest.java b/jimfs/src/test/java/com/google/common/jimfs/AttributeServiceTest.java
index 94b4912..89ea239 100644
--- a/jimfs/src/test/java/com/google/common/jimfs/AttributeServiceTest.java
+++ b/jimfs/src/test/java/com/google/common/jimfs/AttributeServiceTest.java
@@ -219,8 +219,9 @@ public class AttributeServiceTest {
     try {
       service.setInitialAttributes(file, new BasicFileAttribute<>("test:foo", "world"));
       fail();
-    } catch (IllegalArgumentException expected) {
-      // IAE because test:foo just can't be set
+    } catch (UnsupportedOperationException expected) {
+      // it turns out that UOE should be thrown on create even if the attribute isn't settable
+      // under any circumstances
     }
 
     try {
diff --git a/jimfs/src/test/java/com/google/common/jimfs/JimfsUnixLikeFileSystemTest.java b/jimfs/src/test/java/com/google/common/jimfs/JimfsUnixLikeFileSystemTest.java
index adb5fb8..d42cada 100644
--- a/jimfs/src/test/java/com/google/common/jimfs/JimfsUnixLikeFileSystemTest.java
+++ b/jimfs/src/test/java/com/google/common/jimfs/JimfsUnixLikeFileSystemTest.java
@@ -657,16 +657,6 @@ public class JimfsUnixLikeFileSystemTest extends AbstractJimfsIntegrationTest {
 
     assertThatPath("/normal").attribute("posix:permissions").isNot(permissions);
     assertThatPath("/foo").attribute("posix:permissions").is(permissions);
-
-    FileAttribute<UserPrincipal> ownerAttr =
-        new BasicFileAttribute<>(
-            "posix:owner", fs.getUserPrincipalLookupService().lookupPrincipalByName("foo"));
-
-    Files.createFile(path("/foo2"), ownerAttr, permissionsAttr);
-
-    assertThatPath("/normal").attribute("owner:owner").isNot(ownerAttr.value());
-    assertThatPath("/foo2").attribute("owner:owner").is(ownerAttr.value());
-    assertThatPath("/foo2").attribute("posix:permissions").is(permissions);
   }
 
   @Test
diff --git a/jimfs/src/test/java/com/google/common/jimfs/OwnerAttributeProviderTest.java b/jimfs/src/test/java/com/google/common/jimfs/OwnerAttributeProviderTest.java
index c92e6ad..3cd9f76 100644
--- a/jimfs/src/test/java/com/google/common/jimfs/OwnerAttributeProviderTest.java
+++ b/jimfs/src/test/java/com/google/common/jimfs/OwnerAttributeProviderTest.java
@@ -56,7 +56,7 @@ public class OwnerAttributeProviderTest
   @Test
   public void testSet() {
     assertSetAndGetSucceeds("owner", createUserPrincipal("user"));
-    assertSetAndGetSucceedsOnCreate("owner", createUserPrincipal("user"));
+    assertSetFailsOnCreate("owner", createUserPrincipal("user"));
 
     // invalid type
     assertSetFails("owner", "root");
diff --git a/jimfs/src/test/java/com/google/common/jimfs/TestAttributeProvider.java b/jimfs/src/test/java/com/google/common/jimfs/TestAttributeProvider.java
index 79b92ac..306f745 100644
--- a/jimfs/src/test/java/com/google/common/jimfs/TestAttributeProvider.java
+++ b/jimfs/src/test/java/com/google/common/jimfs/TestAttributeProvider.java
@@ -82,7 +82,7 @@ public final class TestAttributeProvider extends AttributeProvider {
         file.setAttribute("test", "baz", checkType(view, attribute, value, Integer.class));
         break;
       default:
-        throw unsettable(view, attribute);
+        throw unsettable(view, attribute, create);
     }
   }