Skip to content

Commit 6d75e7b

Browse files
infeoinfeo
committed
refactor setting filesystem owner
* use a supply method instead of a constant string
1 parent e5e24a5 commit 6d75e7b

File tree

5 files changed

+99
-43
lines changed

5 files changed

+99
-43
lines changed

‎src/main/java/org/cryptomator/cryptofs/CryptoFileSystemModule.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.slf4j.Logger;
2020
import org.slf4j.LoggerFactory;
2121

22+
import javax.inject.Named;
2223
import java.io.IOException;
2324
import java.nio.file.FileStore;
2425
import java.nio.file.Files;
@@ -58,13 +59,23 @@ public Consumer<FilesystemEvent> provideFilesystemEventConsumer(CryptoFileSystem
5859

5960
@Provides
6061
@CryptoFileSystemScoped
61-
public InUseManager provideInUseManager(CryptoFileSystemProperties fsProps, Cryptor cryptor) {
62-
var owner = Objects.requireNonNullElse(fsProps.owner(), "");
63-
if (!owner.isBlank() && !fsProps.readonly()) {
64-
return new RealInUseManager(owner, cryptor);
65-
} else {
66-
return new StubInUseManager();
62+
@Named("fsOwner")
63+
public Optional<String> provideFsOwner(CryptoFileSystemProperties fsProps) {
64+
var owner = Objects.requireNonNullElse(fsProps.ownerGetter().get(), "");
65+
66+
if (owner.isBlank()) {
67+
return Optional.empty();
6768
}
6869

70+
return Optional.of(owner.length() <= 100 ? owner : owner.substring(0, 100));
71+
}
72+
73+
@Provides
74+
@CryptoFileSystemScoped
75+
public InUseManager provideInUseManager(CryptoFileSystemProperties fsProps, @Named("fsOwner") Optional<String> fsOwner, Cryptor cryptor) {
76+
if (fsOwner.isEmpty() || fsProps.readonly()) {
77+
return new StubInUseManager();
78+
}
79+
return new RealInUseManager(fsOwner.get(), cryptor);
6980
}
7081
}

‎src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProperties.java‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Map;
2323
import java.util.Set;
2424
import java.util.function.Consumer;
25+
import java.util.function.Supplier;
2526

2627
import static java.util.Arrays.asList;
2728

@@ -116,12 +117,13 @@ public enum FileSystemFlags {
116117
static final CryptorProvider.Scheme DEFAULT_CIPHER_COMBO = CryptorProvider.Scheme.SIV_GCM;
117118

118119
/**
119-
* Key identifying the filesystem owner.
120+
* Key identifying the filesystem owner supply method.
120121
*
121122
* @since 2.10.0
122123
*/
123-
public static final String PROPERTY_OWNER = "owner";
124-
static final String DEFAULT_OWNER = "";
124+
public static final String PROPERTY_OWNER_GETTER = "ownerGetter";
125+
126+
static final Supplier<String> DEFAULT_OWNER_GETTER = () -> "";
125127

126128
private final Set<Entry<String, Object>> entries;
127129

@@ -135,7 +137,7 @@ private CryptoFileSystemProperties(Builder builder) {
135137
Map.entry(PROPERTY_MAX_CLEARTEXT_NAME_LENGTH, builder.maxCleartextNameLength), //
136138
Map.entry(PROPERTY_SHORTENING_THRESHOLD, builder.shorteningThreshold), //
137139
Map.entry(PROPERTY_CIPHER_COMBO, builder.cipherCombo), //
138-
Map.entry(PROPERTY_OWNER, builder.owner) //
140+
Map.entry(PROPERTY_OWNER_GETTER, builder.ownerGetter) //
139141
);
140142
}
141143

@@ -178,8 +180,8 @@ Consumer<FilesystemEvent> filesystemEventConsumer() {
178180
return (Consumer<FilesystemEvent>) get(PROPERTY_EVENT_CONSUMER);
179181
}
180182

181-
String owner() {
182-
return (String) get(PROPERTY_OWNER);
183+
Supplier<String> ownerGetter() {
184+
return (Supplier<String>) get(PROPERTY_OWNER_GETTER);
183185
}
184186

185187
@Override
@@ -238,7 +240,7 @@ public static class Builder {
238240
private int maxCleartextNameLength = DEFAULT_MAX_CLEARTEXT_NAME_LENGTH;
239241
private int shorteningThreshold = DEFAULT_SHORTENING_THRESHOLD;
240242
private Consumer<FilesystemEvent> eventConsumer = DEFAULT_EVENT_CONSUMER;
241-
private String owner = DEFAULT_OWNER;
243+
private Supplier<String> ownerGetter = DEFAULT_OWNER_GETTER;
242244

243245
private Builder() {
244246
}
@@ -252,7 +254,7 @@ private Builder(Map<String, ?> properties) {
252254
checkedSet(Integer.class, PROPERTY_SHORTENING_THRESHOLD, properties, this::withShorteningThreshold);
253255
checkedSet(CryptorProvider.Scheme.class, PROPERTY_CIPHER_COMBO, properties, this::withCipherCombo);
254256
checkedSet(Consumer.class, PROPERTY_EVENT_CONSUMER, properties, this::withFilesystemEventConsumer);
255-
checkedSet(String.class, PROPERTY_OWNER, properties, this::withOwner);
257+
checkedSet(Supplier.class, PROPERTY_OWNER_GETTER, properties, this::withOwnerGetter);
256258
}
257259

258260
private <T> void checkedSet(Class<T> type, String key, Map<String, ?> properties, Consumer<T> setter) {
@@ -383,19 +385,20 @@ public Builder withFilesystemEventConsumer(Consumer<FilesystemEvent> eventConsum
383385
}
384386

385387
/**
386-
* Sets the owner of the filesystem.
388+
* Sets the getter method for the filesystem owner.
387389
* <p>
388-
* The owners length must be less than or equal to 100.
390+
* The string returned by the supply method must be less than or equal to 100.
391+
* If the string is longer, it will be truncated.
389392
*
390-
* @param owner the owner string used when marking files in-use
393+
* @param ownerGetter method to supply the fs owner
391394
* @return this
392395
* @since 2.10.0
393396
*/
394-
public Builder withOwner(String owner) {
395-
if (owner.length() > 100) {
396-
throw new IllegalArgumentException("owner must have length less than or equal to 100");
397+
public Builder withOwnerGetter(Supplier<String> ownerGetter) {
398+
if (ownerGetter == null) {
399+
throw new IllegalArgumentException("Parameter ownerGetter must not be null");
397400
}
398-
this.owner = owner;
401+
this.ownerGetter = ownerGetter;
399402
return this;
400403
}
401404

‎src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public void beforeAll() throws IOException, MasterkeyLoadingFailedException {
162162
Files.createDirectories(vaultPath);
163163
MasterkeyLoader keyLoader = Mockito.mock(MasterkeyLoader.class);
164164
Mockito.when(keyLoader.loadKey(Mockito.any())).thenAnswer(ignored -> new Masterkey(new byte[64]));
165-
var properties = cryptoFileSystemProperties().withKeyLoader(keyLoader).withOwner("cryptobot").build();
165+
var properties = cryptoFileSystemProperties().withKeyLoader(keyLoader).withOwnerGetter(() -> "cryptobot").build();
166166
CryptoFileSystemProvider.initialize(vaultPath, properties, URI.create("test:key"));
167167
fileSystem = CryptoFileSystemProvider.newFileSystem(vaultPath, properties);
168168
file = fileSystem.getPath("/test.txt");

‎src/test/java/org/cryptomator/cryptofs/CryptoFileSystemModuleTest.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,23 @@
22

33
import org.cryptomator.cryptofs.event.ConflictResolutionFailedEvent;
44
import org.cryptomator.cryptofs.event.FilesystemEvent;
5+
import org.cryptomator.cryptofs.inuse.StubInUseManager;
6+
import org.cryptomator.cryptolib.api.Cryptor;
57
import org.junit.jupiter.api.Assertions;
8+
import org.junit.jupiter.api.DisplayName;
69
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.params.ParameterizedTest;
11+
import org.junit.jupiter.params.provider.Arguments;
12+
import org.junit.jupiter.params.provider.MethodSource;
13+
import org.junit.jupiter.params.provider.NullSource;
14+
import org.junit.jupiter.params.provider.ValueSource;
715
import org.mockito.Mockito;
816

917
import java.nio.file.Path;
18+
import java.util.Optional;
1019
import java.util.function.Consumer;
20+
import java.util.function.Supplier;
21+
import java.util.stream.Stream;
1122

1223
import static org.mockito.Mockito.doThrow;
1324
import static org.mockito.Mockito.mock;
@@ -32,4 +43,46 @@ void testEventConsumerIsDecorated() {
3243
verify(eventConsumer).accept(event);
3344
}
3445

46+
@Test
47+
void testInUseManagerIsStubOnReadOnly() {
48+
var cryptor = Mockito.mock(Cryptor.class);
49+
var props = mock(CryptoFileSystemProperties.class);
50+
when(props.readonly()).thenReturn(true);
51+
var result = inTest.provideInUseManager(props, Optional.of("someOwner"), cryptor);
52+
53+
Assertions.assertInstanceOf(StubInUseManager.class, result);
54+
}
55+
56+
@ParameterizedTest
57+
@DisplayName("If ownerGetter returns null or blank, return empty optional")
58+
@ValueSource(strings = {" \t "})
59+
@NullSource
60+
void testOwnerIsBlankOrNull(String input) {
61+
Supplier<String> ownerGetter = () -> input;
62+
var props = mock(CryptoFileSystemProperties.class);
63+
when(props.ownerGetter()).thenReturn(ownerGetter);
64+
var result = inTest.provideFsOwner(props);
65+
66+
Assertions.assertTrue(result.isEmpty());
67+
}
68+
69+
@ParameterizedTest
70+
@DisplayName("Filesystem owner is always less than 100 chars")
71+
@MethodSource("provideArgsForOwnerIsInsideBounds")
72+
void testOwnerIsInsideBounds(String input, int expectedLength) {
73+
Supplier<String> ownerGetter = () -> input;
74+
var props = mock(CryptoFileSystemProperties.class);
75+
when(props.ownerGetter()).thenReturn(ownerGetter);
76+
var result = inTest.provideFsOwner(props);
77+
78+
Assertions.assertTrue(result.isPresent());
79+
Assertions.assertEquals(expectedLength, result.get().length());
80+
}
81+
82+
private static Stream<Arguments> provideArgsForOwnerIsInsideBounds() {
83+
return Stream.of(
84+
Arguments.of("a", 1),
85+
Arguments.of("b".repeat(101), 100)
86+
);
87+
}
3588
}

‎src/test/java/org/cryptomator/cryptofs/CryptoFileSystemPropertiesTest.java‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,10 @@ public void testSetMasterkeyFilenameAndReadonlyFlag() {
5353
anEntry(PROPERTY_CIPHER_COMBO, DEFAULT_CIPHER_COMBO), //
5454
anEntry(PROPERTY_FILESYSTEM_FLAGS, EnumSet.of(FileSystemFlags.READONLY)), //
5555
anEntry(PROPERTY_EVENT_CONSUMER, DEFAULT_EVENT_CONSUMER), //
56-
anEntry(PROPERTY_OWNER, DEFAULT_OWNER))
56+
anEntry(PROPERTY_OWNER_GETTER, DEFAULT_OWNER_GETTER))
5757
);
5858
}
5959

60-
@Test
61-
public void testOwnerSizeRestriction() {
62-
String owner1 = "\u2741".repeat(100);
63-
String owner2 = "\u2741".repeat(101);
64-
65-
Assertions.assertDoesNotThrow(() -> cryptoFileSystemProperties() //
66-
.withKeyLoader(keyLoader) //
67-
.withOwner(owner1) //
68-
.build());
69-
Assertions.assertThrows(IllegalArgumentException.class, () -> cryptoFileSystemProperties() //
70-
.withKeyLoader(keyLoader) //
71-
.withOwner(owner2) //
72-
.build());
73-
74-
}
75-
7660
@Test
7761
public void testFromMap() {
7862
Map<String, Object> map = new HashMap<>();
@@ -98,7 +82,7 @@ public void testFromMap() {
9882
anEntry(PROPERTY_CIPHER_COMBO, DEFAULT_CIPHER_COMBO), //
9983
anEntry(PROPERTY_FILESYSTEM_FLAGS, EnumSet.of(FileSystemFlags.READONLY)), //
10084
anEntry(PROPERTY_EVENT_CONSUMER, DEFAULT_EVENT_CONSUMER), //
101-
anEntry(PROPERTY_OWNER, DEFAULT_OWNER)) //
85+
anEntry(PROPERTY_OWNER_GETTER, DEFAULT_OWNER_GETTER)) //
10286
);
10387
}
10488

@@ -123,7 +107,7 @@ public void testWrapMapWithTrueReadonly() {
123107
anEntry(PROPERTY_CIPHER_COMBO, DEFAULT_CIPHER_COMBO), //
124108
anEntry(PROPERTY_FILESYSTEM_FLAGS, EnumSet.of(FileSystemFlags.READONLY)), //
125109
anEntry(PROPERTY_EVENT_CONSUMER, DEFAULT_EVENT_CONSUMER), //
126-
anEntry(PROPERTY_OWNER, DEFAULT_OWNER)) //
110+
anEntry(PROPERTY_OWNER_GETTER, DEFAULT_OWNER_GETTER)) //
127111
);
128112
}
129113

