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-7928-5de9bf94766d441e7d1655bf326a9208dc6769a5 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 444fc582840cbac7a6fe3479fd3890dd6efe9549 Author: Tanishq Gandhi <[email protected]> AuthorDate: Wed Aug 26 17:39:02 2026 +0000 refactor(amber): make the hub and unified search resource-agnostic (#7928) ### What changes were proposed in this PR? The hub dispatched on `EntityType` in four separate places, and `DatasetSearchQueryBuilder` carried its own copy of the access-scoped join, the keyword/date/id filter and the LakeFS-sized hydration. Adding a second resource type meant four dispatch edits plus a clone of the query logic, and two of those sites compiled cleanly but failed at runtime. - `EntityTables` now exposes one `EntityTableSet` per entity type, with `cloneTable` as an `Option` and a new `AccessTable` that removes the non-exhaustive match in `HubResource.userAccess`. - `VersionedResourceTables` names the columns of a LakeFS-backed resource; `VersionedResourceSearchQueryBuilder` holds the single copy of FROM / WHERE / hydration. `DatasetSearchQueryBuilder` keeps only its projection. - The five dataset-only slots in `UnifiedResourceSchema` become shared versioned-resource slots, so the next resource type widens the union by two columns instead of seven. Two existing bugs are fixed as a result: `getTops?entityType=dataset` with no `actionTypes` no longer 500s on a throwing `CloneTable(Dataset)`, and `getCounts`' clone guard now asks whether the entity type has a clone table instead of whether it is not a dataset. No user-facing behavior changes. ### Any related issues, documentation, discussions? Prepares the amber side of #6501 (part of #6494): adding models becomes one descriptor plus one registry entry. Contains #6872 and #7922, so their commits show in the diff until they merge. ### How was this PR tested? Existing suites, unmodified except where they pin the renamed projection aliases: `WorkflowExecutionService/testOnly *dashboard*` — 467/467, and `FileService/test` — 376/376. New `VersionedResourceTablesSpec` covers the descriptor contract and the access-join predicate; `EntityTablesSpec` covers the registry; regression tests added for the two bugs above (`getTops` with no `actionTypes`, and the clone bucket for a cloneless entity type). ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --- .../dashboard/DatasetSearchQueryBuilder.scala | 139 +------------- .../resource/dashboard/UnifiedResourceSchema.scala | 41 +++-- .../VersionedResourceSearchQueryBuilder.scala | 96 ++++++++++ .../dashboard/VersionedResourceTables.scala | 147 +++++++++++++++ .../web/resource/dashboard/hub/EntityTables.scala | 120 +++++++++--- .../web/resource/dashboard/hub/HubResource.scala | 204 +++++++++------------ .../dashboard/DatasetSearchQueryBuilderSpec.scala | 12 +- .../dashboard/UnifiedResourceSchemaSpec.scala | 30 +-- .../dashboard/VersionedResourceTablesSpec.scala | 140 ++++++++++++++ .../resource/dashboard/hub/EntityTablesSpec.scala | 47 ++++- .../dashboard/hub/HubEntityModelSpec.scala | 6 +- .../resource/dashboard/hub/HubResourceSpec.scala | 32 +++- 12 files changed, 686 insertions(+), 328 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilder.scala index 0cda3eecdc..ae8b89c371 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilder.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilder.scala @@ -19,149 +19,26 @@ package org.apache.texera.web.resource.dashboard -import com.typesafe.scalalogging.LazyLogging -import org.apache.texera.amber.core.storage.util.LakeFSStorageClient import org.apache.texera.dao.jooq.generated.Tables.{DATASET, DATASET_USER_ACCESS} -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum -import org.apache.texera.dao.jooq.generated.tables.User.USER -import org.apache.texera.dao.jooq.generated.tables.pojos.{Dataset, User} -import org.apache.texera.web.resource.dashboard.DashboardResource.DashboardClickableFileEntry -import org.apache.texera.web.resource.dashboard.FulltextSearchQueryUtils.{ - getContainsFilter, - getDateFilter, - getFullTextSearchFilter -} -import org.apache.texera.web.resource.dashboard.user.dataset.DatasetResource.DashboardDataset import org.jooq.impl.DSL -import org.jooq.{Condition, GroupField, Record, TableLike} -import scala.jdk.CollectionConverters.CollectionHasAsScala +/** Query logic lives in [[VersionedResourceSearchQueryBuilder]]; only the projection is here. */ +object DatasetSearchQueryBuilder + extends VersionedResourceSearchQueryBuilder(VersionedResourceTables.DatasetTables) { -object DatasetSearchQueryBuilder extends SearchQueryBuilder with LazyLogging { override protected val mappedResourceSchema: UnifiedResourceSchema = UnifiedResourceSchema( resourceType = DSL.inline(SearchQueryBuilder.DATASET_RESOURCE_TYPE), name = DATASET.NAME, description = DATASET.DESCRIPTION, creationTime = DATASET.CREATION_TIME, ownerId = DATASET.OWNER_UID, - did = DATASET.DID, + versionedResourceId = DATASET.DID, repositoryName = DATASET.REPOSITORY_NAME, - isDatasetPublic = DATASET.IS_PUBLIC, - isDatasetDownloadable = DATASET.IS_DOWNLOADABLE, - datasetUserAccess = DATASET_USER_ACCESS.PRIVILEGE, - datasetCoverImage = DATASET.COVER_IMAGE + isVersionedResourcePublic = DATASET.IS_PUBLIC, + isVersionedResourceDownloadable = DATASET.IS_DOWNLOADABLE, + versionedResourceUserAccess = DATASET_USER_ACCESS.PRIVILEGE, + versionedResourceCoverImage = DATASET.COVER_IMAGE ) - - /* - * constructs the FROM clause for querying datasets with specific access controls. - * - * Parameter: - * - uid: Integer - Represents the unique identifier of the current user. - * - uid is 'null' if the user is not logged in or performing a public search. - * - Otherwise, `uid` holds the identifier for the logged-in user. - * - includePublic - Boolean - Specifies whether to include public datasets in the result. - */ - override protected def constructFromClause( - uid: Integer, - params: DashboardResource.SearchQueryParams, - includePublic: Boolean = false - ): TableLike[_] = { - // Case 1: if `uid` is (set) and `includePublic` is false - // -> return ONLY datasets that given `uid` has explicit access to. - // Case 2: if `uid` is (null) and `includePublic` is true - // -> return ONLY datasets that are public - // Case 3: if `uid` is (set) and `includePublic` is true - // -> Union of datasets that are public and explicitly shared with user is returned - // Case 4: if `uid` is (null) and `includePublic` is false - // -> return public datasets by default as user might not be logged in - val baseJoin = DATASET - .leftJoin(DATASET_USER_ACCESS) - .on(DATASET_USER_ACCESS.DID.eq(DATASET.DID)) - .and(if (uid == null) DSL.falseCondition() else DATASET_USER_ACCESS.UID.eq(uid)) - .leftJoin(USER) - .on(USER.UID.eq(DATASET.OWNER_UID)) - - // Set the `condition` where clause here - val condition: Condition = - if (uid == null) { - // Case 2 and 4 - // Get all the public datasets by default - DATASET.IS_PUBLIC.eq(true) - } else { - if (includePublic) { - // Case 3 - // Get all the datasets that `uid` has access to and the public datasets - DATASET.IS_PUBLIC.eq(true).or(DATASET_USER_ACCESS.UID.isNotNull) - } else { - // Case 1 - // If `includePublic` is false get only user accessible datasets - DATASET_USER_ACCESS.UID.isNotNull - } - } - baseJoin.where(condition) - } - - override protected def constructWhereClause( - uid: Integer, - params: DashboardResource.SearchQueryParams - ): Condition = { - val splitKeywords = params.keywords.asScala - .flatMap(_.split("[+\\-()<>~*@\"]")) - .filter(_.nonEmpty) - .toSeq - - getDateFilter( - params.creationStartDate, - params.creationEndDate, - DATASET.CREATION_TIME - ) - .and(getContainsFilter(params.datasetIds, DATASET.DID)) - .and( - getFullTextSearchFilter(splitKeywords, List(DATASET.NAME, DATASET.DESCRIPTION)) - ) - } - - override protected def getGroupByFields: Seq[GroupField] = { - Seq.empty - } - - override protected def toEntryImpl( - uid: Integer, - record: Record - ): DashboardResource.DashboardClickableFileEntry = { - val dataset = record.into(DATASET).into(classOf[Dataset]) - val owner = record.into(USER).into(classOf[User]) - var size = 0L - - try { - size = LakeFSStorageClient.retrieveRepositorySize(dataset.getRepositoryName) - } catch { - case e: io.lakefs.clients.sdk.ApiException => - // Treat all LakeFS ApiException as mismatch (repository not found, being deleted, or any fatal error) - logger.error( - s"LakeFS ApiException for dataset repository '${dataset.getRepositoryName}': ${e.getMessage}", - e - ) - return null - } - - val dd = DashboardDataset( - dataset, - owner.getEmail, - Option( - record.get( - DATASET_USER_ACCESS.PRIVILEGE, - classOf[PrivilegeEnum] - ) - ).getOrElse(PrivilegeEnum.NONE), - dataset.getOwnerUid == uid, - size - ) - DashboardClickableFileEntry( - resourceType = SearchQueryBuilder.DATASET_RESOURCE_TYPE, - dataset = Some(dd) - ) - } } class DatasetSearchQueryBuilder {} diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala index aa3681ae1f..cfe3f381a4 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchema.scala @@ -70,13 +70,15 @@ object UnifiedResourceSchema { pid: Field[Integer] = DSL.cast(null, classOf[Integer]), projectOwnerId: Field[Integer] = DSL.cast(null, classOf[Integer]), projectColor: Field[String] = DSL.inline(""), - did: Field[Integer] = DSL.cast(null, classOf[Integer]), + versionedResourceId: Field[Integer] = DSL.cast(null, classOf[Integer]), datasetStoragePath: Field[String] = DSL.cast(null, classOf[String]), repositoryName: Field[String] = DSL.inline(""), - isDatasetPublic: Field[java.lang.Boolean] = DSL.cast(null, classOf[java.lang.Boolean]), - isDatasetDownloadable: Field[java.lang.Boolean] = DSL.cast(null, classOf[java.lang.Boolean]), - datasetUserAccess: Field[PrivilegeEnum] = DSL.castNull(classOf[PrivilegeEnum]), - datasetCoverImage: Field[String] = DSL.cast(null, classOf[String]), + isVersionedResourcePublic: Field[java.lang.Boolean] = + DSL.cast(null, classOf[java.lang.Boolean]), + isVersionedResourceDownloadable: Field[java.lang.Boolean] = + DSL.cast(null, classOf[java.lang.Boolean]), + versionedResourceUserAccess: Field[PrivilegeEnum] = DSL.castNull(classOf[PrivilegeEnum]), + versionedResourceCoverImage: Field[String] = DSL.cast(null, classOf[String]), workflowCoverImage: Field[String] = DSL.cast(null, classOf[String]) ): UnifiedResourceSchema = { new UnifiedResourceSchema( @@ -97,13 +99,17 @@ object UnifiedResourceSchema { pid -> pid.as("pid"), projectOwnerId -> projectOwnerId.as("owner_uid"), projectColor -> projectColor.as("color"), - did -> did.as("did"), + versionedResourceId -> versionedResourceId.as("versioned_resource_id"), datasetStoragePath -> datasetStoragePath.as("dataset_storage_path"), repositoryName -> repositoryName.as("repository_name"), - isDatasetPublic -> isDatasetPublic.as("is_dataset_public"), - isDatasetDownloadable -> isDatasetDownloadable.as("is_dataset_downloadable"), - datasetUserAccess -> datasetUserAccess.as("user_dataset_access"), - datasetCoverImage -> datasetCoverImage.as("cover_image"), + isVersionedResourcePublic -> isVersionedResourcePublic + .as("is_versioned_resource_public"), + isVersionedResourceDownloadable -> isVersionedResourceDownloadable + .as("is_versioned_resource_downloadable"), + versionedResourceUserAccess -> versionedResourceUserAccess + .as("user_versioned_resource_access"), + versionedResourceCoverImage -> versionedResourceCoverImage + .as("versioned_resource_cover_image"), workflowCoverImage -> workflowCoverImage.as("workflow_cover_image") ) ) @@ -141,13 +147,16 @@ object UnifiedResourceSchema { * - `fileSize`: Size of the file, as an `Integer`. * - `fileUserAccess`: Access privileges for the file, as a `UserFileAccessPrivilege`. * - * Attributes specific to datasets: - * - `did`: Dataset ID, as an `Integer`. + * Attributes shared by the LakeFS-backed resources (datasets, models). One row is one + * resource type, so each builder feeds these slots its own columns instead of duplicating + * them per resource; the aliases only line up positionally across the UNION ALL. + * - `versionedResourceId`: Dataset ID / model ID, as an `Integer`. * - `datasetStoragePath`: The storage path of the dataset, as a `String`. - * - `repositoryName`: The name of the repository where the dataset is stored, as a `String`. - * - `isDatasetPublic`: Indicates if the dataset is public, as a `Boolean`. - * - `isDatasetDownloadable`: Indicates if the dataset is downloadable, as a `Boolean`. - * - `datasetUserAccess`: Access privileges for the dataset, as a `PrivilegeEnum` + * - `repositoryName`: The name of the repository where the resource is stored, as a `String`. + * - `isVersionedResourcePublic`: Indicates if the resource is public, as a `Boolean`. + * - `isVersionedResourceDownloadable`: Indicates if the resource is downloadable, as a `Boolean`. + * - `versionedResourceUserAccess`: Access privileges for the resource, as a `PrivilegeEnum` + * - `versionedResourceCoverImage`: Cover image path of the resource, as a `String`. */ class UnifiedResourceSchema private ( fieldMappingSeq: Seq[(Field[_], Field[_])] diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala new file mode 100644 index 0000000000..a1e00ec45a --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceSearchQueryBuilder.scala @@ -0,0 +1,96 @@ +/* + * 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.web.resource.dashboard + +import org.apache.texera.web.resource.dashboard.FulltextSearchQueryUtils.{ + getContainsFilter, + getDateFilter, + getFullTextSearchFilter +} +import org.jooq.impl.DSL +import org.jooq.{Condition, GroupField, Record, TableLike} + +import scala.jdk.CollectionConverters.CollectionHasAsScala + +/** + * The one copy of FROM / WHERE / hydration for every LakeFS-backed resource. A concrete + * builder supplies only its [[VersionedResourceTables]] descriptor and its projection. + */ +abstract class VersionedResourceSearchQueryBuilder[Rec <: Record, P]( + tables: VersionedResourceTables[Rec, P] +) extends SearchQueryBuilder { + + /** + * `uid` is null for anonymous callers. Visibility: public only when `uid` is null; + * explicitly-granted only when `includePublic` is false; both when it is true. + */ + override protected def constructFromClause( + uid: Integer, + params: DashboardResource.SearchQueryParams, + includePublic: Boolean = false + ): TableLike[_] = { + val baseJoin = tables.joinWithAccessAndOwner( + Some(if (uid == null) DSL.falseCondition() else tables.access.uidColumn.eq(uid)) + ) + + val condition: Condition = + if (uid == null) { + tables.isPublicColumn.eq(true) + } else { + if (includePublic) { + tables.isPublicColumn.eq(true).or(tables.access.uidColumn.isNotNull) + } else { + tables.access.uidColumn.isNotNull + } + } + baseJoin.where(condition) + } + + override protected def constructWhereClause( + uid: Integer, + params: DashboardResource.SearchQueryParams + ): Condition = { + val splitKeywords = params.keywords.asScala + .flatMap(_.split("[+\\-()<>~*@\"]")) + .filter(_.nonEmpty) + .toSeq + + getDateFilter( + params.creationStartDate, + params.creationEndDate, + tables.creationTimeColumn + ) + .and(getContainsFilter(tables.searchIds(params), tables.idColumn)) + .and( + getFullTextSearchFilter(splitKeywords, List(tables.nameColumn, tables.descriptionColumn)) + ) + } + + override protected def getGroupByFields: Seq[GroupField] = { + Seq.empty + } + + override protected def toEntryImpl( + uid: Integer, + record: Record + ): DashboardResource.DashboardClickableFileEntry = + // null = mismatch; searchAllResources drops it and flips hasMismatch. + tables.hydrate(record, uid).map(_._2).orNull +} diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTables.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTables.scala new file mode 100644 index 0000000000..27287db7cc --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTables.scala @@ -0,0 +1,147 @@ +/* + * 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.web.resource.dashboard + +import com.typesafe.scalalogging.LazyLogging +import org.apache.texera.amber.core.storage.util.LakeFSStorageClient +import org.apache.texera.dao.jooq.generated.Tables.DATASET +import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.tables.User.USER +import org.apache.texera.dao.jooq.generated.tables.pojos.{Dataset, User} +import org.apache.texera.dao.jooq.generated.tables.records.DatasetRecord +import org.apache.texera.web.resource.dashboard.DashboardResource.{ + DashboardClickableFileEntry, + SearchQueryParams +} +import org.apache.texera.web.resource.dashboard.hub.EntityTables.{AccessTable, BaseEntityTable} +import org.apache.texera.web.resource.dashboard.user.dataset.DatasetResource.DashboardDataset +import org.jooq._ + +import java.sql.Timestamp + +/** + * Names the columns of one LakeFS-backed resource (dataset, model) so unified search and the + * hub are written once. A new resource type is one descriptor plus one `EntityTables` entry. + */ +trait VersionedResourceTables[Rec <: Record, P] extends BaseEntityTable with LazyLogging { + override type R = Rec + override val table: Table[Rec] + override val idColumn: TableField[Rec, Integer] + override val isPublicColumn: TableField[Rec, java.lang.Boolean] + + val resourceType: String + val nameColumn: TableField[Rec, String] + val descriptionColumn: TableField[Rec, String] + val creationTimeColumn: TableField[Rec, Timestamp] + val ownerUidColumn: TableField[Rec, Integer] + val access: AccessTable + + def searchIds(params: SearchQueryParams): java.util.List[Integer] + + def pojo(record: Record): P + def idOf(resource: P): Integer + def ownerUidOf(resource: P): Integer + def repositoryNameOf(resource: P): String + + def entry( + resource: P, + ownerEmail: String, + accessPrivilege: PrivilegeEnum, + isOwner: Boolean, + size: Long + ): DashboardClickableFileEntry + + /** `accessCondition` narrows the access join: search to the caller, the hub to nothing. */ + final def joinWithAccessAndOwner(accessCondition: Option[Condition]): Table[Record] = { + val accessJoin = table + .leftJoin(access.table) + .on(access.idColumn.eq(idColumn)) + val filtered = accessCondition.map(condition => accessJoin.and(condition)).getOrElse(accessJoin) + filtered + .leftJoin(USER) + .on(USER.UID.eq(ownerUidColumn)) + } + + /** `None` when LakeFS cannot size the resource; callers drop it and report a mismatch. */ + final def hydrate( + record: Record, + uid: Integer + ): Option[(Integer, DashboardClickableFileEntry)] = { + val resource = pojo(record) + repositorySize(resource).map { size => + idOf(resource) -> entry( + resource, + record.into(USER).into(classOf[User]).getEmail, + Option(record.get(access.privilegeColumn, classOf[PrivilegeEnum])) + .getOrElse(PrivilegeEnum.NONE), + ownerUidOf(resource) == uid, + size + ) + } + } + + final def repositorySize(resource: P): Option[Long] = { + val repositoryName = repositoryNameOf(resource) + try { + Some(LakeFSStorageClient.retrieveRepositorySize(repositoryName)) + } catch { + case e: io.lakefs.clients.sdk.ApiException => + logger.error( + s"LakeFS ApiException for $resourceType repository '$repositoryName': ${e.getMessage}", + e + ) + None + } + } +} + +object VersionedResourceTables { + + case object DatasetTables extends VersionedResourceTables[DatasetRecord, Dataset] { + override val resourceType: String = SearchQueryBuilder.DATASET_RESOURCE_TYPE + override val table: Table[DatasetRecord] = DATASET + override val idColumn: TableField[DatasetRecord, Integer] = DATASET.DID + override val isPublicColumn: TableField[DatasetRecord, java.lang.Boolean] = DATASET.IS_PUBLIC + override val nameColumn: TableField[DatasetRecord, String] = DATASET.NAME + override val descriptionColumn: TableField[DatasetRecord, String] = DATASET.DESCRIPTION + override val creationTimeColumn: TableField[DatasetRecord, Timestamp] = DATASET.CREATION_TIME + override val ownerUidColumn: TableField[DatasetRecord, Integer] = DATASET.OWNER_UID + override val access: AccessTable = AccessTable.DatasetAccessTable + + override def searchIds(params: SearchQueryParams): java.util.List[Integer] = params.datasetIds + + override def pojo(record: Record): Dataset = record.into(DATASET).into(classOf[Dataset]) + override def idOf(resource: Dataset): Integer = resource.getDid + override def ownerUidOf(resource: Dataset): Integer = resource.getOwnerUid + override def repositoryNameOf(resource: Dataset): String = resource.getRepositoryName + + override def entry( + resource: Dataset, + ownerEmail: String, + accessPrivilege: PrivilegeEnum, + isOwner: Boolean, + size: Long + ): DashboardClickableFileEntry = + DashboardClickableFileEntry( + resourceType = resourceType, + dataset = Some(DashboardDataset(resource, ownerEmail, accessPrivilege, isOwner, size)) + ) + } +} diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/EntityTables.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/EntityTables.scala index b152086d4f..4a2d749fc1 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/EntityTables.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/EntityTables.scala @@ -20,12 +20,62 @@ package org.apache.texera.web.resource.dashboard.hub import org.apache.texera.dao.jooq.generated.Tables._ +import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum import org.apache.texera.dao.jooq.generated.tables.records._ +import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.jooq._ object EntityTables { + + // ==================== THE REGISTRY ==================== + + /** Every table one hub entity type owns. A new entity type is one set plus one line in `apply`. */ + sealed trait EntityTableSet { + val base: BaseEntityTable + val like: LikeTable + val viewCount: ViewCountTable + val access: AccessTable + + /** + * Empty for cloneless entities (datasets, models): callers that can skip cloning read + * this and report zero, instead of `CloneTable.apply` throwing. Not named `clone` + * because `Object.clone` owns that name. + */ + val cloneTable: Option[CloneTable] + + /** Set for LakeFS-backed resources, so the hub can hydrate without a per-type branch. */ + val versionedResource: Option[VersionedResourceTables[_ <: Record, _]] + } + + case object WorkflowTableSet extends EntityTableSet { + override val base: BaseEntityTable = BaseEntityTable.WorkflowTable + override val like: LikeTable = LikeTable.WorkflowLikeTable + override val viewCount: ViewCountTable = ViewCountTable.WorkflowViewCountTable + override val access: AccessTable = AccessTable.WorkflowAccessTable + override val cloneTable: Option[CloneTable] = Some(CloneTable.WorkflowCloneTable) + override val versionedResource: Option[VersionedResourceTables[_ <: Record, _]] = None + } + + case object DatasetTableSet extends EntityTableSet { + override val base: BaseEntityTable = VersionedResourceTables.DatasetTables + override val like: LikeTable = LikeTable.DatasetLikeTable + override val viewCount: ViewCountTable = ViewCountTable.DatasetViewCountTable + override val access: AccessTable = AccessTable.DatasetAccessTable + override val cloneTable: Option[CloneTable] = None + override val versionedResource: Option[VersionedResourceTables[_ <: Record, _]] = Some( + VersionedResourceTables.DatasetTables + ) + } + + def apply(entityType: EntityType): EntityTableSet = + entityType match { + case EntityType.Workflow => WorkflowTableSet + case EntityType.Dataset => DatasetTableSet + } + // ==================== BASE TABLE ==================== - sealed trait BaseEntityTable { + // Not sealed: VersionedResourceTables implements it, so id/is_public are named once. + trait BaseEntityTable { type R <: Record val table: Table[R] val isPublicColumn: TableField[R, java.lang.Boolean] @@ -41,18 +91,7 @@ object EntityTables { override val idColumn: TableField[WorkflowRecord, Integer] = WORKFLOW.WID } - case object DatasetTable extends BaseEntityTable { - override type R = DatasetRecord - override val table: Table[DatasetRecord] = DATASET - override val isPublicColumn: TableField[DatasetRecord, java.lang.Boolean] = DATASET.IS_PUBLIC - override val idColumn: TableField[DatasetRecord, Integer] = DATASET.DID - } - - def apply(entityType: EntityType): BaseEntityTable = - entityType match { - case EntityType.Workflow => WorkflowTable - case EntityType.Dataset => DatasetTable - } + def apply(entityType: EntityType): BaseEntityTable = EntityTables(entityType).base } // ==================== BASE LC (like & clone) TABLE ==================== @@ -83,11 +122,7 @@ object EntityTables { override val idColumn: TableField[DatasetUserLikesRecord, Integer] = DATASET_USER_LIKES.DID } - def apply(entityType: EntityType): LikeTable = - entityType match { - case EntityType.Workflow => WorkflowLikeTable - case EntityType.Dataset => DatasetLikeTable - } + def apply(entityType: EntityType): LikeTable = EntityTables(entityType).like } // ==================== CLONE TABLE ==================== @@ -103,12 +138,11 @@ object EntityTables { WORKFLOW_USER_CLONES.WID } + /** For callers that cannot proceed without one; see `EntityTableSet.cloneTable`. */ def apply(entityType: EntityType): CloneTable = - entityType match { - case EntityType.Workflow => WorkflowCloneTable - case _ => - throw new IllegalArgumentException(s"Unsupported entity type: $entityType for clone") - } + EntityTables(entityType).cloneTable.getOrElse( + throw new IllegalArgumentException(s"Unsupported entity type: $entityType for clone") + ) } // ==================== VIEW COUNT TABLE ==================== @@ -136,10 +170,40 @@ object EntityTables { DATASET_VIEW_COUNT.VIEW_COUNT } - def apply(entityType: EntityType): ViewCountTable = - entityType match { - case EntityType.Workflow => WorkflowViewCountTable - case EntityType.Dataset => DatasetViewCountTable - } + def apply(entityType: EntityType): ViewCountTable = EntityTables(entityType).viewCount + } + + // ==================== ACCESS TABLE ==================== + /** The `<entity>_user_access` sibling table, replacing the inline match in HubResource. */ + sealed trait AccessTable { + type R <: Record + val table: Table[R] + val idColumn: TableField[R, Integer] + val uidColumn: TableField[R, Integer] + val privilegeColumn: TableField[R, PrivilegeEnum] + } + + object AccessTable { + case object WorkflowAccessTable extends AccessTable { + override type R = WorkflowUserAccessRecord + override val table: Table[WorkflowUserAccessRecord] = WORKFLOW_USER_ACCESS + override val idColumn: TableField[WorkflowUserAccessRecord, Integer] = + WORKFLOW_USER_ACCESS.WID + override val uidColumn: TableField[WorkflowUserAccessRecord, Integer] = + WORKFLOW_USER_ACCESS.UID + override val privilegeColumn: TableField[WorkflowUserAccessRecord, PrivilegeEnum] = + WORKFLOW_USER_ACCESS.PRIVILEGE + } + + case object DatasetAccessTable extends AccessTable { + override type R = DatasetUserAccessRecord + override val table: Table[DatasetUserAccessRecord] = DATASET_USER_ACCESS + override val idColumn: TableField[DatasetUserAccessRecord, Integer] = DATASET_USER_ACCESS.DID + override val uidColumn: TableField[DatasetUserAccessRecord, Integer] = DATASET_USER_ACCESS.UID + override val privilegeColumn: TableField[DatasetUserAccessRecord, PrivilegeEnum] = + DATASET_USER_ACCESS.PRIVILEGE + } + + def apply(entityType: EntityType): AccessTable = EntityTables(entityType).access } } diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala index 33ba16c6e1..0dd3cfc1b6 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala @@ -19,30 +19,24 @@ package org.apache.texera.web.resource.dashboard.hub -import com.typesafe.scalalogging.Logger import io.dropwizard.auth.Auth -import org.apache.texera.amber.core.storage.util.LakeFSStorageClient import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.dao.jooq.generated.enums.ActionEnum -import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET -import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS import org.apache.texera.dao.jooq.generated.tables.User.USER -import org.apache.texera.dao.jooq.generated.tables.pojos.{Dataset, DatasetUserAccess} import org.apache.texera.web.resource.dashboard.DashboardResource.DashboardClickableFileEntry +import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.apache.texera.web.resource.dashboard.hub.ActionType.{Clone, Like, Unlike, View} import org.apache.texera.web.resource.dashboard.hub.EntityTables._ import org.apache.texera.web.resource.dashboard.hub.HubResource._ -import org.apache.texera.web.resource.dashboard.user.dataset.DatasetResource.DashboardDataset import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.{ DashboardWorkflow, baseWorkflowSelect, mapWorkflowEntries } -import org.jooq.Table import org.jooq.impl.DSL -import org.slf4j.LoggerFactory +import org.jooq.{Record, Table, TableField} import java.util.regex.Pattern import javax.servlet.http.HttpServletRequest @@ -50,10 +44,8 @@ import javax.ws.rs._ import javax.ws.rs.core.{Context, MediaType} import scala.collection.mutable.ListBuffer import scala.jdk.CollectionConverters._ -import scala.language.existentials object HubResource { - private lazy val logger: Logger = Logger(LoggerFactory.getLogger(getClass.getName)) // Represents an entity reference for general-purpose batch APIs. // Used by: isLikedHelper, recordLikeAction, getCounts, userAccess @@ -120,7 +112,7 @@ object HubResource { .groupBy(_.entityType) .foreach { case (etype, groupReqs) => - val tbl = LikeTable(etype) + val tbl = EntityTables(etype).like val ids = groupReqs.map(_.entityId) val likedSet: Set[Int] = context @@ -191,7 +183,7 @@ object HubResource { ): Boolean = { val (entityId, entityType) = (userRequest.entityId, userRequest.entityType) - val entityTables = LikeTable(entityType) + val entityTables = EntityTables(entityType).like val (table, uidColumn, idColumn) = (entityTables.table, entityTables.uidColumn, entityTables.idColumn) @@ -282,59 +274,40 @@ object HubResource { mapWorkflowEntries(records, uid) } - def fetchDashboardDatasetsByDids(dids: Seq[Integer], uid: Integer): List[DashboardDataset] = { - if (dids.isEmpty) { - return List.empty[DashboardDataset] + /** + * Hydrates ids of one LakeFS-backed resource into hub entries. Unsizable resources are + * dropped; ids are de-duplicated because the access join can match one twice. + */ + def fetchDashboardVersionedResourcesByIds( + tables: VersionedResourceTables[_ <: Record, _], + ids: Seq[Integer], + uid: Integer + ): List[DashboardClickableFileEntry] = { + if (ids.isEmpty) { + return List.empty[DashboardClickableFileEntry] } val records = context .select() - .from( - DATASET - .leftJoin(DATASET_USER_ACCESS) - .on(DATASET_USER_ACCESS.DID.eq(DATASET.DID)) - .leftJoin(USER) - .on(USER.UID.eq(DATASET.OWNER_UID)) - ) - .where(DATASET.DID.in(dids: _*)) + .from(tables.joinWithAccessAndOwner(None)) + .where(tables.idColumn.in(ids: _*)) .groupBy( - DATASET.DID, - DATASET.NAME, - DATASET.DESCRIPTION, - DATASET.OWNER_UID, + tables.idColumn, + tables.nameColumn, + tables.descriptionColumn, + tables.ownerUidColumn, USER.NAME, - DATASET_USER_ACCESS.DID, - DATASET_USER_ACCESS.UID, + tables.access.idColumn, + tables.access.uidColumn, USER.UID ) .fetch() records.asScala - .flatMap { record => - val dataset = record.into(DATASET).into(classOf[Dataset]) - val datasetAccess = record.into(DATASET_USER_ACCESS).into(classOf[DatasetUserAccess]) - val ownerEmail = record.into(USER).getEmail - try { - Some( - DashboardDataset( - isOwner = if (uid == null) false else dataset.getOwnerUid == uid, - dataset = dataset, - accessPrivilege = datasetAccess.getPrivilege, - ownerEmail = ownerEmail, - size = LakeFSStorageClient.retrieveRepositorySize(dataset.getRepositoryName) - ) - ) - } catch { - case e: io.lakefs.clients.sdk.ApiException => - logger.error( - s"LakeFS ApiException for dataset repository '${dataset.getRepositoryName}': ${e.getMessage}", - e - ) - None - } - } + .flatMap(record => tables.hydrate(record, uid)) .toList - .distinctBy(_.dataset.getDid) + .distinctBy(_._1) + .map(_._2) } } @@ -349,7 +322,7 @@ class HubResource { @GET @Path("/count") def getCount(@QueryParam("entityType") entityType: EntityType): Integer = { - val entityTables = BaseEntityTable(entityType) + val entityTables = EntityTables(entityType).base val (table, isPublicColumn) = (entityTables.table, entityTables.isPublicColumn) context @@ -425,7 +398,7 @@ class HubResource { * Unified endpoint to fetch the top N (here N = 8) public entities for a given entity type, * grouped by specified action types, with optional user context. * - * @param entityType The EntityType enum value (Workflow or Dataset) to query. + * @param entityType The EntityType enum value (Workflow, Dataset) to query. * @param actionTypes Optional list of ActionType enums to include (Like, Clone). * If omitted or empty, defaults to [Like, Clone]. * @param uid Optional user ID (Integer) for user-specific context. @@ -445,7 +418,8 @@ class HubResource { @QueryParam("uid") uid: Integer, @QueryParam("limit") limit: Integer ): java.util.Map[String, java.util.List[DashboardClickableFileEntry]] = { - val baseTable = BaseEntityTable(entityType) + val tableSet = EntityTables(entityType) + val baseTable = tableSet.base val isPublicColumn = baseTable.isPublicColumn val baseIdColumn = baseTable.idColumn val topN: Int = Option(limit).filter(_ > 0).map(_.intValue).getOrElse(8) @@ -462,53 +436,50 @@ class HubResource { val result: Map[String, java.util.List[DashboardClickableFileEntry]] = types.map { act => - val (table, idColumn) = act match { + val rankedBy: Option[(Table[_], TableField[_, Integer])] = act match { case ActionType.Like => - val lt = LikeTable(entityType) - (lt.table, lt.idColumn) + val lt = tableSet.like + Some((lt.table, lt.idColumn)) case ActionType.Clone => - val ct = CloneTable(entityType) - (ct.table, ct.idColumn) + tableSet.cloneTable.map(ct => (ct.table, ct.idColumn)) case other => throw new BadRequestException( s"Unsupported actionType: '$other'. Supported: [like, clone]" ) } - val topIds: Seq[Integer] = context - .select(idColumn) - .from(table) - .join(baseTable.table) - .on(idColumn.eq(baseIdColumn)) - .where(isPublicColumn.eq(true)) - .groupBy(idColumn) - .orderBy(DSL.count(idColumn).desc()) - .limit(topN) - .fetchInto(classOf[Integer]) - .asScala - .toSeq + val topIds: Seq[Integer] = rankedBy.toSeq.flatMap { + case (table, idColumn) => + context + .select(idColumn) + .from(table) + .join(baseTable.table) + .on(idColumn.eq(baseIdColumn)) + .where(isPublicColumn.eq(true)) + .groupBy(idColumn) + .orderBy(DSL.count(idColumn).desc()) + .limit(topN) + .fetchInto(classOf[Integer]) + .asScala + .toSeq + } val entries: Seq[DashboardClickableFileEntry] = - if (entityType == EntityType.Workflow) { - fetchDashboardWorkflowsByWids(topIds, currentUid).map { w => - DashboardClickableFileEntry( - resourceType = entityType.value, - workflow = Some(w), - project = None, - dataset = None - ) - } - } else if (entityType == EntityType.Dataset) { - fetchDashboardDatasetsByDids(topIds, currentUid).map { d => - DashboardClickableFileEntry( - resourceType = entityType.value, - workflow = None, - project = None, - dataset = Some(d) - ) - } - } else { - Seq.empty + tableSet.versionedResource match { + case Some(versionedResource) => + fetchDashboardVersionedResourcesByIds(versionedResource, topIds, currentUid) + case None => + entityType match { + case EntityType.Workflow => + fetchDashboardWorkflowsByWids(topIds, currentUid).map { w => + DashboardClickableFileEntry( + resourceType = entityType.value, + workflow = Some(w) + ) + } + case other => + throw new BadRequestException(s"getTops is not supported for '$other'") + } } act.value -> entries.toList.asJava @@ -577,7 +548,8 @@ class HubResource { grouped.foreach { case (etype, ids) => - val viewTbl = ViewCountTable(etype) + val tableSet = EntityTables(etype) + val viewTbl = tableSet.viewCount val viewMap: Map[Int, Int] = if (requestedActions.contains(ActionType.View)) { val raw = context @@ -603,7 +575,7 @@ class HubResource { raw ++ missing.map(id => id.intValue() -> 0).toMap } else Map.empty - val likeTbl = LikeTable(etype) + val likeTbl = tableSet.like val likeMap: Map[Int, Int] = if (requestedActions.contains(ActionType.Like)) { context @@ -621,21 +593,23 @@ class HubResource { } else Map.empty val cloneMap: Map[Int, Int] = - if (requestedActions.contains(ActionType.Clone) && etype != EntityType.Dataset) { - val cloneTbl = CloneTable(etype) - context - .select(cloneTbl.idColumn, DSL.count().`as`("cnt")) - .from(cloneTbl.table) - .where(cloneTbl.idColumn.in(ids: _*)) - .groupBy(cloneTbl.idColumn) - .fetch() - .asScala - .map { r => - r.get(cloneTbl.idColumn).intValue() -> - r.get("cnt", classOf[Integer]).intValue() - } - .toMap - } else Map.empty + tableSet.cloneTable + .filter(_ => requestedActions.contains(ActionType.Clone)) + .map { cloneTbl => + context + .select(cloneTbl.idColumn, DSL.count().`as`("cnt")) + .from(cloneTbl.table) + .where(cloneTbl.idColumn.in(ids: _*)) + .groupBy(cloneTbl.idColumn) + .fetch() + .asScala + .map { r => + r.get(cloneTbl.idColumn).intValue() -> + r.get("cnt", classOf[Integer]).intValue() + } + .toMap + } + .getOrElse(Map.empty) reqs.filter(_.entityType == etype).foreach { req => val key = req.entityId.intValue() @@ -675,18 +649,14 @@ class HubResource { val reqs = entityIds.asScala .zip(entityTypes.asScala) - .map { case (et, id) => UserRequest(et, id) } + .map { case (id, etype) => UserRequest(id, etype) } .toList val responses = ListBuffer[AccessResponse]() reqs.groupBy(_.entityType).foreach { case (etype, groupReqs) => - val (tbl, idCol, uidCol) = etype match { - case EntityType.Workflow => - (WORKFLOW_USER_ACCESS: Table[_], WORKFLOW_USER_ACCESS.WID, WORKFLOW_USER_ACCESS.UID) - case EntityType.Dataset => - (DATASET_USER_ACCESS: Table[_], DATASET_USER_ACCESS.DID, DATASET_USER_ACCESS.UID) - } + val access = EntityTables(etype).access + val (tbl, idCol, uidCol) = (access.table, access.idColumn, access.uidColumn) val records = context .select(idCol, uidCol) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala index 338a2b3651..cc60e1853e 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala @@ -485,12 +485,14 @@ class DatasetSearchQueryBuilderSpec sql should include("dataset.description as resourcedescription") sql should include("dataset.creation_time as resourcecreationtime") sql should include("dataset.owner_uid as resourceownerid") - sql should include("dataset.did as did") sql should include("dataset.repository_name as repository_name") - sql should include("dataset.is_public as is_dataset_public") - sql should include("dataset.is_downloadable as is_dataset_downloadable") - sql should include("dataset_user_access.privilege as user_dataset_access") - sql should include("dataset.cover_image as cover_image") + // The id/publicity/privilege/cover slots are shared with the other versioned resources, + // so their aliases are resource-neutral while the columns feeding them are the dataset's. + sql should include("dataset.did as versioned_resource_id") + sql should include("dataset.is_public as is_versioned_resource_public") + sql should include("dataset.is_downloadable as is_versioned_resource_downloadable") + sql should include("dataset_user_access.privilege as user_versioned_resource_access") + sql should include("dataset.cover_image as versioned_resource_cover_image") } it should "join the owner row on the dataset's owner" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala index 9d5f1e0b0b..52890b4bf0 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/UnifiedResourceSchemaSpec.scala @@ -101,13 +101,13 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { pid = PROJECT.PID, projectOwnerId = PROJECT.OWNER_ID, projectColor = PROJECT.COLOR, - did = DATASET.DID, + versionedResourceId = DATASET.DID, datasetStoragePath = sentinelStoragePath, repositoryName = DATASET.REPOSITORY_NAME, - isDatasetPublic = DATASET.IS_PUBLIC, - isDatasetDownloadable = DATASET.IS_DOWNLOADABLE, - datasetUserAccess = DATASET_USER_ACCESS.PRIVILEGE, - datasetCoverImage = DATASET.COVER_IMAGE, + isVersionedResourcePublic = DATASET.IS_PUBLIC, + isVersionedResourceDownloadable = DATASET.IS_DOWNLOADABLE, + versionedResourceUserAccess = DATASET_USER_ACCESS.PRIVILEGE, + versionedResourceCoverImage = DATASET.COVER_IMAGE, workflowCoverImage = WORKFLOW_COVER_IMAGE.IMAGE ) @@ -129,13 +129,13 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "pid" -> PROJECT.PID, "owner_uid" -> PROJECT.OWNER_ID, "color" -> PROJECT.COLOR, - "did" -> DATASET.DID, + "versioned_resource_id" -> DATASET.DID, "dataset_storage_path" -> sentinelStoragePath, "repository_name" -> DATASET.REPOSITORY_NAME, - "is_dataset_public" -> DATASET.IS_PUBLIC, - "is_dataset_downloadable" -> DATASET.IS_DOWNLOADABLE, - "user_dataset_access" -> DATASET_USER_ACCESS.PRIVILEGE, - "cover_image" -> DATASET.COVER_IMAGE, + "is_versioned_resource_public" -> DATASET.IS_PUBLIC, + "is_versioned_resource_downloadable" -> DATASET.IS_DOWNLOADABLE, + "user_versioned_resource_access" -> DATASET_USER_ACCESS.PRIVILEGE, + "versioned_resource_cover_image" -> DATASET.COVER_IMAGE, "workflow_cover_image" -> WORKFLOW_COVER_IMAGE.IMAGE ) @@ -167,7 +167,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { rendered should include("'' as \"resourceType\"") rendered should include("cast(null as timestamp) as \"resourceCreationTime\"") rendered should include("cast(null as int) as \"resourceOwnerId\"") - rendered should include("cast(null as boolean) as \"is_dataset_public\"") + rendered should include("cast(null as boolean) as \"is_versioned_resource_public\"") } // -- the de-dup behind translatedFieldSet ----------------------------------- @@ -209,7 +209,7 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "resourceOwnerId", // cast(null as int) "workflow_privilege", // cast(null as privilege_enum) "dataset_storage_path", // cast(null as varchar) - "is_dataset_public" // cast(null as boolean) + "is_versioned_resource_public" // cast(null as boolean) ) } @@ -229,10 +229,10 @@ class UnifiedResourceSchemaSpec extends AnyFlatSpec with Matchers { "uid", "owner_uid", "color", - "did", + "versioned_resource_id", "repository_name", - "is_dataset_downloadable", - "cover_image" + "is_versioned_resource_downloadable", + "versioned_resource_cover_image" ) aliases should contain("resourceOwnerId") } diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTablesSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTablesSpec.scala new file mode 100644 index 0000000000..a509f03352 --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/VersionedResourceTablesSpec.scala @@ -0,0 +1,140 @@ +/* + * 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.web.resource.dashboard + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.tables.pojos.Dataset +import org.apache.texera.web.resource.dashboard.DashboardResource.SearchQueryParams +import org.apache.texera.web.resource.dashboard.hub.EntityTables +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.jdk.CollectionConverters._ + +/** + * The descriptor is the only place a resource's columns are named, so a slot wired to the + * wrong column still compiles and runs — it just queries the wrong field. SQL is rendered, + * never executed. `repositorySize` is uncovered: live LakeFS call, no mockable seam. + */ +class VersionedResourceTablesSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private val tables = VersionedResourceTables.DatasetTables + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + } + + override protected def afterAll(): Unit = { + shutdownDB() + } + + private def rendered(sql: String): String = sql.toLowerCase.replace("\"", "") + + private def dataset(did: Int, ownerUid: Int, repositoryName: String): Dataset = { + val d = new Dataset + d.setDid(Integer.valueOf(did)) + d.setOwnerUid(Integer.valueOf(ownerUid)) + d.setRepositoryName(repositoryName) + d + } + + "DatasetTables" should "name every column the shared query logic reads" in { + tables.table.getName shouldBe "dataset" + tables.idColumn.getName shouldBe "did" + tables.isPublicColumn.getName shouldBe "is_public" + tables.nameColumn.getName shouldBe "name" + tables.descriptionColumn.getName shouldBe "description" + tables.creationTimeColumn.getName shouldBe "creation_time" + tables.ownerUidColumn.getName shouldBe "owner_uid" + } + + it should "share the hub's access table rather than naming it twice" in { + tables.access shouldBe EntityTables.AccessTable.DatasetAccessTable + } + + it should "tag its rows with the resource type DashboardResource dispatches on" in { + // DashboardResource matches this literal with no default branch. + tables.resourceType shouldBe SearchQueryBuilder.DATASET_RESOURCE_TYPE + tables.resourceType shouldBe "dataset" + } + + it should "read its id filter out of the shared search params" in { + // A sibling's param list here would silently apply the wrong filter. + val ids = List(Integer.valueOf(7), Integer.valueOf(9)).asJava + tables.searchIds(SearchQueryParams(datasetIds = ids)) shouldBe ids + tables.searchIds(SearchQueryParams()) shouldBe empty + } + + it should "expose the POJO fields the hub de-dupes and sizes by" in { + val d = dataset(11, 22, "dataset-11") + tables.idOf(d) shouldBe Integer.valueOf(11) + tables.ownerUidOf(d) shouldBe Integer.valueOf(22) + tables.repositoryNameOf(d) shouldBe "dataset-11" + } + + "entry" should "wrap the resource in the slot the frontend reads it from" in { + val d = dataset(11, 22, "dataset-11") + val entry = tables.entry(d, "[email protected]", PrivilegeEnum.READ, isOwner = true, size = 512L) + + entry.resourceType shouldBe "dataset" + entry.workflow shouldBe None + entry.project shouldBe None + val dashboardDataset = entry.dataset.getOrElse(fail("expected a dataset entry")) + dashboardDataset.dataset shouldBe d + dashboardDataset.ownerEmail shouldBe "[email protected]" + dashboardDataset.accessPrivilege shouldBe PrivilegeEnum.READ + dashboardDataset.isOwner shouldBe true + dashboardDataset.size shouldBe 512L + } + + "joinWithAccessAndOwner" should "join the access rows and the owner" in { + // Rendered names are schema-qualified (texera_db.dataset.did), hence the regex. + val sql = rendered(getDSLContext.render(tables.joinWithAccessAndOwner(None))) + + sql should include regex "dataset_user_access\\.did = [\\w.]*dataset\\.did" + sql should include regex "user\\.uid = [\\w.]*dataset\\.owner_uid" + // Both joins are outer: a resource with no grant row must still appear. + sql.split("left outer join").length shouldBe 3 + } + + it should "leave the access rows unfiltered when no condition is given" in { + // The hub's call: every grant row. + val sql = rendered(getDSLContext.render(tables.joinWithAccessAndOwner(None))) + + sql should not include "dataset_user_access.uid =" + } + + it should "narrow the access rows when a condition is given" in { + // The search's call. Without the predicate, `uid is not null` matches anybody's grant. + val sql = rendered( + getDSLContext.renderInlined( + tables.joinWithAccessAndOwner(Some(tables.access.uidColumn.eq(Integer.valueOf(42)))) + ) + ) + + sql should include("dataset_user_access.uid = 42") + } +} diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/EntityTablesSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/EntityTablesSpec.scala index 5be87fdbd0..f60f92ebd9 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/EntityTablesSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/EntityTablesSpec.scala @@ -19,6 +19,7 @@ package org.apache.texera.web.resource.dashboard.hub +import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -31,9 +32,10 @@ class EntityTablesSpec extends AnyFlatSpec with Matchers { EntityTables.BaseEntityTable.WorkflowTable } - it should "dispatch Dataset → DatasetTable" in { + it should "dispatch Dataset → the dataset's search descriptor" in { + // Datasets have no BaseEntityTable object: VersionedResourceTables implements it. EntityTables.BaseEntityTable(EntityType.Dataset) shouldBe - EntityTables.BaseEntityTable.DatasetTable + VersionedResourceTables.DatasetTables } "BaseEntityTable.WorkflowTable" should "wire up id and isPublic columns from WORKFLOW" in { @@ -42,8 +44,8 @@ class EntityTablesSpec extends AnyFlatSpec with Matchers { t.isPublicColumn.getName shouldBe "is_public" } - "BaseEntityTable.DatasetTable" should "wire up id and isPublic columns from DATASET" in { - val t = EntityTables.BaseEntityTable.DatasetTable + "VersionedResourceTables.DatasetTables" should "wire up id and isPublic columns from DATASET" in { + val t = VersionedResourceTables.DatasetTables t.idColumn.getName shouldBe "did" t.isPublicColumn.getName shouldBe "is_public" } @@ -79,8 +81,7 @@ class EntityTablesSpec extends AnyFlatSpec with Matchers { it should "throw IllegalArgumentException for Dataset because there is no DatasetClone table" in { // The asymmetry is intentional today: dataset clones aren't a modelled - // entity. Pinning the exception so a future addition of DatasetCloneTable - // forces this spec to be updated alongside the new dispatch branch. + // entity. CloneTable.apply stays for recordCloneAction. val ex = intercept[IllegalArgumentException] { EntityTables.CloneTable(EntityType.Dataset) } @@ -88,6 +89,40 @@ class EntityTablesSpec extends AnyFlatSpec with Matchers { ex.getMessage should include("clone") } + // -- the registry ----------------------------------------------------------- + + "EntityTables.apply" should "expose every table an entity type owns" in { + val workflow = EntityTables(EntityType.Workflow) + workflow.base shouldBe EntityTables.BaseEntityTable.WorkflowTable + workflow.like shouldBe EntityTables.LikeTable.WorkflowLikeTable + workflow.viewCount shouldBe EntityTables.ViewCountTable.WorkflowViewCountTable + workflow.access shouldBe EntityTables.AccessTable.WorkflowAccessTable + workflow.cloneTable shouldBe Some(EntityTables.CloneTable.WorkflowCloneTable) + workflow.versionedResource shouldBe None + + val dataset = EntityTables(EntityType.Dataset) + dataset.base shouldBe VersionedResourceTables.DatasetTables + dataset.like shouldBe EntityTables.LikeTable.DatasetLikeTable + dataset.viewCount shouldBe EntityTables.ViewCountTable.DatasetViewCountTable + dataset.access shouldBe EntityTables.AccessTable.DatasetAccessTable + dataset.cloneTable shouldBe None + dataset.versionedResource shouldBe Some(VersionedResourceTables.DatasetTables) + } + + // -- AccessTable ------------------------------------------------------------ + + "EntityTables.AccessTable" should "expose id, uid and privilege per entity" in { + val w = EntityTables.AccessTable(EntityType.Workflow) + w.idColumn.getName shouldBe "wid" + w.uidColumn.getName shouldBe "uid" + w.privilegeColumn.getName shouldBe "privilege" + + val d = EntityTables.AccessTable(EntityType.Dataset) + d.idColumn.getName shouldBe "did" + d.uidColumn.getName shouldBe "uid" + d.privilegeColumn.getName shouldBe "privilege" + } + // -- ViewCountTable --------------------------------------------------------- "EntityTables.ViewCountTable.apply" should "dispatch Workflow → WorkflowViewCountTable" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubEntityModelSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubEntityModelSpec.scala index 2924cf54b4..03a0908532 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubEntityModelSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubEntityModelSpec.scala @@ -21,6 +21,7 @@ package org.apache.texera.web.resource.dashboard.hub import org.apache.texera.amber.util.JSONUtils.objectMapper import org.apache.texera.dao.jooq.generated.Tables._ +import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.scalatest.flatspec.AnyFlatSpec class HubEntityModelSpec extends AnyFlatSpec { @@ -164,9 +165,10 @@ class HubEntityModelSpec extends AnyFlatSpec { assert(t.idColumn == WORKFLOW.WID) } - it should "dispatch Dataset → DatasetTable" in { + it should "dispatch Dataset → the dataset's search descriptor" in { + // Datasets have no BaseEntityTable object: VersionedResourceTables implements it. val t = EntityTables.BaseEntityTable(EntityType.Dataset) - assert(t == EntityTables.BaseEntityTable.DatasetTable) + assert(t == VersionedResourceTables.DatasetTables) assert(t.table == DATASET) assert(t.isPublicColumn == DATASET.IS_PUBLIC) assert(t.idColumn == DATASET.DID) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala index ffd860541a..c7f7c772a4 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala @@ -35,6 +35,7 @@ import org.apache.texera.dao.jooq.generated.tables.pojos.{ WorkflowOfUser, WorkflowUserAccess } +import org.apache.texera.web.resource.dashboard.VersionedResourceTables import org.apache.texera.web.resource.dashboard.hub.HubResource._ import org.scalamock.scalatest.MockFactory import org.scalatest.BeforeAndAfterAll @@ -715,8 +716,6 @@ class HubResourceSpec seedDatasetViewCount(820801, 7) Seq(ownerUid, likerUid, thirdUid).foreach(seedDatasetLike(820801, _)) - // CloneTable(Dataset) throws; the `etype != Dataset` guard is what keeps this - // request from turning into a 500. val counts = hub.getCounts(types(Ds), ids(820801), null).asScala.head.counts counts.asScala.toMap shouldBe Map( @@ -760,6 +759,22 @@ class HubResourceSpec hub.getTops(Wf, null, null, null).asScala.keySet shouldBe Set("like", "clone") } + // Regression: the default bucket list ([like, clone]) used to drive getTops into a + // throwing CloneTable(Dataset), making this plain request a 500. + it should "answer for a cloneless entity type instead of failing on its absent clone table" in { + val tops = hub.getTops(Ds, null, null, null).asScala + + tops.keySet shouldBe Set("like", "clone") + tops("clone").asScala shouldBe empty + } + + it should "return an empty clone bucket when clone is asked for explicitly" in { + seedDataset(820850, "ds_no_clones") + seedDatasetLike(820850, likerUid) + + hub.getTops(Ds, actions(ActionType.Clone), null, null).get("clone").asScala shouldBe empty + } + it should "select the most-liked public workflows and honour the limit" in { val hot = seedWorkflow(810701, "wf_hot") val warm = seedWorkflow(810702, "wf_warm") @@ -914,11 +929,12 @@ class HubResourceSpec fetchDashboardWorkflowsByWids(Seq(id), null).head.isOwner shouldBe false } - // fetchDashboardDatasetsByDids calls LakeFSStorageClient for every did it resolves, - // so an empty id list is the only input exercisable without a LakeFS server. This - // pins the contract (empty in, empty out), not the early-return branch itself: the - // query would also come back empty if the guard were removed. - "fetchDashboardDatasetsByDids" should "return an empty list for an empty did list" in { - fetchDashboardDatasetsByDids(Seq.empty, Integer.valueOf(ownerUid)) shouldBe empty + // Every resolved id hits LakeFS, so an empty list is the only input testable without one. + "fetchDashboardVersionedResourcesByIds" should "return an empty list for an empty id list" in { + fetchDashboardVersionedResourcesByIds( + VersionedResourceTables.DatasetTables, + Seq.empty, + Integer.valueOf(ownerUid) + ) shouldBe empty } }
