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-7745-bf0e7779ecbe64a918beadf20181306105449677 in repository https://gitbox.apache.org/repos/asf/texera.git
commit c927890160a8701c1b8ecb4fc4cc17711ea011ad Author: Meng Wang <[email protected]> AuthorDate: Wed Aug 19 20:51:05 2026 +0000 feat(amber): expose the warehouse owner in DashboardWarehouse (#7745) ### What changes were proposed in this PR? `GET /warehouse/status` returns `DashboardWarehouse(whid, name, warehouseName, flavor, createdAtMillis)` with no owner information, so the warehouse dashboard tab and picker (#6933) can only render the owner avatar from the currently signed-in user. That is correct only while warehouses are strictly per-user, and would show the wrong person as soon as warehouses can be shared. Computing units already model this properly: `DashboardWorkflowComputingUnit` carries `ownerName` / `ownerAvatar` resolved per entry. - Add `ownerName` and `ownerAvatar` to `DashboardWarehouse`, mirroring the computing-unit semantics: resolved from the user table per entry, **null** when the user has no name or avatar set. - Resolution is batched over the distinct owner uids of a listing (one query per request) — today every entry belongs to the caller, but the shape is ready for shared warehouses, which is the point of the change. - Both mapping paths are wired: the `status()` listing and the `create()` response. - Frontend is deliberately untouched: the tab/picker PR (#7536) is still open and can bind to the new fields directly. ### Any related issues, documentation, discussions? Closes #7743. Part of #6870, follow-up to #6932; mirrors `DashboardWorkflowComputingUnit`'s owner fields. ### How was this PR tested? - `WarehouseResourceSpec` (embedded Postgres + stubbed `LakekeeperClient`, no external infra) asserts the fields on both mapping paths: `create` returns the caller's `ownerName` with a **null** `ownerAvatar` for the avatar-less fixture user, and `status` resolves another user's name and avatar per entry. - **Teeth verified**: temporarily breaking the owner resolution turns exactly the two owner assertions red (both mapping paths), confirming the tests catch a regression rather than passing vacuously. - Full spec run locally: 11/11 passed; `WorkflowExecutionService/scalafmtCheck` (main + Test) passes. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-4-8) --- .../user/warehouse/WarehouseResource.scala | 52 ++++++++++++++++++---- .../user/warehouse/WarehouseResourceSpec.scala | 15 +++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala index d4aed3ccdc..cb196ceb2f 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala @@ -21,12 +21,14 @@ package org.apache.texera.web.resource.dashboard.user.warehouse import com.typesafe.scalalogging.LazyLogging import io.dropwizard.auth.Auth +import org.apache.commons.lang3.StringUtils import org.apache.texera.amber.core.storage.VFSURIFactory import org.apache.texera.auth.SessionUser import org.apache.texera.common.config.StorageConfig import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.Tables.USER_WAREHOUSE +import org.apache.texera.dao.jooq.generated.Tables.{USER, USER_WAREHOUSE} import org.apache.texera.dao.jooq.generated.enums.UserWarehouseFlavorEnum +import org.apache.texera.dao.jooq.generated.tables.pojos.User import org.apache.texera.dao.jooq.generated.tables.records.UserWarehouseRecord import org.apache.texera.web.resource.dashboard.user.warehouse.WarehouseResource._ import org.apache.texera.web.service.LakekeeperClient @@ -34,6 +36,7 @@ import org.apache.texera.web.service.LakekeeperClient import javax.annotation.security.RolesAllowed import javax.ws.rs._ import javax.ws.rs.core.MediaType +import scala.jdk.CollectionConverters.ListHasAsScala object WarehouseResource { private def context = @@ -53,16 +56,34 @@ object WarehouseResource { name: String, warehouseName: String, flavor: String, - createdAtMillis: Long + createdAtMillis: Long, + // Owner display info, mirroring DashboardWorkflowComputingUnit: today every + // warehouse belongs to the caller, but the UI binds to the entry rather than + // the session user so shared warehouses render the right person (#7743). + ownerName: String, + ownerAvatar: String ) - private def toDashboardWarehouse(row: UserWarehouseRecord): DashboardWarehouse = + // (name, avatar), null for either when the user has not set it. + private type Owner = (String, String) + + private def ownerOf(name: String, avatar: String): Owner = + (StringUtils.trimToNull(name), StringUtils.trimToNull(avatar)) + + private def ownerOf(user: User): Owner = ownerOf(user.getName, user.getAvatar) + + private def toDashboardWarehouse( + row: UserWarehouseRecord, + owner: Owner + ): DashboardWarehouse = DashboardWarehouse( row.getWhid, row.getName, row.getWarehouseName, row.getFlavor.getLiteral, - row.getCreatedAt.toInstant.toEpochMilli + row.getCreatedAt.toInstant.toEpochMilli, + ownerName = owner._1, + ownerAvatar = owner._2 ) case class WarehouseStatus(enabled: Boolean, warehouses: List[DashboardWarehouse]) @@ -94,15 +115,26 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy if (!enabled) { return WarehouseStatus(enabled = false, warehouses = List()) } - val warehouses = context - .selectFrom(USER_WAREHOUSE) + // Joined rather than resolved in a second query: one round trip, and every row + // carries its own owner once warehouses can be shared. + val rows = context + .select(USER_WAREHOUSE.fields() ++ Seq(USER.NAME, USER.AVATAR): _*) + .from(USER_WAREHOUSE) + .leftJoin(USER) + .on(USER.UID.eq(USER_WAREHOUSE.UID)) .where(USER_WAREHOUSE.UID.eq(current_user.getUid)) .orderBy(USER_WAREHOUSE.CREATED_AT.asc()) .fetch() - .map(row => toDashboardWarehouse(row)) + .asScala + .toList WarehouseStatus( enabled = true, - warehouses = warehouses.toArray(Array[DashboardWarehouse]()).toList + warehouses = rows.map(r => + toDashboardWarehouse( + r.into(USER_WAREHOUSE), + ownerOf(r.get(USER.NAME), r.get(USER.AVATAR)) + ) + ) ) } @@ -169,7 +201,9 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy } throw new WebApplicationException(e.getMessage, 500) } - toDashboardWarehouse(row) + // The caller owns what they just created, and SessionUser already carries their + // display info -- no lookup needed. + toDashboardWarehouse(row, ownerOf(current_user.getUser)) } @DELETE diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResourceSpec.scala index 1d3aeb9eed..58d4f2e9c8 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResourceSpec.scala @@ -91,6 +91,7 @@ class WarehouseResourceSpec val other = new User other.setName("warehouse_spec_other") other.setEmail(s"user_${UUID.randomUUID()}@example.com") + other.setAvatar("other-avatar.png") userDao.insert(other) otherUser = new SessionUser(other) } @@ -132,12 +133,26 @@ class WarehouseResourceSpec created.warehouseName shouldBe s"user-${sessionUser.getUid}-mybucket" created.flavor shouldBe "local" createdNames.toList shouldBe List(s"user-${sessionUser.getUid}-mybucket") + created.ownerName shouldBe "warehouse_spec_user" + // The fixture user has no avatar set; the DTO carries null, not "" (#7743). + created.ownerAvatar shouldBe null val status = resource.status(sessionUser) status.enabled shouldBe true status.warehouses.map(_.whid) shouldBe List(created.whid) } + it should "resolve each entry's owner name and avatar for the dashboard" in { + // Mirrors DashboardWorkflowComputingUnit: the UI binds to the entry's owner, + // not the session user, so shared warehouses will render the right person. + resource.create(CreateWarehouseRequest("theirs"), otherUser) + + val entries = resource.status(otherUser).warehouses + entries should have size 1 + entries.head.ownerName shouldBe "warehouse_spec_other" + entries.head.ownerAvatar shouldBe "other-avatar.png" + } + it should "reject an unsafe or duplicate name" in { a[BadRequestException] should be thrownBy resource.create(CreateWarehouseRequest("a/b"), sessionUser)