@@ -148,7 +132,7 @@ public void testWrapMapWithFalseReadonly() {
148132
anEntry(PROPERTY_CIPHER_COMBO, DEFAULT_CIPHER_COMBO), //
149133
anEntry(PROPERTY_FILESYSTEM_FLAGS, EnumSet.noneOf(FileSystemFlags.class)), //
150134
anEntry(PROPERTY_EVENT_CONSUMER, DEFAULT_EVENT_CONSUMER), //
151-
anEntry(PROPERTY_OWNER, DEFAULT_OWNER)) //
135+
anEntry(PROPERTY_OWNER_GETTER, DEFAULT_OWNER_GETTER)) //
152136
);
153137
}
154138

@@ -201,6 +185,11 @@ public void testNullEventConsumerThrowsIAE() {
201185
Assertions.assertThrows(IllegalArgumentException.class, () -> CryptoFileSystemProperties.cryptoFileSystemProperties().withFilesystemEventConsumer(null));
202186
}
203187

188+
@Test
189+
public void testNullOwnerGetterThrowsIAE() {
190+
Assertions.assertThrows(IllegalArgumentException.class, () -> CryptoFileSystemProperties.cryptoFileSystemProperties().withOwnerGetter(null));
191+
}
192+
204193
@Test
205194
public void testWrapMapWithoutReadonly() {
206195
Map<String, Object> map = new HashMap<>();
@@ -219,7 +208,7 @@ public void testWrapMapWithoutReadonly() {
219208
anEntry(PROPERTY_CIPHER_COMBO, DEFAULT_CIPHER_COMBO), //
220209
anEntry(PROPERTY_FILESYSTEM_FLAGS, EnumSet.noneOf(FileSystemFlags.class)), //
221210
anEntry(PROPERTY_EVENT_CONSUMER, DEFAULT_EVENT_CONSUMER), //
222-
anEntry(PROPERTY_OWNER, DEFAULT_OWNER)) //
211+
anEntry(PROPERTY_OWNER_GETTER, DEFAULT_OWNER_GETTER)) //
223212
);
224213
}
225214

0 commit comments

Comments
 (0)