Pivotal ID # 185649200: FIRE Duplicated Folder Name #748
Conversation
jhoanmanuelms
commented
Sep 19, 2023
jhoanmanuelms
commented
- Preserve inner directories path in order to deal with duplicates
- Add iTest
- Preserve inner directories path in order to deal with duplicates - Add iTest
| line("File", "folder/inner") | ||
| line("Type", "inner folder") | ||
| line("File", "folder/inner/directory") | ||
| line("Type", "inner directory") |
There was a problem hiding this comment.
I think this is really specialized scenario and can be easily miss in a refactor. IMO we should have specialized test "6-3-2 submission with directories with the same name on FIRE". In that way is super clear what is that that we are testing.
| @@ -73,7 +73,7 @@ class SubmissionRequestLoader( | |||
|
|
|||
| private fun asCompressedFile(accNo: String, version: Int, directory: NfsFile): NfsFile { | |||
| fun compress(file: File): File { | |||
| val tempFolder = fireTempDirPath.resolve("$accNo/$version") | |||
| val tempFolder = fireTempDirPath.resolve("$accNo/$version/${directory.filePath.substringBeforeLast("/")}") | |||
There was a problem hiding this comment.
really subjective but I think the problem is not in this line but rather tempFolder.resolve("${file.name}.zip") the problem with that code is that we are using fixed file path rather than do that we should use Files.createTempFile which will atuomatically garantee to us that same name is not used.
There was a problem hiding this comment.
I see your point and it actually solves the problem but, this method uses the VM's temp folder which is only 10 GB currently which I think won't be enough for the imaging people's requirements
There was a problem hiding this comment.
what I mean is use the same method over the same folder fireTempDirPath.createTempFile("$accNo/$version")
There was a problem hiding this comment.
Got it, makes sense. Done!
- Use temporary file names - Improve testing structure
jhoanmanuelms
deleted the
bugfix/pivotal-#185756018-duplicated-directory-name
branch
