tanishqgandhi1908 commented on code in PR #7762:
URL: https://github.com/apache/texera/pull/7762#discussion_r3833086069
##########
file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala:
##########
@@ -119,23 +128,27 @@ object DatasetFileNode {
var currentPath = ""
var parentNode: DatasetFileNode = versionNode
- pathParts.foreach { part =>
- currentPath = if (currentPath.isEmpty) part else
s"$currentPath/$part"
+ pathParts.zipWithIndex.foreach {
+ case (part, idx) =>
+ currentPath = if (currentPath.isEmpty) part else
s"$currentPath/$part"
- val isFile = pathParts.last == part
- val nodeType = if (isFile) "file" else "directory"
- val fileSize = if (isFile) Some(obj.getSizeBytes.longValue()) else
None
+ // Positional, not by value: a path that repeats its final
segment (e.g.
+ // "model/model") would otherwise treat the intermediate
directory as the leaf,
+ // giving it the object's size and nesting the real file
underneath it.
+ val isFile = idx == pathParts.length - 1
+ val nodeType = if (isFile) "file" else "directory"
+ val fileSize = if (isFile) Some(obj.getSizeBytes.longValue())
else None
- val existingNode = directoryMap.get(currentPath)
+ val existingNode = directoryMap.get(currentPath)
- val node = existingNode.getOrElse {
- val newNode = new DatasetFileNode(part, nodeType, parentNode,
ownerEmail, fileSize)
- parentNode.children = Some(parentNode.getChildren :+ newNode)
- if (!isFile) directoryMap(currentPath) = newNode
- newNode
- }
+ val node = existingNode.getOrElse {
+ val newNode = new DatasetFileNode(part, nodeType, parentNode,
ownerEmail, fileSize)
+ parentNode.children = Some(parentNode.getChildren :+ newNode)
+ if (!isFile) directoryMap(currentPath) = newNode
Review Comment:
Confirmed and fixed. Wrote the test first and it failed exactly as you
described — `model` plus `model/weights.bin` produced two children named
`model`.
`directoryMap` is now `nodeMap` and registers leaves too, and on collision
the file node is promoted to a directory rather than a sibling being appended,
as you suggested.
One consequence worth naming: the promoted directory has to hold the deeper
object, so the colliding object itself isn't representable in the tree — it's
not shown and not counted (the test pins the total at 5, not 8). Preventing the
upload outright would be the fuller fix; happy to file that separately if you
think users will hit it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
