Stricter Checks on Paths in DataSources #8741
Conversation
📝 WalkthroughWalkthroughThis update standardizes file path handling across the codebase by replacing Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
fm3
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
test/e2e/End2EndSpec.scala (1)
67-68: Preferresolveover string-basedPath.ofto avoid redundanttoStringconversion
Path.of(dataDirectory.toPath.toString, "test-dataset")first converts thePathback to aString, then reparses it.
A cleaner and safer variant is to stay in thePathdomain:- Path.of(dataDirectory.toPath.toString, "test-dataset"), + dataDirectory.toPath.resolve("test-dataset"),This avoids platform-specific path parsing quirks and makes the intent explicit.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala (1)
75-75: Normalise the configured base directory onceStoring the raw
Path.of(...)may preserve stray./..segments or symlinks coming from configuration, which then propagate through everyresolvecall. Normalising it up-front avoids subtle path mismatches and simplifies debugging.- private val dataBaseDir = Path.of(config.Datastore.baseDirectory) + private val dataBaseDir = Path + .of(config.Datastore.baseDirectory) + .toAbsolutePath + .normalize()webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeTracingService.scala (1)
73-76: Prefer an explicit"."over an empty string when constructing a Path
Path.of("")yields an empty path whose meaning (CWD) is implicit and can be confusing.
A self-describing alternative keeps the intent clear and is immune to corner cases on some platforms:- private val binaryDataService = new BinaryDataService(Path.of(""), None, None, None, datasetErrorLoggingService) + // Use the current working directory as a dummy path; BinaryDataService never reads from disk here. + private val binaryDataService = new BinaryDataService(Path.of("."), None, None, None, datasetErrorLoggingService)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/ConnectomeFileService.scala (1)
255-259: PreferresolveSibling+FilenameUtils.removeExtensionfor robustness.Direct string slicing on the full path (
dropRight(connectomeFileExtension.length)) relies on the extension length matching exactly and keeps the trailing dot implicitly. A safer, OS-agnostic approach is to manipulate the filename only and keep the parent path untouched. Proposed refactor:-import java.nio.file.Path +import java.nio.file.{Path, Files} @@ - val typeNamesPath = Path.of(s"${connectomeFilePath.toString.dropRight(connectomeFileExtension.length)}json") - if (new File(typeNamesPath.toString).exists()) { + val typeNamesPath = connectomeFilePath + .resolveSibling(s"${FilenameUtils.removeExtension(connectomeFilePath.getFileName.toString)}json") + + if (Files.exists(typeNamesPath)) {Benefits:
• No manual string math
• Works even if the extension ever changes
• Avoids unnecessarynew File(...)instantiation by usingFiles.exists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
conf/messages(1 hunks)test/e2e/End2EndSpec.scala(2 hunks)util/src/main/scala/com/scalableminds/util/io/PathUtils.scala(4 hunks)util/src/main/scala/com/scalableminds/util/io/ZipIO.scala(4 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala(5 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/ExportsController.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/datavault/FileSystemDataVault.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataLayer.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataSource.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/BinaryDataServiceHolder.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/ConnectomeFileService.scala(3 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSUsedStorageService.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataSourceService.scala(4 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/AgglomerateService.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/MappingService.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/Hdf5MeshFileService.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/Hdf5SegmentIndexFileService.scala(1 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/SegmentIndexFileService.scala(2 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala(5 hunks)webknossos-datastore/app/com/scalableminds/webknossos/datastore/storage/RemoteSourceDescriptorService.scala(6 hunks)webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/files/TempFileService.scala(2 hunks)webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingService.scala(2 hunks)webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeTracingService.scala(2 hunks)
🧰 Additional context used
🧠 Learnings (14)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/datavault/FileSystemDataVault.scala (2)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeTracingService.scala (2)
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.
Learnt from: frcroth
PR: scalableminds/webknossos#8202
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DatasetErrorLoggingService.scala:68-72
Timestamp: 2024-11-25T10:02:03.702Z
Learning: In `DatasetErrorLoggingService.scala`, prefer using `TextUtils.stackTraceAsString(exception)` when logging exceptions instead of passing the exception directly to `logger.error`.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSUsedStorageService.scala (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/ConnectomeFileService.scala (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/AgglomerateService.scala (2)
Learnt from: frcroth
PR: scalableminds/webknossos#8202
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DatasetErrorLoggingService.scala:68-72
Timestamp: 2024-11-25T10:02:03.702Z
Learning: In `DatasetErrorLoggingService.scala`, prefer using `TextUtils.stackTraceAsString(exception)` when logging exceptions instead of passing the exception directly to `logger.error`.
Learnt from: frcroth
PR: scalableminds/webknossos#8598
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DatasetLayerAttachments.scala:89-95
Timestamp: 2025-06-02T09:49:51.047Z
Learning: In WebKnossos dataset layer attachments, multiple file types can safely use the same directory name (like "agglomerates") because the scanning logic filters by file extension. For example, AgglomerateFileInfo scans for .hdf5 files while CumsumFileInfo scans for .json files in the same "agglomerates" directory without interference.
util/src/main/scala/com/scalableminds/util/io/ZipIO.scala (2)
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingService.scala (3)
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshService.scala:55-66
Timestamp: 2025-04-23T09:22:26.829Z
Learning: In the NeuroglancerMesh class, shardingSpecification is defined as a concrete ShardingSpecification value, not an Option. It uses meshInfo.sharding.getOrElse(ShardingSpecification.empty) to provide a default empty specification if none is present, ensuring that mesh.shardingSpecification is never null.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshService.scala:55-66
Timestamp: 2025-04-23T09:22:26.829Z
Learning: In the NeuroglancerMesh class, shardingSpecification is defined as a concrete ShardingSpecification value, not an Option. It uses meshInfo.sharding.getOrElse(ShardingSpecification.empty) to provide a default empty specification if none is present, ensuring that mesh.shardingSpecification is never null.
Learnt from: frcroth
PR: scalableminds/webknossos#8202
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DatasetErrorLoggingService.scala:68-72
Timestamp: 2024-11-25T10:02:03.702Z
Learning: In `DatasetErrorLoggingService.scala`, prefer using `TextUtils.stackTraceAsString(exception)` when logging exceptions instead of passing the exception directly to `logger.error`.
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/files/TempFileService.scala (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
conf/messages (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8393
File: app/controllers/AuthenticationController.scala:566-573
Timestamp: 2025-03-21T16:49:17.956Z
Learning: In the webknossos project, prefer using the ?~> operator for error handling with message keys stored in conf/messages rather than extensive custom error handling.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (1)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataLayer.scala (1)
Learnt from: frcroth
PR: scalableminds/webknossos#8609
File: app/models/dataset/Dataset.scala:753-775
Timestamp: 2025-05-12T13:07:29.637Z
Learning: In the `updateMags` method of DatasetMagsDAO (Scala), the code handles different dataset types distinctly:
1. Non-WKW datasets have `magsOpt` populated and use the first branch which includes axisOrder, channelIndex, and credentialId.
2. WKW datasets will have `wkwResolutionsOpt` populated and use the second branch which includes cubeLength.
3. The final branch is a fallback for legacy data.
This ensures appropriate fields are populated for each dataset type.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala (3)
Learnt from: frcroth
PR: scalableminds/webknossos#8609
File: app/models/dataset/Dataset.scala:753-775
Timestamp: 2025-05-12T13:07:29.637Z
Learning: In the `updateMags` method of DatasetMagsDAO (Scala), the code handles different dataset types distinctly:
1. Non-WKW datasets have `magsOpt` populated and use the first branch which includes axisOrder, channelIndex, and credentialId.
2. WKW datasets will have `wkwResolutionsOpt` populated and use the second branch which includes cubeLength.
3. The final branch is a fallback for legacy data.
This ensures appropriate fields are populated for each dataset type.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/storage/RemoteSourceDescriptorService.scala (2)
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataSourceService.scala (3)
Learnt from: frcroth
PR: scalableminds/webknossos#8609
File: app/models/dataset/Dataset.scala:753-775
Timestamp: 2025-05-12T13:07:29.637Z
Learning: In the `updateMags` method of DatasetMagsDAO (Scala), the code handles different dataset types distinctly:
1. Non-WKW datasets have `magsOpt` populated and use the first branch which includes axisOrder, channelIndex, and credentialId.
2. WKW datasets will have `wkwResolutionsOpt` populated and use the second branch which includes cubeLength.
3. The final branch is a fallback for legacy data.
This ensures appropriate fields are populated for each dataset type.
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8352
File: app/models/organization/CreditTransactionService.scala:0-0
Timestamp: 2025-01-27T12:06:42.865Z
Learning: In Scala's for-comprehension with Fox (Future-like type), the `<-` operator ensures sequential execution. If any step fails, the entire chain short-circuits and returns early, preventing subsequent operations from executing. This makes it safe to perform validation checks before database operations.
Learnt from: frcroth
PR: scalableminds/webknossos#8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
🧬 Code Graph Analysis (9)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/datavault/FileSystemDataVault.scala (1)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DatasetLayerAttachments.scala (1)
localPath(43-49)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/Hdf5SegmentIndexFileService.scala (2)
vitest.config.ts (1)
config(9-25)webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (1)
dataBaseDir(158-158)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/SegmentIndexFileService.scala (1)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (1)
dataBaseDir(158-158)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/ConnectomeFileService.scala (1)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/DataStoreConfig.scala (1)
Datastore(17-64)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/AgglomerateService.scala (2)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (1)
dataBaseDir(158-158)vitest.config.ts (1)
config(9-25)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/ExportsController.scala (2)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (1)
dataBaseDir(158-158)webknossos-datastore/app/com/scalableminds/webknossos/datastore/DataStoreConfig.scala (1)
Datastore(17-64)
util/src/main/scala/com/scalableminds/util/io/ZipIO.scala (2)
util/src/main/scala/com/scalableminds/util/io/PathUtils.scala (5)
commonPrefix(41-46)commonPrefix(48-51)PathUtils(14-14)cutOffPathAtLastOccurrenceOf(152-168)removeSingleFileNameFromPrefix(171-179)util/src/main/scala/com/scalableminds/util/tools/Fox.scala (7)
map(265-268)map(361-361)empty(56-56)Fox(30-223)Fox(225-289)successful(53-56)shiftBox(296-296)
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/files/TempFileService.scala (2)
app/files/WkTempFileService.scala (1)
moduleName(12-12)webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/files/TsTempFileService.scala (1)
moduleName(10-10)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/storage/RemoteSourceDescriptorService.scala (4)
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DatasetLayerAttachments.scala (1)
localPath(43-49)util/src/main/scala/com/scalableminds/util/tools/Fox.scala (3)
flatMap(268-278)flatMap(363-363)toFox(14-14)webknossos-datastore/app/com/scalableminds/webknossos/datastore/storage/DataVaultService.scala (2)
DataVaultService(22-31)DataVaultService(33-73)webknossos-datastore/app/com/scalableminds/webknossos/datastore/DataStoreConfig.scala (1)
Datastore(17-64)
🔇 Additional comments (43)
test/e2e/End2EndSpec.scala (1)
13-14: Switch toPathimport looks goodMigrating from
PathstoPathkeeps the tests aligned with the production change and removes the deprecated/verbose call sites. No issues here.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/BinaryDataServiceHolder.scala (2)
3-3: Confirm build targets Java 11+ before merging
Path.ofwas introduced in Java 11. If any build, CI, or runtime environment still targets Java 8, this change will break compilation. Double-check yourbuild.sbt/ Docker images / CI matrix and bump the tool-chain if necessary.
26-26: No functional change – looks good
Switching fromPaths.gettoPath.ofkeeps identical semantics for a single string argument, so there’s no behavioral risk here.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/Hdf5SegmentIndexFileService.scala (2)
10-10: Import update aligns with new path-handling policy – looks good.
The switch tojava.nio.file.Pathis consistent with the project-wide migration and removes the deprecatedPathsutil.
16-16:Path.ofrequires JDK 11+ – confirm build/runtime baseline.
Path.ofisn’t available on JDK 8. Verify that the CI and production runtime already target ≥ 11; otherwise, this will fail at compile time.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/segmentindex/SegmentIndexFileService.scala (2)
29-29: Import cleanup LGTM
java.nio.file.Pathis now the only symbol required after droppingPaths.get, so the revised import is correct and avoids the unusedPathsimport.
44-44: Verify JDK 11+ availability forPath.of
Path.ofwas introduced in Java 11. If any build/test nodes are still on Java 8, this will fail to compile.
Confirm all build environments (CI and production) target JDK 11 or newer, or revert toPaths.getuntil the upgrade is complete.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DSUsedStorageService.scala (2)
13-13: LGTM: Import cleanup aligns with modernization.Removing the unused
Pathsimport after migrating toPath.ofis consistent with the codebase modernization effort.
33-33: LGTM: Modernized path creation using Path.of.The replacement of
Paths.get()withPath.of()follows Java 11+ best practices while maintaining identical functionality. This change aligns with the broader codebase modernization described in the PR objectives.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala (1)
22-22: Ensure build is on JDK 11+
Path.ofonly exists since Java 11. Confirm that the CI image, local dev setup and production runtime have all been upgraded from the previous JDK 8 baseline; otherwise this will break compilation.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/MappingService.scala (2)
10-10: Confirm JDK 11+ target due toPath.ofusage
Path.ofis only available from JDK 11 onwards. Please make sure the build toolchain and CI pipelines compile against 11+; otherwise this will fail to compile.
22-24: Path instantiation looks good – verifybaseDirectorytypeThe switch to
Path.of(config.Datastore.baseDirectory)is correct and consistent with the migration away fromPaths.get.
Just ensurebaseDirectoryis aString; if it’s already aPath, this will not compile (noPath.of(Path)overload).webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeTracingService.scala (1)
33-33: UnusedPathsimport removed – good catch.Import scope is now minimal and consistent with the new
Path.ofidiom.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/ConnectomeFileService.scala (2)
4-4: Migration tojava.nio.file.Pathlooks good.Switching from
Paths.gettoPath.ofis idiomatic since Java 11 and avoids the extraPathsimport. No further action required.
87-87: Path construction change is behaviour-preserving.
Path.of(config.Datastore.baseDirectory)is a 1-to-1 replacement for the previousPaths.get(...)call, preserving relative/absolute semantics while staying consistent with the new style.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/Hdf5MeshFileService.scala (2)
12-12: Import switch tojava.nio.file.Pathis fineThe change cleanly removes the
java.nio.file.Pathsdependency and aligns with the newPath.ofusage elsewhere. No issues spotted.
20-21: Verify Java 11+ ToolchainIt looks like we couldn’t find any explicit Java source/target levels or
--releaseflags in your build definitions (e.g.build.sbt,pom.xml). SincePath.ofis only available on JDK 11 and above, please double-check that:
- Your sbt settings (e.g.
javacOptions,javaHome) target Java 11+.- Any Docker images or CI pipelines are using a JDK ≥ 11.
JAVA_HOMEin your local and CI environments points to Java 11+.webknossos-datastore/app/com/scalableminds/webknossos/datastore/datavault/FileSystemDataVault.scala (1)
11-11: LGTM: Clean refactoring to standardize path handlingThe changes correctly replace
Paths.getwithPath.ofand remove the unused import. This is functionally equivalent and aligns with the codebase-wide standardization effort.Also applies to: 99-99
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mapping/AgglomerateService.scala (1)
24-24: LGTM: Consistent path handling standardizationThe import cleanup and
Path.ofusage are correct and align with the broader codebase refactoring. ThedataBaseDirfield maintains its functionality while using the more modern API.Also applies to: 37-37
util/src/main/scala/com/scalableminds/util/io/PathUtils.scala (1)
4-4: LGTM: Comprehensive path handling standardizationAll instances of
Paths.gethave been correctly replaced withPath.of. The wildcard import is appropriate for the utility class, and the changes maintain identical functionality across all the path manipulation methods.Also applies to: 45-45, 165-165, 172-172, 183-183
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/files/TempFileService.scala (1)
10-10: LGTM: Correct temporary directory path constructionThe refactoring correctly updates the temporary directory path initialization to use
Path.ofinstead ofPaths.get. The functionality remains identical while following the standardization pattern.Also applies to: 24-24
webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/ExportsController.scala (1)
3-3: LGTM: Completes the path handling standardizationThe import cleanup and
Path.ofusage fordataBaseDirinitialization are correct. This change maintains the same export functionality while following the consistent refactoring pattern across the codebase.Also applies to: 33-33
util/src/main/scala/com/scalableminds/util/io/ZipIO.scala (4)
4-4: LGTM: Import cleanup aligns with Path.of migrationRemoving the unused
Pathsimport after migrating toPath.ofis appropriate and maintains clean dependencies.
181-181: LGTM: Consistent migration from Paths.get to Path.ofThe standardization from
Paths.gettoPath.ofis correctly applied across all zip entry name processing. Both methods have identical semantics, and this change aligns with the PR's objective to modernize path handling throughout the codebase.Also applies to: 197-197, 233-233, 251-251
185-185: LGTM: Path.of migration in common prefix computationThe replacement maintains the existing logic for computing common prefixes from zip entry names. The change is consistent with the broader standardization effort.
Also applies to: 237-237
190-190: LGTM: Empty path creation standardizedUsing
Path.of("")instead ofPaths.get("")for creating empty paths is semantically equivalent and follows the modernization pattern.Also applies to: 242-242
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataSource.scala (1)
68-68: LGTM: Clean aggregation method for explicit pathsThe
allExplicitPathsmethod provides a clean way to collect all explicit paths from data layers usingflatMap. This supports the path validation objectives mentioned in the PR and follows functional programming best practices.conf/messages (1)
129-130: LGTM: Clear error messages for path validationThe new error messages provide clear, actionable feedback for path validation scenarios:
- Line 129 addresses forbidden paths in datasource addition
- Line 130 guides users to use compose functionality for adding new paths
Both messages follow existing naming conventions and align with the PR's stricter path validation objectives.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/models/datasource/DataLayer.scala (1)
278-280: LGTM: Comprehensive path aggregation implementationThe
allExplicitPathsmethod correctly aggregates paths from multiple sources:
- Handles both mag-based and WKW resolution-based layers using the
orElsepattern- Includes attachment paths by converting
Pathobjects to strings- Uses functional programming idioms (
flatMap,++) appropriately- Provides fallback to empty sequence when no paths are present
This implementation supports the broader path validation framework introduced in this PR.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/uploading/UploadService.scala (4)
25-25: LGTM!The dependency injection of
RemoteSourceDescriptorServiceis properly implemented.Also applies to: 116-116
389-391: Good addition of path validation for EXPLORED datasources.The change from no-op to path validation ensures that uploaded datasources with explicit paths are properly validated, aligning with the PR's objective of stricter path checks.
403-412: Well-implemented path validation method.The method correctly validates explicit paths in uploaded datasources using proper Fox monad composition and error handling.
510-512: Correct use of preventNewPaths parameter.Setting
preventNewPaths = falseis appropriate here as this is part of the upload/add flow where new paths should be allowed after validation.webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/DataSourceService.scala (3)
49-52: Good modernization to Path.of.Replacing
Paths.getwithPath.ofaligns with modern Java NIO practices.Also applies to: 141-141
272-272: Well-designed path restriction logic.The
preventNewPathsparameter correctly enforces path restrictions only during updates when explicitly requested.Also applies to: 280-280
287-303: Robust implementation of path validation.The method correctly validates that no new explicit paths are introduced during updates, with proper handling of edge cases and efficient set-based comparison.
webknossos-datastore/app/com/scalableminds/webknossos/datastore/controllers/DataSourceController.scala (3)
25-25: Proper dependency injection.The
RemoteSourceDescriptorServiceis correctly injected for path validation.Also applies to: 53-53
395-397: Effective pre-validation of explicit paths.The validation correctly ensures all explicit paths are allowed before proceeding with datasource creation, providing early failure with a clear error message.
383-385: Correct enforcement of path restrictions.The
preventNewPathsparameter is properly set totruefor updates (preventing new paths) andfalsefor additions (allowing new paths), aligning with the PR's objectives.Also applies to: 413-415
webknossos-datastore/app/com/scalableminds/webknossos/datastore/storage/RemoteSourceDescriptorService.scala (3)
14-14: Consistent modernization to Path.of.All
Paths.getusages properly replaced withPath.of.Also applies to: 71-71, 117-117, 164-164
129-130: Good refactoring to simplify credential lookup.Changing
findGlobalCredentialForto returnOptioninstead ofFoxis appropriate since it's a synchronous operation. The conversion toFoxat call sites maintains compatibility.Also applies to: 139-139, 148-148
151-176: Well-designed path validation logic.The path validation methods provide comprehensive security checks:
- Prevents path traversal attacks in
pathIsDataSourceLocal- Ensures remote paths don't use global credentials directly
- Allows whitelisted local paths for flexibility
The separation of concerns into focused helper methods makes the logic clear and maintainable.
webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingService.scala (1)
44-44: Consistent switch tojava.nio.file.Pathlooks good
Migration away fromPathsis in line with the new guidelines across the PR. No further action required.
.../scalableminds/webknossos/tracingstore/tracings/editablemapping/EditableMappingService.scala
Show resolved
Hide resolved
Steps to test:
Issues: