Make DosFileAttributes immutable (#305)

references #42

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Sebastian Stenzel <[email protected]>

Author: 3 people

src/main/java/org/cryptomator/cryptofs/attr/CryptoDosFileAttributes.java

@@ -19,32 +19,41 @@
 
 final class CryptoDosFileAttributes extends CryptoBasicFileAttributes implements DosFileAttributes {
 
-	private final boolean readonlyFileSystem;
-	private final DosFileAttributes delegate;
-
-	public CryptoDosFileAttributes(DosFileAttributes delegate, CiphertextFileType ciphertextFileType, Path ciphertextPath, Cryptor cryptor, Optional<OpenCryptoFile> openCryptoFile, CryptoFileSystemProperties fileSystemProperties) {
+	private final boolean isReadOnly;
+	private final boolean isArchive;
+	private final boolean isHidden;
+	private final boolean isSystem;
+
+	public CryptoDosFileAttributes(DosFileAttributes delegate, //
+								   CiphertextFileType ciphertextFileType, //
+								   Path ciphertextPath, //
+								   Cryptor cryptor, //
+								   Optional<OpenCryptoFile> openCryptoFile, //
+								   CryptoFileSystemProperties fileSystemProperties) {
 		super(delegate, ciphertextFileType, ciphertextPath, cryptor, openCryptoFile);
-		this.readonlyFileSystem = fileSystemProperties.readonly();
-		this.delegate = delegate;
+		this.isReadOnly = fileSystemProperties.readonly() || delegate.isReadOnly();
+		this.isHidden = delegate.isHidden();
+		this.isArchive = delegate.isArchive();
+		this.isSystem = delegate.isSystem();
 	}
 
 	@Override
 	public boolean isReadOnly() {
-		return readonlyFileSystem || delegate.isReadOnly();
+		return isReadOnly;
 	}
 
 	@Override
 	public boolean isHidden() {
-		return delegate.isHidden();
+		return isHidden;
 	}
 
 	@Override
 	public boolean isArchive() {
-		return delegate.isArchive();
+		return isArchive;
 	}
 
 	@Override
 	public boolean isSystem() {
-		return delegate.isSystem();
+		return isSystem;
 	}
 }

src/test/java/org/cryptomator/cryptofs/attr/CryptoDosFileAttributesTest.java

@@ -7,21 +7,22 @@
 import org.cryptomator.cryptolib.api.FileContentCryptor;
 import org.cryptomator.cryptolib.api.FileHeaderCryptor;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
-import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
 import java.nio.file.attribute.DosFileAttributes;
 import java.util.Optional;
 
 import static org.cryptomator.cryptofs.common.CiphertextFileType.FILE;
+import static org.mockito.Mockito.atMostOnce;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 public class CryptoDosFileAttributesTest {
 
 	private DosFileAttributes delegate = mock(DosFileAttributes.class);
@@ -32,9 +33,9 @@ public class CryptoDosFileAttributesTest {
 	private OpenCryptoFile openCryptoFile = mock(OpenCryptoFile.class);
 	private CryptoFileSystemProperties cryptoFileSystemProperties = mock(CryptoFileSystemProperties.class);
 
-	@BeforeAll
+	@BeforeEach
 	public void setup() {
-		when(delegate.size()).thenReturn(0l);
+		when(delegate.size()).thenReturn(0L);
 		when(cryptor.fileHeaderCryptor()).thenReturn(headerCryptor);
 		when(cryptor.fileContentCryptor()).thenReturn(contentCryptor);
 		when(headerCryptor.headerSize()).thenReturn(0);
@@ -43,77 +44,85 @@ public void setup() {
 	}
 
 	@Nested
-	@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 	@DisplayName("on read-write filesystem")
 	public class ReadWriteFileSystem {
 
 		private CryptoDosFileAttributes inTest;
 
-		@BeforeAll
-		public void setup() {
+		@BeforeEach
+		public void beforeEach() {
 			when(cryptoFileSystemProperties.readonly()).thenReturn(false);
-			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 		}
 
 		@DisplayName("isArchive()")
 		@ParameterizedTest(name = "is {0} if delegate.isArchive() is {0}")
 		@CsvSource({"true", "false"})
-		public void testIsArchiveDelegates(boolean value) {
+		public void testIsArchiveImmutable(boolean value) {
 			when(delegate.isArchive()).thenReturn(value);
+			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 
-			Assertions.assertSame(value, inTest.isArchive());
+			verify(delegate, times(1)).isArchive();
+			Assertions.assertEquals(value, inTest.isArchive());
+			verify(delegate, times(1)).isArchive();
 		}
 
 		@DisplayName("isHidden()")
 		@ParameterizedTest(name = "is {0} if delegate.isHidden() is {0}")
 		@CsvSource({"true", "false"})
-		public void testIsHiddenDelegates(boolean value) {
+		public void testIsHiddenImmutable(boolean value) {
 			when(delegate.isHidden()).thenReturn(value);
+			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 
-			Assertions.assertSame(value, inTest.isHidden());
+			verify(delegate, times(1)).isHidden();
+			Assertions.assertEquals(value, inTest.isHidden());
+			verify(delegate, times(1)).isHidden();
 		}
 
 		@DisplayName("isReadOnly()")
 		@ParameterizedTest(name = "is {0} if delegate.readOnly() is {0}")
 		@CsvSource({"true", "false"})
-		public void testIsReadOnlyDelegates(boolean value) {
+		public void testIsReadOnlyImmutable(boolean value) {
 			when(delegate.isReadOnly()).thenReturn(value);
+			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 
-			Assertions.assertSame(value, inTest.isReadOnly());
+			verify(delegate, times(1)).isReadOnly();
+			Assertions.assertEquals(value, inTest.isReadOnly());
+			verify(delegate, times(1)).isReadOnly();
 		}
 
 		@DisplayName("isSystem()")
 		@ParameterizedTest(name = "is {0} if delegate.isSystem() is {0}")
 		@CsvSource({"true", "false"})
-		public void testIsSystemDelegates(boolean value) {
+		public void testIsSystemImmutable(boolean value) {
 			when(delegate.isSystem()).thenReturn(value);
+			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 
-			Assertions.assertSame(value, inTest.isSystem());
+			verify(delegate, times(1)).isSystem();
+			Assertions.assertEquals(value, inTest.isSystem());
+			verify(delegate, times(1)).isSystem();
 		}
 
 	}
 
 	@Nested
-	@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 	@DisplayName("on read-only filesystem")
 	public class ReadOnlyFileSystem {
 
-		private CryptoDosFileAttributes inTest;
-
-		@BeforeAll
-		public void setup() {
+		@BeforeEach
+		public void beforeEach() {
 			when(cryptoFileSystemProperties.readonly()).thenReturn(true);
-			inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 		}
 
 		@DisplayName("isReadOnly()")
 		@ParameterizedTest(name = "is true if delegate.readOnly() is {0}")
 		@CsvSource({"true", "false"})
 		public void testIsReadOnlyForReadonlyFileSystem(boolean value) {
 			when(delegate.isReadOnly()).thenReturn(value);
+			var inTest = new CryptoDosFileAttributes(delegate, FILE, path, cryptor, Optional.of(openCryptoFile), cryptoFileSystemProperties);
 
+			verify(delegate, atMostOnce()).isReadOnly();
 			Assertions.assertTrue(inTest.isReadOnly());
+			verify(delegate, atMostOnce()).isReadOnly();
 		}
-
 	}
 }