tanishqgandhi1908 commented on PR #7762: URL: https://github.com/apache/texera/pull/7762#issuecomment-5363961770
> The prefix fix looks right — `ResourceType.Datasets.toString == "datasets"`, so the three dataset call sites build exactly the tree they built before. > > One pre-existing issue worth folding into this PR, since it's the builder being generalized: the file-vs-directory test at [`DatasetFileNode.scala:134`](https://github.com/apache/texera/blob/e8b117c6f7f893e7755829a73c7409798003d65a/file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala#L134) decides by string equality instead of position. > > ```scala > val isFile = pathParts.last == part > ``` > > Any object whose path repeats its final segment matches early. That was hard to hit with dataset layouts, but `model/model` is a plausible one for models — and there, the intermediate `model` directory gets built as a `"file"` node carrying the object's full size, with the real leaf attached as its child. Two consequences: > > * `calculateTotalSize` counts that object twice. > * The frontend receives a node typed `file` that has children. > > `getFilePath` on the leaf still resolves correctly, so this is size/type corruption rather than a resolution failure. > > The fix is positional: > > ```scala > pathParts.zipWithIndex.foreach { case (part, idx) => > ... > val isFile = idx == pathParts.length - 1 > ``` > > (Note `pathParts.indexOf(part)` has the same bug as `last ==`.) A case with `model/model` alongside the new `models`-prefix test would lock it in. > > Minor, separate: the three new `ResourceType.Datasets` arguments could pass the existing `private val resourceType` field (`DatasetResource.scala:355`), which the sibling `logicalPath(...)` calls already use — keeps one spelling of the fact per file. Both addressed. -- 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]
