Skip to content

Commit 96e683c

Browse files
authored
refactor: merge pull request (#30) from refactor/polish-code
refactor: rename SecureTempFileManager to TempFileManager & handle privilege access with FileValidatorTest
2 parents 8a061a2 + 852eb94 commit 96e683c

File tree

13 files changed

+91
-100
lines changed

13 files changed

+91
-100
lines changed

src/main/java/com/example/arinfra/file/BucketComponent.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
*
3939
* <ul>
4040
* <li>All bucket keys are sanitized before logging
41-
* <li>Temporary files are created using {@link SecureTempFileManager}
41+
* <li>Temporary files are created using {@link TempFileManager}
4242
* <li>Exceptions are domain-specific and controller-advice friendly
4343
* </ul>
4444
*
@@ -51,7 +51,7 @@
5151
public class BucketComponent {
5252

5353
private final BucketConf bucketConf;
54-
private final SecureTempFileManager secureTempFileManager;
54+
private final TempFileManager tempFileManager;
5555

5656
/**
5757
* Uploads a file or directory to the configured bucket.
@@ -116,7 +116,7 @@ public File download(String bucketKey) {
116116

117117
try {
118118
File destination =
119-
secureTempFileManager.createSecureTempFile("b2-", "-" + bucketKey.replace("/", "-"));
119+
tempFileManager.createSecureTempFile("b2-", "-" + bucketKey.replace("/", "-"));
120120

121121
var request =
122122
DownloadFileRequest.builder()

src/main/java/com/example/arinfra/file/FileZipManager.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public class FileZipManager {
8383
private static final int SYMLINK_UNIX_MODE = 0120000;
8484
private static final int UNIX_FILE_TYPE_MASK = 0170000;
8585

86-
private final SecureTempFileManager secureTempFileManager;
86+
private final TempFileManager tempFileManager;
8787
private final FilenameSanitizer filenameSanitizer;
8888
private final FileValidator fileValidator;
8989
private final ZipEntryValidator zipEntryValidator;
@@ -105,7 +105,7 @@ public File zipFile(
105105
fileValidator.validateReadableFile(sourcePath);
106106

107107
String sanitizedZipFileName = ensureZipExtension(filenameSanitizer.apply(zipFileName));
108-
File zipFile = secureTempFileManager.createSecureTempFile(ZIP_PREFIX, sanitizedZipFileName);
108+
File zipFile = tempFileManager.createSecureTempFile(ZIP_PREFIX, sanitizedZipFileName);
109109

110110
try (ZipArchiveOutputStream zos = createZipOutputStream(zipFile.toPath())) {
111111
addFileToZip(zos, sourcePath, sourcePath.getFileName().toString());
@@ -116,7 +116,7 @@ public File zipFile(
116116
forJava(zipFile.getAbsolutePath()));
117117
return zipFile;
118118
} catch (IOException e) {
119-
secureTempFileManager.deleteTempFile(zipFile);
119+
tempFileManager.deleteTempFile(zipFile);
120120
log.error("Failed to zip file: {}", forJava(sourcePath.toString()), e);
121121
throw e;
122122
}
@@ -139,7 +139,7 @@ public File zipDirectory(
139139
fileValidator.validateReadableDirectory(sourcePath);
140140

141141
String sanitizedZipFileName = ensureZipExtension(filenameSanitizer.apply(zipFileName));
142-
File zipFile = secureTempFileManager.createSecureTempFile(ZIP_PREFIX, sanitizedZipFileName);
142+
File zipFile = tempFileManager.createSecureTempFile(ZIP_PREFIX, sanitizedZipFileName);
143143

144144
try (ZipArchiveOutputStream zos = createZipOutputStream(zipFile.toPath())) {
145145
AtomicLong entryCount = new AtomicLong(0);
@@ -152,7 +152,7 @@ public File zipDirectory(
152152
forJava(zipFile.getAbsolutePath()));
153153
return zipFile;
154154
} catch (IOException | SecurityException e) {
155-
secureTempFileManager.deleteTempFile(zipFile);
155+
tempFileManager.deleteTempFile(zipFile);
156156
log.error("Failed to zip directory: {}", forJava(sourcePath.toString()), e);
157157
throw e;
158158
}
@@ -211,7 +211,7 @@ public File unzipToTempDirectory(@NotNull(message = "ZIP file cannot be null") F
211211
throws IOException {
212212

213213
fileValidator.validateReadableFile(zipFile.toPath());
214-
File tempDir = secureTempFileManager.createSecureTempDirectory(UNZIP_PREFIX);
214+
File tempDir = tempFileManager.createSecureTempDirectory(UNZIP_PREFIX);
215215
return unzip(zipFile, tempDir);
216216
}
217217

src/main/java/com/example/arinfra/file/MultipartFileConverter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public class MultipartFileConverter implements Function<MultipartFile, File> {
2727
private static final String DEFAULT_EXTENSION = "tmp";
2828
private static final int MAX_EXTENSION_LENGTH = 10;
2929

30-
private final SecureTempFileManager secureTempFileManager;
30+
private final TempFileManager tempFileManager;
3131

3232
/**
3333
* Converts a MultipartFile to a secure temporary File.
@@ -43,7 +43,7 @@ public File apply(MultipartFile multipartFile) {
4343

4444
try {
4545
String safeSuffix = "." + extractSafeFileExtension(originalFilename);
46-
File tempFile = secureTempFileManager.createSecureTempFile(UPLOAD_PREFIX, safeSuffix);
46+
File tempFile = tempFileManager.createSecureTempFile(UPLOAD_PREFIX, safeSuffix);
4747

4848
multipartFile.transferTo(tempFile);
4949

src/main/java/com/example/arinfra/file/SecureTempFileManager.java renamed to src/main/java/com/example/arinfra/file/TempFileManager.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
@Component
3333
@InfraGenerated
3434
@RequiredArgsConstructor
35-
public class SecureTempFileManager {
35+
public class TempFileManager {
3636

3737
private static final String POSIX_OWNER_ONLY_PERMISSIONS = "rw-------";
3838
private static final String POSIX_DIRECTORY_OWNER_ONLY_PERMISSIONS = "rwx------";

src/main/java/com/example/arinfra/service/health/HealthBucketService.java

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import com.example.arinfra.InfraGenerated;
77
import com.example.arinfra.exception.bucket.BucketHealthCheckException;
88
import com.example.arinfra.file.BucketComponent;
9-
import com.example.arinfra.file.SecureTempFileManager;
9+
import com.example.arinfra.file.TempFileManager;
1010
import java.io.File;
1111
import java.io.IOException;
1212
import java.net.URL;
@@ -29,7 +29,7 @@ public class HealthBucketService {
2929
private static final Duration PRESIGN_DURATION = Duration.ofMinutes(2);
3030

3131
private final BucketComponent bucketComponent;
32-
private final SecureTempFileManager secureTempFileManager;
32+
private final TempFileManager tempFileManager;
3333

3434
/**
3535
* Performs a comprehensive health check on bucket operations including file upload, download,
@@ -66,7 +66,7 @@ private void testFileUploadAndDownload() throws IOException {
6666
String content = generateRandomContent();
6767

6868
File fileToUpload =
69-
secureTempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
69+
tempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
7070

7171
try {
7272
String fileBucketKey = buildBucketKey(fileId);
@@ -79,10 +79,10 @@ private void testFileUploadAndDownload() throws IOException {
7979
validateFileContent(fileToUpload, downloaded);
8080
log.debug("File upload and download validation successful");
8181
} finally {
82-
secureTempFileManager.deleteTempFile(downloaded);
82+
tempFileManager.deleteTempFile(downloaded);
8383
}
8484
} finally {
85-
secureTempFileManager.deleteTempFile(fileToUpload);
85+
tempFileManager.deleteTempFile(fileToUpload);
8686
}
8787
}
8888

@@ -94,13 +94,13 @@ private void testDirectoryUpload() throws IOException {
9494
String dirId = randomUUID().toString();
9595
String dirPrefix = DIR_PREFIX + dirId;
9696

97-
File dir = secureTempFileManager.createSecureTempDirectory(dirPrefix);
97+
File dir = tempFileManager.createSecureTempDirectory(dirPrefix);
9898
File fileInDir = null;
9999

100100
try {
101101
String content = generateRandomContent();
102102
fileInDir =
103-
secureTempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
103+
tempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
104104

105105
File targetFile = new File(dir, fileInDir.getName());
106106
Files.move(fileInDir.toPath(), targetFile.toPath());
@@ -126,15 +126,14 @@ private String uploadTestFile() throws IOException {
126126
String fileId = randomUUID().toString();
127127
String content = generateRandomContent();
128128

129-
File file =
130-
secureTempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
129+
File file = tempFileManager.createSecureTempFileWithContent(FILE_PREFIX, FILE_SUFFIX, content);
131130

132131
try {
133132
String bucketKey = buildBucketKey(fileId);
134133
bucketComponent.upload(file, bucketKey);
135134
return bucketKey;
136135
} finally {
137-
secureTempFileManager.deleteTempFile(file);
136+
tempFileManager.deleteTempFile(file);
138137
}
139138
}
140139

@@ -183,7 +182,7 @@ private void validateFileContent(File original, File downloaded) throws IOExcept
183182
* @param fileInDir the file inside the directory (can be null)
184183
*/
185184
private void cleanupDirectory(File dir, File fileInDir) {
186-
if (fileInDir != null && fileInDir.exists()) secureTempFileManager.deleteTempFile(fileInDir);
185+
if (fileInDir != null && fileInDir.exists()) tempFileManager.deleteTempFile(fileInDir);
187186

188187
if (dir != null && dir.exists()) {
189188
try {

src/main/java/com/example/arinfra/service/health/HealthEmailService.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import com.example.arinfra.InfraGenerated;
66
import com.example.arinfra.exception.health.EmailHealthCheckException;
7-
import com.example.arinfra.file.SecureTempFileManager;
7+
import com.example.arinfra.file.TempFileManager;
88
import com.example.arinfra.mail.Email;
99
import com.example.arinfra.mail.Mailer;
1010
import jakarta.mail.internet.AddressException;
@@ -27,7 +27,7 @@ public class HealthEmailService {
2727
private static final String TEST_ATTACHMENT_SUFFIX = ".txt";
2828

2929
private final Mailer mailer;
30-
private final SecureTempFileManager secureTempFileManager;
30+
private final TempFileManager tempFileManager;
3131

3232
/**
3333
* Sends a comprehensive set of test emails to verify email functionality.
@@ -139,7 +139,7 @@ private void sendEmailWithAttachment(InternetAddress toAddress) throws EmailHeal
139139
File attachment = null;
140140
try {
141141
attachment =
142-
secureTempFileManager.createSecureTempFileWithContent(
142+
tempFileManager.createSecureTempFileWithContent(
143143
TEST_ATTACHMENT_PREFIX, TEST_ATTACHMENT_SUFFIX, attachmentContent);
144144

145145
mailer.accept(
@@ -156,7 +156,7 @@ private void sendEmailWithAttachment(InternetAddress toAddress) throws EmailHeal
156156
"with-attachment", "Failed to create or send attachment", e);
157157
} finally {
158158
if (attachment != null) {
159-
secureTempFileManager.deleteTempFile(attachment);
159+
tempFileManager.deleteTempFile(attachment);
160160
}
161161
}
162162
}

src/test/java/com/example/arinfra/conf/FacadeIT.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818
* <p>This class starts and manages the lifecycle of the following Testcontainers:
1919
*
2020
* <ul>
21-
* <li><b>PostgreSQL</b> persistence layer
22-
* <li><b>RabbitMQ</b> asynchronous messaging
23-
* <li><b>S3-compatible bucket</b> file storage
24-
* <li><b>Email service</b> outbound email testing
21+
* <li><b>PostgreSQL</b> - persistence layer
22+
* <li><b>RabbitMQ</b> - asynchronous messaging
23+
* <li><b>S3-compatible bucket</b> - file storage
24+
* <li><b>Email service</b> - outbound email testing
2525
* </ul>
2626
*
2727
* <p>Additionally, it dynamically injects environment variables and container connection properties

src/test/java/com/example/arinfra/file/FileZipManagerTest.java

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class FileZipManagerTest {
4545

4646
@TempDir Path tempDir;
4747

48-
@Mock private SecureTempFileManager secureTempFileManager;
48+
@Mock private TempFileManager tempFileManager;
4949

5050
@Mock private FilenameSanitizer filenameSanitizer;
5151

@@ -58,8 +58,7 @@ class FileZipManagerTest {
5858
@BeforeEach
5959
void setUp() {
6060
fileZipManager =
61-
new FileZipManager(
62-
secureTempFileManager, filenameSanitizer, fileValidator, zipEntryValidator);
61+
new FileZipManager(tempFileManager, filenameSanitizer, fileValidator, zipEntryValidator);
6362
}
6463

6564
@Test
@@ -68,8 +67,7 @@ void zip_file_with_valid_file_should_create_zip() throws IOException {
6867
File zipFile = tempDir.resolve(TEST_ZIP_NAME).toFile();
6968

7069
when(filenameSanitizer.apply(TEST_ZIP_NAME)).thenReturn(TEST_ZIP_NAME);
71-
when(secureTempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME)))
72-
.thenReturn(zipFile);
70+
when(tempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME))).thenReturn(zipFile);
7371
doNothing().when(fileValidator).validateReadableFile(any(Path.class));
7472

7573
File result = fileZipManager.zipFile(sourceFile, TEST_ZIP_NAME);
@@ -78,7 +76,7 @@ void zip_file_with_valid_file_should_create_zip() throws IOException {
7876
assertTrue(result.exists());
7977
verify(fileValidator).validateReadableFile(sourceFile.toPath());
8078
verify(filenameSanitizer).apply(TEST_ZIP_NAME);
81-
verify(secureTempFileManager).createSecureTempFile(anyString(), eq(TEST_ZIP_NAME));
79+
verify(tempFileManager).createSecureTempFile(anyString(), eq(TEST_ZIP_NAME));
8280

8381
verifyZipContainsEntry(result, TEST_FILE_NAME);
8482
}
@@ -114,8 +112,7 @@ void zip_file_should_add_zip_extension_if_missing() throws IOException {
114112
String nameWithoutExtension = "test";
115113

116114
when(filenameSanitizer.apply(nameWithoutExtension)).thenReturn(nameWithoutExtension);
117-
when(secureTempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME)))
118-
.thenReturn(zipFile);
115+
when(tempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME))).thenReturn(zipFile);
119116
doNothing().when(fileValidator).validateReadableFile(any(Path.class));
120117

121118
File result = fileZipManager.zipFile(sourceFile, nameWithoutExtension);
@@ -130,8 +127,7 @@ void zip_directory_with_valid_directory_should_create_zip() throws IOException {
130127
File zipFile = tempDir.resolve(TEST_ZIP_NAME).toFile();
131128

132129
when(filenameSanitizer.apply(anyString())).thenAnswer(i -> i.getArgument(0));
133-
when(secureTempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME)))
134-
.thenReturn(zipFile);
130+
when(tempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME))).thenReturn(zipFile);
135131
doNothing().when(fileValidator).validateReadableDirectory(any(Path.class));
136132
doNothing().when(zipEntryValidator).validateEntryCount(anyInt());
137133

@@ -157,7 +153,7 @@ void zip_directory_with_nested_structure_should_preserve_hierarchy() throws IOEx
157153
File zipFile = tempDir.resolve(TEST_ZIP_NAME).toFile();
158154

159155
when(filenameSanitizer.apply(anyString())).thenAnswer(i -> i.getArgument(0));
160-
when(secureTempFileManager.createSecureTempFile(anyString(), anyString())).thenReturn(zipFile);
156+
when(tempFileManager.createSecureTempFile(anyString(), anyString())).thenReturn(zipFile);
161157
doNothing().when(fileValidator).validateReadableDirectory(any(Path.class));
162158
doNothing().when(zipEntryValidator).validateEntryCount(anyInt());
163159

@@ -176,8 +172,7 @@ void zip_directory_with_empty_directory_should_succeed() throws IOException {
176172
File zipFile = tempDir.resolve(TEST_ZIP_NAME).toFile();
177173

178174
when(filenameSanitizer.apply(TEST_ZIP_NAME)).thenReturn(TEST_ZIP_NAME);
179-
when(secureTempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME)))
180-
.thenReturn(zipFile);
175+
when(tempFileManager.createSecureTempFile(anyString(), eq(TEST_ZIP_NAME))).thenReturn(zipFile);
181176
doNothing().when(fileValidator).validateReadableDirectory(any(Path.class));
182177

183178
File result = fileZipManager.zipDirectory(emptyDir.toFile(), TEST_ZIP_NAME);
@@ -192,18 +187,18 @@ void zip_directory_exceeding_entry_limit_should_throw_exception() throws IOExcep
192187
File zipFile = tempDir.resolve(TEST_ZIP_NAME).toFile();
193188

194189
when(filenameSanitizer.apply(anyString())).thenAnswer(i -> i.getArgument(0));
195-
when(secureTempFileManager.createSecureTempFile(anyString(), anyString())).thenReturn(zipFile);
190+
when(tempFileManager.createSecureTempFile(anyString(), anyString())).thenReturn(zipFile);
196191
doNothing().when(fileValidator).validateReadableDirectory(any(Path.class));
197192
doThrow(new SecurityException("Too many entries"))
198193
.when(zipEntryValidator)
199194
.validateEntryCount(anyInt());
200-
doNothing().when(secureTempFileManager).deleteTempFile(zipFile);
195+
doNothing().when(tempFileManager).deleteTempFile(zipFile);
201196

202197
assertThrows(
203198
SecurityException.class,
204199
() -> fileZipManager.zipDirectory(sourceDir.toFile(), TEST_ZIP_NAME));
205200

206-
verify(secureTempFileManager).deleteTempFile(zipFile);
201+
verify(tempFileManager).deleteTempFile(zipFile);
207202
}
208203

209204
@Test
@@ -309,7 +304,7 @@ void unzip_to_temp_directory_should_extract_to_temp_location() throws IOExceptio
309304
File zipFile = createZipWithFiles();
310305
Path tempExtractDir = tempDir.resolve("temp-extract");
311306

312-
when(secureTempFileManager.createSecureTempDirectory(anyString()))
307+
when(tempFileManager.createSecureTempDirectory(anyString()))
313308
.thenReturn(tempExtractDir.toFile());
314309
doNothing().when(fileValidator).validateReadableFile(any(Path.class));
315310
doNothing().when(fileValidator).validateWritableDirectory(any(Path.class));
@@ -325,7 +320,7 @@ void unzip_to_temp_directory_should_extract_to_temp_location() throws IOExceptio
325320
File result = fileZipManager.unzipToTempDirectory(zipFile);
326321

327322
assertNotNull(result);
328-
verify(secureTempFileManager).createSecureTempDirectory(anyString());
323+
verify(tempFileManager).createSecureTempDirectory(anyString());
329324
verify(fileValidator, times(2)).validateReadableFile(any(Path.class));
330325
}
331326

src/test/java/com/example/arinfra/file/MultipartFileConverterTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class MultipartFileConverterTest {
2222
private static final String TEST_CONTENT = "test file content";
2323
private static final String VALID_FILENAME = "document.pdf";
2424
private final TempFileCleaner cleaner = new TempFileCleaner();
25-
private final SecureTempFileManager TEMP_FILE_MANAGER = new SecureTempFileManager(cleaner);
25+
private final TempFileManager TEMP_FILE_MANAGER = new TempFileManager(cleaner);
2626
private MultipartFileConverter converter;
2727

2828
@BeforeEach

src/test/java/com/example/arinfra/file/SecureTempFileManagerTest.java renamed to src/test/java/com/example/arinfra/file/TempFileManagerTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
@Slf4j
2929
@InfraGenerated
30-
class SecureTempFileManagerTest {
30+
class TempFileManagerTest {
3131

3232
private static final String TEST_PREFIX = "test-";
3333
private static final String TEST_SUFFIX = ".txt";
@@ -41,13 +41,13 @@ class SecureTempFileManagerTest {
4141
private static final int LARGE_CONTENT_SIZE = 10_000;
4242

4343
private final TempFileCleaner tempFileCleaner = new TempFileCleaner();
44-
private SecureTempFileManager subject;
44+
private TempFileManager subject;
4545
private File createdFile;
4646

4747
@BeforeEach
4848
void setUp() {
4949

50-
subject = new SecureTempFileManager(tempFileCleaner);
50+
subject = new TempFileManager(tempFileCleaner);
5151
createdFile = null;
5252
}
5353

0 commit comments

Comments
 (0)