This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6502-71faf440de7936d07451d214dbd5777e6906b949 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 321fe45505ecadf3aba184c2caee2b948c0d86dc Author: Tanishq Gandhi <[email protected]> AuthorDate: Wed Aug 12 22:47:36 2026 +0000 feat(storage): add datasets resource-type prefix to logical paths (#6502) ### What changes were proposed in this PR? Adds an explicit resource-type prefix to asset logical file paths, changing the format from `/<owner>/<name>/<version>/<file>` to `/datasets/<owner>/<name>/<version>/<file>`. Makes the `datasets` resource-type prefix **required** on dataset logical file paths (`/datasets/<owner>/<name>/<version>/<file>`), so other resource types (e.g. models) can be told apart by the prefix and routed to their own table. Unlike the initial approach, an **unprefixed path no longer resolves** — the prefix is what selects the resource's table. - **Path resolver (`FileResolver`):** a dataset path must start with `datasets`; the previous "parse unprefixed as-is" fallback is removed. - **Python file API (`DatasetFileDocument`):** same rule, mirroring the backend. - **File Lister operator:** parses the now-prefixed `datasetVersionPath` (the second dataset-path property, alongside scan sources' `fileName`). - **File tree / frontend:** tree rooted at a `datasets` node; the selection modal emits prefixed paths; relative-path extraction strips the 4-segment prefix; unused client-side parser removed. - **Cover images (`DatasetResource`):** cover-image handlers now build prefixed paths. - **Example workflows:** updated to prefixed paths. - **Migration (`sql/updates/36.sql`):** one-time, Liquibase-run rewrite that prepends `datasets/` to legacy paths in `workflow.content` and `workflow_version.content` (both `fileName` and `datasetVersionPath`). Only values whose first two segments match an existing `(user.email, dataset.name)` are rewritten (local paths/URLs untouched; email format is irrelevant); idempotent. **Known migration limits:** - A path whose dataset was **renamed or deleted** since the workflow was saved won't match `(email, name)`, so it stays unprefixed and will fail to resolve (it was already unusable). - **Hardcoded paths inside user code are not migrated (breaking).** The migration rewrites only the `fileName` and `datasetVersionPath` operator properties, so a path written by hand inside a Python UDF — e.g. `DatasetFileDocument("/[email protected]/ds/v1/f.csv")`, stored in the operator's `code` property — is left untouched and now raises `ValueError: Invalid file path format. Expected: /datasets/ownerEmail/datasetName/versionName/fileRelativePath`. Unlike the renamed/deleted case above, these paths **were working before this change**. Users must add the `datasets/` prefix in their UDF code; the error message states the expected format. Rewriting arbitrary user source in a SQL migration would risk corrupting code, so this is documented rather than automated — it needs a release note. - The migration assumes `content` is valid JSON (an app invariant) and aborts on a malformed row rather than skipping, so a bad row rolls the whole migration back instead of applying partially. ### Any related issues, documentation, discussions? Closes #6495. ### How was this PR tested? New and updated unit tests, all passing locally: - `FileResolverSpec`: an unprefixed path (and an unknown resource-type prefix) no longer resolves; a valid prefixed path resolves; too-few-segments is rejected. - `FileListerSourceOpExecSpec` (new): a prefixed `datasetVersionPath` parses; unprefixed / unknown-resource-type / too-few rejected. - Frontend: `datasetVersionFileTree` and `dataset-selection-modal` specs updated for the prefix. - Python: `test_dataset_file_document.py` — prefix required, presign re-emits it. The migration (`36.sql`) was verified manually against sample data: unprefixed→prefixed; already-prefixed left unchanged (idempotent); local paths/URLs untouched; dangling (renamed/deleted) datasets untouched; operators without the property get no spurious key added; both `fileName` and `datasetVersionPath` covered. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --------- Co-authored-by: ali <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../pytexera/storage/dataset_file_document.py | 30 ++- .../main/python/pytexera/storage/resource_type.py | 24 +++ .../pytexera/storage/test_dataset_file_document.py | 51 ++++-- .../workflow/WorkflowExecutionsResourceSpec.scala | 4 +- ...xample] Data Exploration on Movies Dataset.json | 2 +- ...[Example] Machine Learning on Iris Dataset.json | 2 +- .../texera/amber/core/storage/FileResolver.scala | 25 ++- .../texera/amber/core/storage/ResourceType.scala | 32 ++++ .../texera/amber/storage/FileResolverSpec.scala | 42 ++++- .../source/dataset/FileListerSourceOpExec.scala | 42 ++++- .../dataset/FileListerSourceOpExecSpec.scala | 91 +++++++++ .../texera/service/resource/DatasetResource.scala | 47 ++++- .../service/type/dataset/DatasetFileNode.scala | 15 +- .../service/type/dataset/DatasetFileNodeSpec.scala | 11 +- frontend/src/app/common/type/dataset-file.spec.ts | 85 --------- frontend/src/app/common/type/dataset-file.ts | 60 ------ .../app/common/type/datasetVersionFileTree.spec.ts | 50 +++-- .../src/app/common/type/datasetVersionFileTree.ts | 11 +- frontend/src/app/common/type/resource-type.ts | 24 +++ ...user-dataset-version-filetree.component.spec.ts | 4 +- .../dataset-selection-modal.component.spec.ts | 4 +- .../dataset-selection-modal.component.ts | 6 +- sql/changelog.xml | 5 + sql/updates/36.sql | 203 +++++++++++++++++++++ 24 files changed, 628 insertions(+), 242 deletions(-) diff --git a/amber/src/main/python/pytexera/storage/dataset_file_document.py b/amber/src/main/python/pytexera/storage/dataset_file_document.py index 5a063f6047..82c1c1ae91 100644 --- a/amber/src/main/python/pytexera/storage/dataset_file_document.py +++ b/amber/src/main/python/pytexera/storage/dataset_file_document.py @@ -22,6 +22,8 @@ import urllib.parse from requests.adapters import HTTPAdapter from urllib3.util.retry import Retry +from .resource_type import ResourceType + class DatasetFileDocument: # (connect, read) timeout and retry settings for the file-service GETs below. @@ -56,20 +58,29 @@ class DatasetFileDocument: Parses the file path into dataset metadata. :param file_path: - Expected format - "/ownerEmail/datasetName/versionName/fileRelativePath" - Example: "/[email protected]/twitterDataset/v1/california/irvine/tw1.csv" + Expected format - + "/datasets/ownerEmail/datasetName/versionName/fileRelativePath" + Example: + "/datasets/[email protected]/twitterDataset/v1/california/tw1.csv" """ parts = file_path.strip("/").split("/") - if len(parts) < 4: + + if len(parts) < 5: raise ValueError( - "Invalid file path format. " - "Expected: /ownerEmail/datasetName/versionName/fileRelativePath" + "Invalid file path format. Expected: " + "/datasets/ownerEmail/datasetName/versionName/fileRelativePath" ) - self.owner_email = parts[0] - self.dataset_name = parts[1] - self.version_name = parts[2] - self.file_relative_path = "/".join(parts[3:]) + # Validate the leading prefix against the known resource types. + try: + self.resource_type = ResourceType(parts[0]) + except ValueError: + raise ValueError(f"Unknown resource type prefix: {parts[0]!r}") + + self.owner_email = parts[1] + self.dataset_name = parts[2] + self.version_name = parts[3] + self.file_relative_path = "/".join(parts[4:]) self.jwt_token = os.getenv("USER_JWT_TOKEN") self.presign_endpoint = os.getenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT") @@ -90,6 +101,7 @@ class DatasetFileDocument: """ headers = {"Authorization": f"Bearer {self.jwt_token}"} encoded_file_path = urllib.parse.quote( + f"/{self.resource_type.value}" f"/{self.owner_email}" f"/{self.dataset_name}" f"/{self.version_name}" diff --git a/amber/src/main/python/pytexera/storage/resource_type.py b/amber/src/main/python/pytexera/storage/resource_type.py new file mode 100644 index 0000000000..596fb5d745 --- /dev/null +++ b/amber/src/main/python/pytexera/storage/resource_type.py @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from enum import Enum + + +class ResourceType(str, Enum): + """The leading segment of a logical file path, identifying the resource kind""" + + DATASETS = "datasets" diff --git a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py index 36882fe27e..284a75fbfe 100644 --- a/amber/src/test/python/pytexera/storage/test_dataset_file_document.py +++ b/amber/src/test/python/pytexera/storage/test_dataset_file_document.py @@ -44,51 +44,61 @@ def make_response(status_code: int, body=None, content: bytes = b""): class TestDatasetFileDocumentInit: - def test_parses_minimal_four_part_path(self, auth_env): - doc = DatasetFileDocument("/[email protected]/ds/v1/file.csv") + def test_parses_prefixed_path(self, auth_env): + doc = DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") assert doc.owner_email == "[email protected]" assert doc.dataset_name == "ds" assert doc.version_name == "v1" assert doc.file_relative_path == "file.csv" def test_joins_nested_relative_path_back_with_slashes(self, auth_env): - doc = DatasetFileDocument("/[email protected]/ds/v1/a/b/c/file.csv") + doc = DatasetFileDocument("/datasets/[email protected]/ds/v1/a/b/c/file.csv") assert doc.file_relative_path == "a/b/c/file.csv" def test_strips_leading_and_trailing_slashes_before_parsing(self, auth_env): - doc = DatasetFileDocument("///[email protected]/ds/v1/file.csv///") + doc = DatasetFileDocument("///datasets/[email protected]/ds/v1/file.csv///") assert doc.owner_email == "[email protected]" assert doc.file_relative_path == "file.csv" - def test_rejects_path_with_fewer_than_four_segments(self, auth_env): + def test_rejects_unprefixed_path(self, auth_env): + # Without the datasets prefix the path is not a dataset path. with pytest.raises(ValueError, match="Invalid file path format"): - DatasetFileDocument("/[email protected]/ds/v1") + DatasetFileDocument("/[email protected]/ds/v1/file.csv") + + def test_rejects_unknown_resource_type_prefix(self, auth_env): + # A leading segment that is not a known resource type is rejected. + with pytest.raises(ValueError, match="Unknown resource type prefix"): + DatasetFileDocument("/notAResourceType/[email protected]/ds/v1/file.csv") + + def test_rejects_path_with_too_few_segments(self, auth_env): + with pytest.raises(ValueError, match="Invalid file path format"): + DatasetFileDocument("/datasets/[email protected]/ds/v1") def test_requires_jwt_token_in_environment(self, monkeypatch): monkeypatch.delenv("USER_JWT_TOKEN", raising=False) monkeypatch.setenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT", CUSTOM_ENDPOINT) with pytest.raises(ValueError, match="JWT token is required"): - DatasetFileDocument("/[email protected]/ds/v1/file.csv") + DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") def test_treats_empty_jwt_as_missing(self, monkeypatch): # An empty string is falsy and should be rejected just like an unset var. monkeypatch.setenv("USER_JWT_TOKEN", "") with pytest.raises(ValueError, match="JWT token is required"): - DatasetFileDocument("/[email protected]/ds/v1/file.csv") + DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") def test_falls_back_to_default_endpoint_when_env_missing(self, monkeypatch): monkeypatch.setenv("USER_JWT_TOKEN", "tok") monkeypatch.delenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT", raising=False) - doc = DatasetFileDocument("/[email protected]/ds/v1/file.csv") + doc = DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") assert doc.presign_endpoint == DEFAULT_ENDPOINT def test_uses_explicit_endpoint_from_environment(self, auth_env): - doc = DatasetFileDocument("/[email protected]/ds/v1/file.csv") + doc = DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") assert doc.presign_endpoint == CUSTOM_ENDPOINT class TestGetPresignedUrl: - def _make_doc(self, monkeypatch, path="/[email protected]/ds/v1/file.csv"): + def _make_doc(self, monkeypatch, path="/datasets/[email protected]/ds/v1/file.csv"): monkeypatch.setenv("USER_JWT_TOKEN", "test-jwt-token") monkeypatch.setenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT", CUSTOM_ENDPOINT) return DatasetFileDocument(path) @@ -116,7 +126,9 @@ class TestGetPresignedUrl: def test_url_encodes_filepath_query_parameter(self, monkeypatch): # urllib.parse.quote keeps "/" as safe by default, but encodes "@" # and " " — pin both pieces so the contract is explicit. - doc = self._make_doc(monkeypatch, path="/[email protected]/ds/v1/data file.csv") + doc = self._make_doc( + monkeypatch, path="/datasets/[email protected]/ds/v1/data file.csv" + ) with patch( "pytexera.storage.dataset_file_document.requests.Session.get" ) as mock_get: @@ -128,6 +140,17 @@ class TestGetPresignedUrl: assert "bob%40x.com" in file_path assert file_path.startswith("/") + def test_sends_datasets_prefixed_filepath(self, monkeypatch): + # The reconstructed filePath sent to the file-service carries the "datasets" prefix. + doc = self._make_doc(monkeypatch, path="/datasets/[email protected]/ds/v1/file.csv") + with patch( + "pytexera.storage.dataset_file_document.requests.Session.get" + ) as mock_get: + mock_get.return_value = make_response(200, body={"presignedUrl": "u"}) + doc.get_presigned_url() + _, kwargs = mock_get.call_args + assert kwargs["params"]["filePath"].startswith("/datasets/") + def test_calls_configured_endpoint(self, monkeypatch): doc = self._make_doc(monkeypatch) with patch( @@ -192,7 +215,7 @@ class TestReadFile: def _make_doc(self, monkeypatch): monkeypatch.setenv("USER_JWT_TOKEN", "test-jwt-token") monkeypatch.setenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT", CUSTOM_ENDPOINT) - return DatasetFileDocument("/[email protected]/ds/v1/file.csv") + return DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") def test_returns_bytesio_with_downloaded_content(self, monkeypatch): doc = self._make_doc(monkeypatch) @@ -246,7 +269,7 @@ class TestTimeoutsAndRetries: def _make_doc(self, monkeypatch): monkeypatch.setenv("USER_JWT_TOKEN", "test-jwt-token") monkeypatch.setenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT", CUSTOM_ENDPOINT) - return DatasetFileDocument("/[email protected]/ds/v1/file.csv") + return DatasetFileDocument("/datasets/[email protected]/ds/v1/file.csv") def test_presigned_url_request_passes_request_timeout(self, monkeypatch): doc = self._make_doc(monkeypatch) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala index af8c24f05a..3739f4ebde 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala @@ -804,7 +804,7 @@ class WorkflowExecutionsResourceSpec val content = """{ | "operators": [ - | {"operatorID": "scanA", "operatorProperties": {"fileName": "/[email protected]/LockedDS/v1/data.csv"}}, + | {"operatorID": "scanA", "operatorProperties": {"fileName": "/datasets/[email protected]/LockedDS/v1/data.csv"}}, | {"operatorID": "downstreamB", "operatorProperties": {}} | ], | "links": [ @@ -863,7 +863,7 @@ class WorkflowExecutionsResourceSpec val content = """{ | "operators": [ - | {"operatorID": "scan", "operatorProperties": {"fileName": "/[email protected]/MyDS/v1/data.csv"}} + | {"operatorID": "scan", "operatorProperties": {"fileName": "/datasets/[email protected]/MyDS/v1/data.csv"}} | ], | "links": [] |}""".stripMargin diff --git a/bin/single-node/examples/workflows/[Example] Data Exploration on Movies Dataset.json b/bin/single-node/examples/workflows/[Example] Data Exploration on Movies Dataset.json index 3e9f99c613..cdc9a0e870 100644 --- a/bin/single-node/examples/workflows/[Example] Data Exploration on Movies Dataset.json +++ b/bin/single-node/examples/workflows/[Example] Data Exploration on Movies Dataset.json @@ -8,7 +8,7 @@ "fileEncoding": "UTF_8", "customDelimiter": ",", "hasHeader": true, - "fileName": "/texera/popular-movies-of-imdb/v1/TMDb_updated.csv", + "fileName": "/datasets/texera/popular-movies-of-imdb/v1/TMDb_updated.csv", "offset": 0, "limit": 1000 }, diff --git a/bin/single-node/examples/workflows/[Example] Machine Learning on Iris Dataset.json b/bin/single-node/examples/workflows/[Example] Machine Learning on Iris Dataset.json index d36d35dae5..7e9a5d11fd 100644 --- a/bin/single-node/examples/workflows/[Example] Machine Learning on Iris Dataset.json +++ b/bin/single-node/examples/workflows/[Example] Machine Learning on Iris Dataset.json @@ -8,7 +8,7 @@ "fileEncoding": "UTF_8", "customDelimiter": ",", "hasHeader": true, - "fileName": "/texera/iris-species/v1/Iris.csv" + "fileName": "/datasets/texera/iris-species/v1/Iris.csv" }, "inputPorts": [], "outputPorts": [ diff --git a/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala b/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala index c8a407df99..88df489991 100644 --- a/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala +++ b/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/FileResolver.scala @@ -76,8 +76,8 @@ object FileResolver { } /** - * Parses a dataset file path and extracts its components. - * Expected format: /ownerEmail/datasetName/versionName/fileRelativePath + * Parses a dataset logical path into its components, or None if it is not a well-formed dataset path. + * Expected format: /datasets/ownerEmail/datasetName/versionName/fileRelativePath * * @param fileName The file path to parse * @return Some((ownerEmail, datasetName, versionName, fileRelativePath)) if valid, None otherwise @@ -88,14 +88,14 @@ object FileResolver { val filePath = Paths.get(fileName) val pathSegments = (0 until filePath.getNameCount).map(filePath.getName(_).toString).toArray - if (pathSegments.length < 4) { + if (pathSegments.length < 5 || !ResourceType.isValidPrefix(pathSegments(0))) { return None } - val ownerEmail = pathSegments(0) - val datasetName = pathSegments(1) - val versionName = pathSegments(2) - val fileRelativePathSegments = pathSegments.drop(3) + val ownerEmail = pathSegments(1) + val datasetName = pathSegments(2) + val versionName = pathSegments(3) + val fileRelativePathSegments = pathSegments.drop(4) Some((ownerEmail, datasetName, versionName, fileRelativePathSegments)) } @@ -103,8 +103,8 @@ object FileResolver { /** * Attempts to resolve a given fileName to a URI. * - * The fileName format should be: /ownerEmail/datasetName/versionName/fileRelativePath - * e.g. /[email protected]/twitterDataset/v1/california/irvine/tw1.csv + * The fileName format should be: /datasets/ownerEmail/datasetName/versionName/fileRelativePath + * e.g. /datasets/[email protected]/twitterDataset/v1/california/irvine/tw1.csv * The output dataset URI format is: {DATASET_FILE_URI_SCHEME}:///{repositoryName}/{versionHash}/fileRelativePath * e.g. {DATASET_FILE_URI_SCHEME}:///dataset-15/adeq233td/some/dir/file.txt * @@ -194,11 +194,8 @@ object FileResolver { } /** - * Parses a dataset file path to extract owner email and dataset name. - * Expected format: /ownerEmail/datasetName/versionName/fileRelativePath - * - * @param path The file path from operator properties - * @return Some((ownerEmail, datasetName)) if path is valid, None otherwise + * Extracts the owner email and dataset name from a dataset logical path, + * or None if it is not a well-formed dataset path. */ def parseDatasetOwnerAndName(path: String): Option[(String, String)] = { if (path == null) { diff --git a/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/ResourceType.scala b/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/ResourceType.scala new file mode 100644 index 0000000000..cb3db537d5 --- /dev/null +++ b/common/workflow-core/src/main/scala/org/apache/texera/amber/core/storage/ResourceType.scala @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.core.storage + +/** + * The leading segment of a logical file path, identifying which resource kind (and thus which + * backing table) a path belongs to. + * + * Path shape: /<prefix>/ownerEmail/resourceName/versionName/fileRelativePath + */ +object ResourceType extends Enumeration { + val Datasets: Value = Value("datasets") + + def isValidPrefix(segment: String): Boolean = values.exists(_.toString == segment) +} diff --git a/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala b/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala index 7056ee5304..921ce0052c 100644 --- a/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala +++ b/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala @@ -78,9 +78,16 @@ class FileResolverSpec private val localCsvFilePath = "common/workflow-core/src/test/resources/country_sales_small.csv" - private val datasetACsvFilePath = "/[email protected]/test_dataset/v2/directory/a.csv" + private val datasetACsvFilePath = "/datasets/[email protected]/test_dataset/v2/directory/a.csv" - private val dataset1TxtFilePath = "/[email protected]/test_dataset/v1/1.txt" + private val dataset1TxtFilePath = "/datasets/[email protected]/test_dataset/v1/1.txt" + + // Unprefixed form (no resource-type segment); no longer resolvable as a dataset. + private val unprefixedDataset1TxtFilePath = "/[email protected]/test_dataset/v1/1.txt" + + // The leading segment is not a known resource type, so this is not a resolvable path. + private val unknownResourceTypeFilePath = + "/notAResourceType/[email protected]/test_dataset/v1/1.txt" override protected def beforeAll(): Unit = { initializeDBAndReplaceDSLContext() @@ -117,6 +124,25 @@ class FileResolverSpec ) } + "FileResolver" should "not resolve a path without a resource-type prefix" in { + // Without a leading resource-type segment the path is not resolvable + assertThrows[FileNotFoundException] { + FileResolver.resolve(unprefixedDataset1TxtFilePath) + } + } + + "FileResolver" should "not resolve a path whose prefix is not a known resource type" in { + assertThrows[FileNotFoundException] { + FileResolver.resolve(unknownResourceTypeFilePath) + } + } + + "FileResolver" should "throw not found exception when a prefixed path has too few segments" in { + assertThrows[FileNotFoundException] { + FileResolver.resolve("/datasets/[email protected]/test_dataset") + } + } + "FileResolver" should "throw not found exception" in { assertThrows[FileNotFoundException] { FileResolver.resolve("some/random/path") @@ -142,18 +168,22 @@ class FileResolverSpec "parseDatasetOwnerAndName" should "extract owner email and dataset name from a valid path" in { assert( - FileResolver.parseDatasetOwnerAndName("/[email protected]/test_dataset/v1/1.txt") + FileResolver.parseDatasetOwnerAndName("/datasets/[email protected]/test_dataset/v1/1.txt") == Some(("[email protected]", "test_dataset")) ) // extra segments beyond the file-relative path are ignored assert( - FileResolver.parseDatasetOwnerAndName("/[email protected]/ds/v2/directory/nested/a.csv") + FileResolver.parseDatasetOwnerAndName("/datasets/[email protected]/ds/v2/directory/nested/a.csv") == Some(("[email protected]", "ds")) ) } - it should "return None when the path has fewer than four segments" in { - assert(FileResolver.parseDatasetOwnerAndName("/[email protected]/ds/v1").isEmpty) + it should "return None for an unprefixed path (the datasets prefix is required)" in { + assert(FileResolver.parseDatasetOwnerAndName(unprefixedDataset1TxtFilePath).isEmpty) + } + + it should "return None when the prefixed path has too few segments" in { + assert(FileResolver.parseDatasetOwnerAndName("/datasets/[email protected]/ds").isEmpty) assert(FileResolver.parseDatasetOwnerAndName("owner/dataset").isEmpty) } diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExec.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExec.scala index c58da8ca84..54bc607be1 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExec.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExec.scala @@ -20,6 +20,7 @@ package org.apache.texera.amber.operator.source.dataset import org.apache.texera.amber.core.executor.SourceOperatorExecutor +import org.apache.texera.amber.core.storage.ResourceType import org.apache.texera.amber.core.storage.util.LakeFSStorageClient import org.apache.texera.amber.core.tuple.TupleLike import org.apache.texera.amber.util.JSONUtils.objectMapper @@ -28,13 +29,48 @@ import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET import org.apache.texera.dao.jooq.generated.tables.DatasetVersion.DATASET_VERSION import org.apache.texera.dao.jooq.generated.tables.User.USER +object FileListerSourceOpExec { + + /** + * Parses a dataset version path (/datasets/ownerEmail/datasetName/versionName) into its + * (resourceTypePrefix, ownerEmail, datasetName, versionName) components. + * + * @throws IllegalArgumentException if the path is not a well-formed dataset version path + */ + private[dataset] def parseDatasetVersionPath( + datasetVersionPath: String + ): (String, String, String, String) = { + val segments = datasetVersionPath.split("/").filter(_.nonEmpty) + require( + segments.length >= 4 && ResourceType.isValidPrefix(segments.head), + s"Invalid dataset version path '$datasetVersionPath'; " + + "expected /datasets/ownerEmail/datasetName/versionName" + ) + (segments(0), segments(1), segments(2), segments(3)) + } + + private[dataset] def canonicalVersionPath( + resourceTypePrefix: String, + ownerEmail: String, + datasetName: String, + versionName: String + ): String = s"/$resourceTypePrefix/$ownerEmail/$datasetName/$versionName" +} + class FileListerSourceOpExec private[dataset] (descString: String) extends SourceOperatorExecutor { private val desc: FileListerSourceOpDesc = objectMapper.readValue(descString, classOf[FileListerSourceOpDesc]) override def produceTuple(): Iterator[TupleLike] = { - val Seq(_, ownerEmail, datasetName, versionName, _*) = - desc.datasetVersionPath.split("/").toSeq + val (resourceTypePrefix, ownerEmail, datasetName, versionName) = + FileListerSourceOpExec.parseDatasetVersionPath(desc.datasetVersionPath) + + val versionPath = FileListerSourceOpExec.canonicalVersionPath( + resourceTypePrefix, + ownerEmail, + datasetName, + versionName + ) val (repositoryName, versionHash) = SqlServer @@ -53,7 +89,7 @@ class FileListerSourceOpExec private[dataset] (descString: String) extends Sourc LakeFSStorageClient .retrieveObjectsOfVersion(repositoryName, versionHash) - .map(obj => TupleLike("filename" -> s"${desc.datasetVersionPath}/${obj.getPath}")) + .map(obj => TupleLike("filename" -> s"$versionPath/${obj.getPath}")) .iterator } } diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExecSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExecSpec.scala new file mode 100644 index 0000000000..11908a9d49 --- /dev/null +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/dataset/FileListerSourceOpExecSpec.scala @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.amber.operator.source.dataset + +import org.scalatest.flatspec.AnyFlatSpec + +class FileListerSourceOpExecSpec extends AnyFlatSpec { + + "parseDatasetVersionPath" should "extract components from a datasets-prefixed path" in { + val (prefix, owner, name, version) = + FileListerSourceOpExec.parseDatasetVersionPath("/datasets/[email protected]/twitterDataset/v1") + assert(prefix == "datasets") + assert(owner == "[email protected]") + assert(name == "twitterDataset") + assert(version == "v1") + } + + it should "work when the owner segment is a username without an '@'" in { + val (prefix, owner, name, version) = + FileListerSourceOpExec.parseDatasetVersionPath("/datasets/texera/test-ds/v1") + assert(owner == "texera") + assert(name == "test-ds") + assert(version == "v1") + } + + it should "ignore trailing slashes and extra segments" in { + val (prefix, owner, name, version) = + FileListerSourceOpExec.parseDatasetVersionPath("/datasets/alice/ds/v2/extra/") + assert(prefix == "datasets") + assert(owner == "alice") + assert(name == "ds") + assert(version == "v2") + } + + "canonicalVersionPath" should "rebuild the prefixed version path from its components" in { + assert( + FileListerSourceOpExec.canonicalVersionPath( + "datasets", + "[email protected]", + "twitterDataset", + "v1" + ) == "/datasets/[email protected]/twitterDataset/v1" + ) + } + + it should "drop extra segments so emitted file paths stay canonical" in { + // Emitted paths must be rooted at the parsed components, not the raw configured path: + // a stray "extra" segment would otherwise leak into every emitted file path. + val (prefix, owner, name, version) = + FileListerSourceOpExec.parseDatasetVersionPath("/datasets/alice/ds/v2/extra/") + assert( + FileListerSourceOpExec.canonicalVersionPath(prefix, owner, name, version) + == "/datasets/alice/ds/v2" + ) + } + + it should "reject a path without a resource-type prefix" in { + assertThrows[IllegalArgumentException] { + FileListerSourceOpExec.parseDatasetVersionPath("/alice/ds/v1") + } + } + + it should "reject a path whose prefix is not a known resource type" in { + assertThrows[IllegalArgumentException] { + FileListerSourceOpExec.parseDatasetVersionPath("/notAResourceType/alice/ds/v1") + } + } + + it should "reject a path with too few segments" in { + assertThrows[IllegalArgumentException] { + FileListerSourceOpExec.parseDatasetVersionPath("/datasets/alice/ds") + } + } +} diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index 289832fada..50af933699 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -28,7 +28,7 @@ import org.apache.texera.common.config.StorageConfig import org.apache.texera.common.util.EmailUtil import org.apache.texera.amber.core.storage.model.OnDataset import org.apache.texera.amber.core.storage.util.LakeFSStorageClient -import org.apache.texera.amber.core.storage.{DocumentFactory, FileResolver} +import org.apache.texera.amber.core.storage.{DocumentFactory, FileResolver, ResourceType} import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SiteSettings import org.apache.texera.dao.SqlServer @@ -91,6 +91,15 @@ object DatasetResource { .getInstance() .createDSLContext() + // Builds a resource logical path (/<resourceType>/ownerEmail/resourceName/relativePath). + private def logicalPath( + resourceType: ResourceType.Value, + ownerEmail: String, + resourceName: String, + relativePath: String + ): String = + s"$resourceType/$ownerEmail/$resourceName/$relativePath" + private def singleFileUploadMaxBytes(defaultMiB: Long = 20L): Long = SiteSettings.getLong("single_file_upload_max_size_mib", defaultMiB) * 1024L * 1024L @@ -343,6 +352,8 @@ class DatasetResource extends LazyLogging { private val COVER_IMAGE_SIZE_LIMIT_BYTES: Long = 10 * 1024 * 1024 // 10 MB private val ALLOWED_IMAGE_EXTENSIONS: Set[String] = Set(".jpg", ".jpeg", ".png", ".gif", ".webp") + private val resourceType = ResourceType.Datasets + /** * Helper function to get the dataset from DB with additional information including user access privilege and owner email */ @@ -1413,15 +1424,25 @@ class DatasetResource extends LazyLogging { throw new NotFoundException(ERR_DATASET_VERSION_NOT_FOUND_MESSAGE) ) - val ownerNode = DatasetFileNode + val datasetsNode = DatasetFileNode .fromLakeFSRepositoryCommittedObjects( Map( - (user.getEmail, dataset.getName, latestVersion.getName) -> LakeFSStorageClient + ( + getOwner(ctx, did).getEmail, + dataset.getName, + latestVersion.getName + ) -> LakeFSStorageClient .retrieveObjectsOfVersion(dataset.getRepositoryName, latestVersion.getVersionHash) ) ) .head + val ownerNode = datasetsNode.getChildren.headOption.getOrElse( + throw new IllegalStateException( + s"Dataset file tree for ${dataset.getName} is missing its owner node" + ) + ) + DashboardDatasetVersion( latestVersion, ownerNode.children.get @@ -1648,7 +1669,7 @@ class DatasetResource extends LazyLogging { val datasetName = dataset.dataset.getName val repositoryName = dataset.dataset.getRepositoryName - val ownerFileNode = DatasetFileNode + val datasetsNode = DatasetFileNode .fromLakeFSRepositoryCommittedObjects( Map( (dataset.ownerEmail, datasetName, datasetVersion.getName) -> LakeFSStorageClient @@ -1657,6 +1678,12 @@ class DatasetResource extends LazyLogging { ) .head + val ownerFileNode = datasetsNode.getChildren.headOption.getOrElse( + throw new IllegalStateException( + s"Dataset file tree for $datasetName is missing its owner node" + ) + ) + DatasetVersionRootFileNodesResponse( ownerFileNode.children.get .find(_.getName == datasetName) @@ -1667,7 +1694,7 @@ class DatasetResource extends LazyLogging { .head .children .get, - DatasetFileNode.calculateTotalSize(List(ownerFileNode)) + DatasetFileNode.calculateTotalSize(List(datasetsNode)) ) } @@ -2375,7 +2402,9 @@ class DatasetResource extends LazyLogging { val owner = getOwner(ctx, did) val document = DocumentFactory .openReadonlyDocument( - FileResolver.resolve(s"${owner.getEmail}/${dataset.getName}/$normalized") + FileResolver.resolve( + logicalPath(resourceType, owner.getEmail, dataset.getName, normalized) + ) ) .asInstanceOf[OnDataset] @@ -2429,7 +2458,8 @@ class DatasetResource extends LazyLogging { ) val owner = getOwner(ctx, did) - val fullPath = s"${owner.getEmail}/${dataset.getName}/$coverImage" + val fullPath = + logicalPath(resourceType, owner.getEmail, dataset.getName, coverImage) val document = DocumentFactory .openReadonlyDocument(FileResolver.resolve(fullPath)) @@ -2478,7 +2508,8 @@ class DatasetResource extends LazyLogging { Response.ok(Map("url" -> null)).build() case Some(coverImage) => val owner = getOwner(ctx, did) - val fullPath = s"${owner.getEmail}/${dataset.getName}/$coverImage" + val fullPath = + logicalPath(resourceType, owner.getEmail, dataset.getName, coverImage) val document = DocumentFactory .openReadonlyDocument(FileResolver.resolve(fullPath)) diff --git a/file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala b/file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala index ba789f0e5a..821d02045c 100644 --- a/file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala +++ b/file-service/src/main/scala/org/apache/texera/service/type/dataset/DatasetFileNode.scala @@ -20,13 +20,13 @@ package org.apache.texera.service.`type` import io.lakefs.clients.sdk.model.ObjectStats +import org.apache.texera.amber.core.storage.ResourceType import scala.collection.mutable // DatasetFileNode represents a unique file in dataset, its full path is in the format of: -// /ownerEmail/datasetName/versionName/fileRelativePath -// e.g. /[email protected]/twitterDataset/v1/california/irvine/tw1.csv -// ownerName is [email protected]; datasetName is twitterDataset, versionName is v1, fileRelativePath is california/irvine/tw1.csv +// /datasets/ownerEmail/datasetName/versionName/fileRelativePath +// e.g. /datasets/[email protected]/twitterDataset/v1/california/irvine/tw1.csv class DatasetFileNode( val name: String, // direct name of this node val nodeType: String, // "file" or "directory" @@ -79,6 +79,11 @@ object DatasetFileNode { ): List[DatasetFileNode] = { val rootNode = new DatasetFileNode("/", "directory", null, "") + // Root the tree at the datasets prefix node (a directory node named "datasets"). + val datasetsNode = + new DatasetFileNode(ResourceType.Datasets.toString, "directory", rootNode, "") + rootNode.children = Some(List(datasetsNode)) + // Owner level nodes map val ownerNodes = mutable.Map[String, DatasetFileNode]() @@ -86,8 +91,8 @@ object DatasetFileNode { case ((ownerEmail, datasetName, versionName), objects) => val ownerNode = ownerNodes.getOrElseUpdate( ownerEmail, { - val newNode = new DatasetFileNode(ownerEmail, "directory", rootNode, ownerEmail) - rootNode.children = Some(rootNode.getChildren :+ newNode) + val newNode = new DatasetFileNode(ownerEmail, "directory", datasetsNode, ownerEmail) + datasetsNode.children = Some(datasetsNode.getChildren :+ newNode) newNode } ) diff --git a/file-service/src/test/scala/org/apache/texera/service/type/dataset/DatasetFileNodeSpec.scala b/file-service/src/test/scala/org/apache/texera/service/type/dataset/DatasetFileNodeSpec.scala index e4ec48bbf6..160412f8b9 100644 --- a/file-service/src/test/scala/org/apache/texera/service/type/dataset/DatasetFileNodeSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/type/dataset/DatasetFileNodeSpec.scala @@ -99,10 +99,13 @@ class DatasetFileNodeSpec extends AnyFlatSpec with Matchers { Map(("[email protected]", "twitter", "v1") -> objects) ) - // One owner root. + // The tree is rooted at a single "datasets" prefix node; owners nest under it. roots should have size 1 - val ownerNode = roots.head - ownerNode.getName shouldBe "[email protected]" + val datasetsNode = roots.head + datasetsNode.getName shouldBe "datasets" + datasetsNode.getNodeType shouldBe "directory" + + val ownerNode = datasetsNode.getChildren.find(_.getName == "[email protected]").get ownerNode.getNodeType shouldBe "directory" val datasetNode = ownerNode.getChildren.find(_.getName == "twitter").get @@ -120,7 +123,7 @@ class DatasetFileNodeSpec extends AnyFlatSpec with Matchers { val file1 = bDir.getChildren.find(_.getName == "1.csv").get file1.getNodeType shouldBe "file" file1.getSize shouldBe Some(2L) - file1.getFilePath shouldBe "/[email protected]/twitter/v1/b/1.csv" + file1.getFilePath shouldBe "/datasets/[email protected]/twitter/v1/b/1.csv" // Total size equals the sum of the three files. DatasetFileNode.calculateTotalSize(roots) shouldBe 6L diff --git a/frontend/src/app/common/type/dataset-file.spec.ts b/frontend/src/app/common/type/dataset-file.spec.ts deleted file mode 100644 index 76acdfe914..0000000000 --- a/frontend/src/app/common/type/dataset-file.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { DatasetFile, parseDatasetFileToFilePath, parseFilePathToDatasetFile } from "./dataset-file"; - -describe("parseFilePathToDatasetFile", () => { - it("parses owner, dataset, version, and single-segment relative path", () => { - const result = parseFilePathToDatasetFile("/[email protected]/twitterDataset/v1/tw1.csv"); - expect(result).toEqual({ - ownerEmail: "[email protected]", - datasetName: "twitterDataset", - versionName: "v1", - fileRelativePath: "tw1.csv", - }); - }); - - it("joins remaining segments into a nested relative path", () => { - const result = parseFilePathToDatasetFile("/[email protected]/twitterDataset/v1/california/irvine/tw1.csv"); - expect(result.ownerEmail).toBe("[email protected]"); - expect(result.datasetName).toBe("twitterDataset"); - expect(result.versionName).toBe("v1"); - expect(result.fileRelativePath).toBe("california/irvine/tw1.csv"); - }); - - it("ignores empty segments from leading, trailing, and duplicate slashes", () => { - const result = parseFilePathToDatasetFile("//[email protected]//twitterDataset/v1/dir//file.csv/"); - expect(result).toEqual({ - ownerEmail: "[email protected]", - datasetName: "twitterDataset", - versionName: "v1", - fileRelativePath: "dir/file.csv", - }); - }); - - it("throws when there are fewer than four path segments", () => { - expect(() => parseFilePathToDatasetFile("/[email protected]/twitterDataset/v1")).toThrow("Invalid file path format"); - expect(() => parseFilePathToDatasetFile("")).toThrow("Invalid file path format"); - expect(() => parseFilePathToDatasetFile("/just/three/parts")).toThrow("Invalid file path format"); - }); -}); - -describe("parseDatasetFileToFilePath", () => { - it("assembles a slash-delimited path with a leading slash", () => { - const datasetFile: DatasetFile = { - ownerEmail: "[email protected]", - datasetName: "twitterDataset", - versionName: "v1", - fileRelativePath: "california/irvine/tw1.csv", - }; - expect(parseDatasetFileToFilePath(datasetFile)).toBe("/[email protected]/twitterDataset/v1/california/irvine/tw1.csv"); - }); -}); - -describe("dataset-file round trips", () => { - it("path -> DatasetFile -> path is stable for a canonical path", () => { - const path = "/[email protected]/twitterDataset/v1/california/irvine/tw1.csv"; - expect(parseDatasetFileToFilePath(parseFilePathToDatasetFile(path))).toBe(path); - }); - - it("DatasetFile -> path -> DatasetFile is stable for a canonical object", () => { - const datasetFile: DatasetFile = { - ownerEmail: "[email protected]", - datasetName: "sensorData", - versionName: "v42", - fileRelativePath: "2026/reading.json", - }; - expect(parseFilePathToDatasetFile(parseDatasetFileToFilePath(datasetFile))).toEqual(datasetFile); - }); -}); diff --git a/frontend/src/app/common/type/dataset-file.ts b/frontend/src/app/common/type/dataset-file.ts deleted file mode 100644 index 5fe561d372..0000000000 --- a/frontend/src/app/common/type/dataset-file.ts +++ /dev/null @@ -1,60 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -// user given filePath is /ownerEmail/datasetName/versionName/fileRelativePath -// e.g. /[email protected]/twitterDataset/v1/california/irvine/tw1.csv -export interface DatasetFile { - ownerEmail: string; - datasetName: string; - versionName: string; - fileRelativePath: string; -} - -/** - * Parses a file path string to a DatasetFile interface. - * @param filePath - The file path string to parse. - * @returns The parsed DatasetFile object. - */ -export function parseFilePathToDatasetFile(filePath: string): DatasetFile { - const parts = filePath.split("/").filter(part => part.length > 0); - - if (parts.length < 4) { - throw new Error("Invalid file path format"); - } - - const [ownerEmail, datasetName, versionName, ...fileRelativePathParts] = parts; - const fileRelativePath = fileRelativePathParts.join("/"); - - return { - ownerEmail, - datasetName, - versionName, - fileRelativePath, - }; -} - -/** - * Converts a DatasetFile object to a file path string. - * @param datasetFile - The DatasetFile object to convert. - * @returns The file path string. - */ -export function parseDatasetFileToFilePath(datasetFile: DatasetFile): string { - const { ownerEmail, datasetName, versionName, fileRelativePath } = datasetFile; - return `/${ownerEmail}/${datasetName}/${versionName}/${fileRelativePath}`; -} diff --git a/frontend/src/app/common/type/datasetVersionFileTree.spec.ts b/frontend/src/app/common/type/datasetVersionFileTree.spec.ts index 76aa8f286d..64231dd156 100644 --- a/frontend/src/app/common/type/datasetVersionFileTree.spec.ts +++ b/frontend/src/app/common/type/datasetVersionFileTree.spec.ts @@ -9,12 +9,11 @@ * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. */ import { @@ -38,27 +37,40 @@ describe("getFullPathFromDatasetFileNode", () => { }); describe("getRelativePathFromDatasetFileNode", () => { - it("strips the first three path segments", () => { - const node: DatasetFileNode = { name: "file.csv", type: "file", parentDir: "/owner/dataset/v1" }; - // full path is /owner/dataset/v1/file.csv -> segments [owner, dataset, v1, file.csv] - expect(getRelativePathFromDatasetFileNode(node)).toBe("file.csv"); + it("strips the datasets/owner/dataset/version prefix (4 segments)", () => { + const node: DatasetFileNode = { + name: "tw1.csv", + type: "file", + parentDir: "/datasets/[email protected]/twitterDataset/v1/california/irvine", + }; + expect(getRelativePathFromDatasetFileNode(node)).toBe("california/irvine/tw1.csv"); }); - it("preserves nested relative segments beyond the first three", () => { - const node: DatasetFileNode = { name: "f.txt", type: "file", parentDir: "/owner/dataset/v1/sub/dir" }; - expect(getRelativePathFromDatasetFileNode(node)).toBe("sub/dir/f.txt"); + it("returns the bare file name for a file at the version root", () => { + const node: DatasetFileNode = { + name: "readme.txt", + type: "file", + parentDir: "/datasets/[email protected]/twitterDataset/v1", + }; + expect(getRelativePathFromDatasetFileNode(node)).toBe("readme.txt"); }); - it("returns an empty string when there are three or fewer segments", () => { - const node: DatasetFileNode = { name: "v1", type: "directory", parentDir: "/owner/dataset" }; - // full path /owner/dataset/v1 -> exactly 3 segments -> no relative path + it("returns an empty string when there is no path below the version", () => { + const node: DatasetFileNode = { + name: "v1", + type: "directory", + parentDir: "/datasets/[email protected]/twitterDataset", + }; expect(getRelativePathFromDatasetFileNode(node)).toBe(""); }); it("ignores empty segments from duplicate slashes when counting", () => { - const node: DatasetFileNode = { name: "file.csv", type: "file", parentDir: "/owner//dataset/v1" }; - // empty segment between the duplicate slashes is filtered out, leaving 4 real segments - expect(getRelativePathFromDatasetFileNode(node)).toBe("file.csv"); + const node: DatasetFileNode = { + name: "f.csv", + type: "file", + parentDir: "/datasets/[email protected]/twitterDataset//v1/sub", + }; + expect(getRelativePathFromDatasetFileNode(node)).toBe("sub/f.csv"); }); }); diff --git a/frontend/src/app/common/type/datasetVersionFileTree.ts b/frontend/src/app/common/type/datasetVersionFileTree.ts index 8d1686998c..98f898d443 100644 --- a/frontend/src/app/common/type/datasetVersionFileTree.ts +++ b/frontend/src/app/common/type/datasetVersionFileTree.ts @@ -31,19 +31,20 @@ export function getFullPathFromDatasetFileNode(node: DatasetFileNode): string { } /** - * Returns the relative path of a DatasetFileNode by stripping the first three segments. + * Returns the relative path of a DatasetFileNode by stripping the first four segments + * (datasets/ownerEmail/datasetName/versionName). * @param node The DatasetFileNode whose relative path is needed. - * @returns The relative path (without the first three segments and without a leading slash). + * @returns The relative path (without the first four segments and without a leading slash). */ export function getRelativePathFromDatasetFileNode(node: DatasetFileNode): string { const fullPath = getFullPathFromDatasetFileNode(node); // Get the full path const pathSegments = fullPath.split("/").filter(segment => segment.length > 0); // Split and remove empty segments - if (pathSegments.length <= 3) { - return ""; // If there are 3 or fewer segments, return an empty string (no relative path exists) + if (pathSegments.length <= 4) { + return ""; // If there are 4 or fewer segments, return an empty string (no relative path exists) } - return pathSegments.slice(3).join("/"); // Join remaining segments as the relative path + return pathSegments.slice(4).join("/"); // Join remaining segments as the relative path } export function getPathsUnderOrEqualDatasetFileNode(node: DatasetFileNode): string[] { diff --git a/frontend/src/app/common/type/resource-type.ts b/frontend/src/app/common/type/resource-type.ts new file mode 100644 index 0000000000..a129c917e2 --- /dev/null +++ b/frontend/src/app/common/type/resource-type.ts @@ -0,0 +1,24 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * The leading segment of a logical file path, identifying the resource kind (e.g. /datasets/...). + */ +export enum ResourceType { + Datasets = "datasets", +} diff --git a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts index 0abb962621..673c1d39bd 100644 --- a/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts +++ b/frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/user-dataset-version-filetree/user-dataset-version-filetree.component.spec.ts @@ -167,9 +167,9 @@ describe("UserDatasetVersionFiletreeComponent", () => { const emitted: string[] = []; component.setCoverImage.subscribe((path: string) => emitted.push(path)); - // parentDir has exactly the three stripped segments (owner/dataset/version), + // parentDir has exactly the four stripped segments (datasets/owner/dataset/version), // so the relative path is just the file name. - component.onSetCover({ name: "photo.png", type: "file", parentDir: "/owner/dataset/v1" }); + component.onSetCover({ name: "photo.png", type: "file", parentDir: "/datasets/owner/dataset/v1" }); expect(emitted).toEqual(["photo.png"]); }); diff --git a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts index bf3bfdb133..d1483b71ad 100644 --- a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts +++ b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.spec.ts @@ -108,7 +108,7 @@ describe("DatasetSelectionModalComponent", () => { it("ngOnInit initializes selectedDataset and selectedVersion from data.selectedPath", () => { modalData.fileMode = true; - modalData.selectedPath = `/${OWNER}/myds/v1`; + modalData.selectedPath = `/datasets/${OWNER}/myds/v1`; build(); @@ -141,7 +141,7 @@ describe("DatasetSelectionModalComponent", () => { expect(datasetService.retrieveDatasetVersionFileTree).toHaveBeenCalledWith(10, 100); expect(component.fileTree).toEqual([fileNode]); - expect(component.selectedPath).toBe(`/${OWNER}/myds/v1`); + expect(component.selectedPath).toBe(`/datasets/${OWNER}/myds/v1`); }); it("onFileSelected sets selectedPath to the node's full path in file mode", () => { diff --git a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.ts b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.ts index fb289b0290..5ee5b9115e 100644 --- a/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.ts +++ b/frontend/src/app/workspace/component/dataset-selection-modal/dataset-selection-modal.component.ts @@ -22,6 +22,7 @@ import { NZ_MODAL_DATA, NzModalRef } from "ng-zorro-antd/modal"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { DatasetFileNode, getFullPathFromDatasetFileNode } from "../../../common/type/datasetVersionFileTree"; import { DatasetVersion } from "../../../common/type/dataset"; +import { ResourceType } from "../../../common/type/resource-type"; import { DashboardDataset } from "../../../dashboard/type/dashboard-dataset.interface"; import { DatasetService } from "../../../dashboard/service/user/dataset/dataset.service"; import { NzRowDirective, NzColDirective } from "ng-zorro-antd/grid"; @@ -83,7 +84,8 @@ export class DatasetSelectionModalComponent implements OnInit { this.datasets = datasets; const selectedPath = this.data.selectedPath; if (selectedPath) { - const [ownerEmail, datasetName, versionName] = selectedPath.split("/").filter(part => part.length > 0); + // Stored paths always carry the resource-type prefix; skip it so that owner/dataset/version line up. + const [, ownerEmail, datasetName, versionName] = selectedPath.split("/").filter(part => part.length > 0); this.selectedDataset = this.datasets.find( dataset => dataset.ownerEmail === ownerEmail && dataset.dataset.name === datasetName ); @@ -118,7 +120,7 @@ export class DatasetSelectionModalComponent implements OnInit { this.fileTree = data.fileNodes; }); if (!this.data.fileMode) { - this.selectedPath = `/${this.selectedDataset.ownerEmail}/${this.selectedDataset.dataset.name}/${this.selectedVersion.name}`; + this.selectedPath = `/${ResourceType.Datasets}/${this.selectedDataset.ownerEmail}/${this.selectedDataset.dataset.name}/${this.selectedVersion.name}`; } } } diff --git a/sql/changelog.xml b/sql/changelog.xml index 42e40f7b83..a945b6bcb6 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -89,6 +89,11 @@ <sqlFile path="sql/updates/35.sql"/> </changeSet> + <!-- Prefix dataset paths in stored workflow content with "datasets/" --> + <changeSet id="36" author="tanishqgandhi1908"> + <sqlFile path="sql/updates/36.sql"/> + </changeSet> + <!-- example changeSet <changeSet id="1" author="author"> <sqlFile path="sql/updates/1.sql"/> diff --git a/sql/updates/36.sql b/sql/updates/36.sql new file mode 100644 index 0000000000..5e3c1a182e --- /dev/null +++ b/sql/updates/36.sql @@ -0,0 +1,203 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +\c texera_db + +SET search_path TO texera_db; + +BEGIN; + +-- The file resolver now requires an explicit resource-type prefix on dataset +-- logical paths (/datasets/ownerEmail/datasetName/versionName/...) so other +-- resource types (e.g. models) can be told apart by the prefix. Existing +-- workflows store unprefixed dataset paths inside workflow.content and +-- workflow_version.content, in two operator properties: +-- * fileName (scan-source operators): /owner/name/version/file +-- * datasetVersionPath (file-lister operator): /owner/name/version +-- This migration prepends the "datasets" segment to both. +-- +-- A value is treated as a dataset path only when its first two segments match an +-- existing (user.email, dataset.name) pair -- that pair is unique. +-- Local file paths and URLs match no dataset and are left untouched. +-- Already-prefixed values are skipped (idempotent). jsonb_set +-- uses create_missing = false so absent properties are never added. + +DO $$ +DECLARE + wf_count INT := 0; + wv_count INT := 0; +BEGIN + WITH affected AS ( + SELECT w.wid + FROM workflow w, + jsonb_array_elements( + CASE + WHEN jsonb_typeof(w.content::jsonb -> 'operators') = 'array' + THEN w.content::jsonb -> 'operators' + ELSE '[]'::jsonb + END + ) AS op, + LATERAL ( + SELECT op #>> '{operatorProperties,fileName}' AS fn, + op #>> '{operatorProperties,datasetVersionPath}' AS dvp + ) f + WHERE jsonb_typeof(w.content::jsonb -> 'operators') = 'array' + AND ( + (f.fn IS NOT NULL AND left(f.fn, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.fn, '/'), '/', 1) + AND d.name = split_part(ltrim(f.fn, '/'), '/', 2))) + OR + (f.dvp IS NOT NULL AND left(f.dvp, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.dvp, '/'), '/', 1) + AND d.name = split_part(ltrim(f.dvp, '/'), '/', 2))) + ) + GROUP BY w.wid + ), + updated AS ( + UPDATE workflow w + SET content = jsonb_set(w.content::jsonb, '{operators}', ( + SELECT jsonb_agg( + jsonb_set( + jsonb_set( + op, + '{operatorProperties,fileName}', + CASE + WHEN f.fn IS NOT NULL AND left(f.fn, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.fn, '/'), '/', 1) + AND d.name = split_part(ltrim(f.fn, '/'), '/', 2)) + THEN to_jsonb('/datasets/' || ltrim(f.fn, '/')) + ELSE COALESCE(op #> '{operatorProperties,fileName}', 'null'::jsonb) + END, + false + ), + '{operatorProperties,datasetVersionPath}', + CASE + WHEN f.dvp IS NOT NULL AND left(f.dvp, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.dvp, '/'), '/', 1) + AND d.name = split_part(ltrim(f.dvp, '/'), '/', 2)) + THEN to_jsonb('/datasets/' || ltrim(f.dvp, '/')) + ELSE COALESCE(op #> '{operatorProperties,datasetVersionPath}', 'null'::jsonb) + END, + false + ) + ORDER BY ord + ) + FROM jsonb_array_elements( + CASE + WHEN jsonb_typeof(w.content::jsonb -> 'operators') = 'array' + THEN w.content::jsonb -> 'operators' + ELSE '[]'::jsonb + END + ) WITH ORDINALITY AS t(op, ord), + LATERAL ( + SELECT op #>> '{operatorProperties,fileName}' AS fn, + op #>> '{operatorProperties,datasetVersionPath}' AS dvp + ) f + ))::text + FROM affected a + WHERE w.wid = a.wid + RETURNING 1 + ) + SELECT count(*) INTO wf_count FROM updated; + + WITH affected AS ( + SELECT wv.vid + FROM workflow_version wv, + jsonb_array_elements( + CASE + WHEN jsonb_typeof(wv.content::jsonb -> 'operators') = 'array' + THEN wv.content::jsonb -> 'operators' + ELSE '[]'::jsonb + END + ) AS op, + LATERAL ( + SELECT op #>> '{operatorProperties,fileName}' AS fn, + op #>> '{operatorProperties,datasetVersionPath}' AS dvp + ) f + WHERE jsonb_typeof(wv.content::jsonb -> 'operators') = 'array' + AND ( + (f.fn IS NOT NULL AND left(f.fn, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.fn, '/'), '/', 1) + AND d.name = split_part(ltrim(f.fn, '/'), '/', 2))) + OR + (f.dvp IS NOT NULL AND left(f.dvp, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.dvp, '/'), '/', 1) + AND d.name = split_part(ltrim(f.dvp, '/'), '/', 2))) + ) + GROUP BY wv.vid + ), + updated AS ( + UPDATE workflow_version wv + SET content = jsonb_set(wv.content::jsonb, '{operators}', ( + SELECT jsonb_agg( + jsonb_set( + jsonb_set( + op, + '{operatorProperties,fileName}', + CASE + WHEN f.fn IS NOT NULL AND left(f.fn, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.fn, '/'), '/', 1) + AND d.name = split_part(ltrim(f.fn, '/'), '/', 2)) + THEN to_jsonb('/datasets/' || ltrim(f.fn, '/')) + ELSE COALESCE(op #> '{operatorProperties,fileName}', 'null'::jsonb) + END, + false + ), + '{operatorProperties,datasetVersionPath}', + CASE + WHEN f.dvp IS NOT NULL AND left(f.dvp, 10) <> '/datasets/' + AND EXISTS (SELECT 1 FROM dataset d JOIN "user" u ON d.owner_uid = u.uid + WHERE u.email = split_part(ltrim(f.dvp, '/'), '/', 1) + AND d.name = split_part(ltrim(f.dvp, '/'), '/', 2)) + THEN to_jsonb('/datasets/' || ltrim(f.dvp, '/')) + ELSE COALESCE(op #> '{operatorProperties,datasetVersionPath}', 'null'::jsonb) + END, + false + ) + ORDER BY ord + ) + FROM jsonb_array_elements( + CASE + WHEN jsonb_typeof(wv.content::jsonb -> 'operators') = 'array' + THEN wv.content::jsonb -> 'operators' + ELSE '[]'::jsonb + END + ) WITH ORDINALITY AS t(op, ord), + LATERAL ( + SELECT op #>> '{operatorProperties,fileName}' AS fn, + op #>> '{operatorProperties,datasetVersionPath}' AS dvp + ) f + ))::text + FROM affected a + WHERE wv.vid = a.vid + RETURNING 1 + ) + SELECT count(*) INTO wv_count FROM updated; + + RAISE NOTICE 'Prefixed legacy dataset paths with "datasets/" in % workflow and % workflow_version row(s).', wf_count, wv_count; +END $$; + +COMMIT;
