From 6f77fcda266891268448e112528fd6c420500994 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:38:27 -0700 Subject: [PATCH 1/3] feat(amber): expose the warehouse owner in DashboardWarehouse GET /warehouse/status returned DashboardWarehouse without owner information, so the warehouse dashboard tab and picker could only render the owner avatar from the currently signed-in user -- correct only while warehouses are strictly per-user, and wrong as soon as they can be shared. Mirror how computing units model this: add ownerName / ownerAvatar to DashboardWarehouse, resolved per entry from the user table (null when the user has no name or avatar set), batched over the distinct owner uids of a listing. The UI can then bind each entry to its own owner instead of the session user. WarehouseResourceSpec asserts the fields on both mapping paths: create returns the caller's name with a null avatar for the avatar-less fixture user, and status resolves another user's name and avatar per entry. Part of #6870, follow-up to #6932. Closes #7743. --- .../user/warehouse/WarehouseResource.scala | 51 ++++++++++++++++--- .../warehouse/WarehouseResourceSpec.scala | 15 ++++++ 2 files changed, 58 insertions(+), 8 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 d4aed3ccdcf..6ea4012f3dc 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 @@ -25,7 +25,7 @@ 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.records.UserWarehouseRecord import org.apache.texera.web.resource.dashboard.user.warehouse.WarehouseResource._ @@ -34,6 +34,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 +54,47 @@ 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) per uid; null when the user has no name / avatar set, matching + // how computing units resolve their owner info. + private def resolveOwners(uids: Seq[Integer]): Map[Integer, (String, String)] = + if (uids.isEmpty) Map.empty + else + context + .select(USER.UID, USER.NAME, USER.AVATAR) + .from(USER) + .where(USER.UID.in(uids: _*)) + .fetch() + .asScala + .map(r => + r.get(USER.UID) -> ( + ( + Option(r.get(USER.NAME)).filter(_.nonEmpty).orNull, + Option(r.get(USER.AVATAR)).filter(_.nonEmpty).orNull + ) + ) + ) + .toMap + + private def toDashboardWarehouse( + row: UserWarehouseRecord, + owner: (String, String) + ): 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 +126,18 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy if (!enabled) { return WarehouseStatus(enabled = false, warehouses = List()) } - val warehouses = context + val rows = context .selectFrom(USER_WAREHOUSE) .where(USER_WAREHOUSE.UID.eq(current_user.getUid)) .orderBy(USER_WAREHOUSE.CREATED_AT.asc()) .fetch() - .map(row => toDashboardWarehouse(row)) + .asScala + .toList + val owners = resolveOwners(rows.map(_.getUid).distinct) WarehouseStatus( enabled = true, - warehouses = warehouses.toArray(Array[DashboardWarehouse]()).toList + warehouses = + rows.map(row => toDashboardWarehouse(row, owners.getOrElse(row.getUid, (null, null)))) ) } @@ -169,7 +204,7 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy } throw new WebApplicationException(e.getMessage, 500) } - toDashboardWarehouse(row) + toDashboardWarehouse(row, resolveOwners(Seq(uid)).getOrElse(uid, (null, null))) } @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 1d3aeb9eed2..58d4f2e9c89 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) From efa90521aafb5123976460c35c0aa3bf8b92ef6f Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:04:11 -0700 Subject: [PATCH 2/3] refactor(amber): resolve the warehouse owner in the listing query Review feedback on the owner fields: The listing fetched warehouses and then looked their owners up in a second query. Join instead, so one round trip carries both. `create` went to the database at all, though the caller is the owner it is resolving and SessionUser already holds their display info -- it now reads from there. What remains of the resolution is turning an unset name or avatar into null, so that is all `ownerOf` does. The `(null, null)` fallback a reviewer asked to name is gone rather than named: with the left join supplying nulls directly and `create` reading the session, nothing is left that has to invent an absent owner. --- .../user/warehouse/WarehouseResource.scala | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 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 6ea4012f3dc..cabb4c5caaf 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 @@ -27,6 +27,7 @@ import org.apache.texera.common.config.StorageConfig import org.apache.texera.dao.SqlServer 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 @@ -62,30 +63,17 @@ object WarehouseResource { ownerAvatar: String ) - // (name, avatar) per uid; null when the user has no name / avatar set, matching - // how computing units resolve their owner info. - private def resolveOwners(uids: Seq[Integer]): Map[Integer, (String, String)] = - if (uids.isEmpty) Map.empty - else - context - .select(USER.UID, USER.NAME, USER.AVATAR) - .from(USER) - .where(USER.UID.in(uids: _*)) - .fetch() - .asScala - .map(r => - r.get(USER.UID) -> ( - ( - Option(r.get(USER.NAME)).filter(_.nonEmpty).orNull, - Option(r.get(USER.AVATAR)).filter(_.nonEmpty).orNull - ) - ) - ) - .toMap + // (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 = + (Option(name).filter(_.nonEmpty).orNull, Option(avatar).filter(_.nonEmpty).orNull) + + private def ownerOf(user: User): Owner = ownerOf(user.getName, user.getAvatar) private def toDashboardWarehouse( row: UserWarehouseRecord, - owner: (String, String) + owner: Owner ): DashboardWarehouse = DashboardWarehouse( row.getWhid, @@ -126,18 +114,26 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy if (!enabled) { return WarehouseStatus(enabled = false, warehouses = List()) } + // 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 - .selectFrom(USER_WAREHOUSE) + .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() .asScala .toList - val owners = resolveOwners(rows.map(_.getUid).distinct) WarehouseStatus( enabled = true, - warehouses = - rows.map(row => toDashboardWarehouse(row, owners.getOrElse(row.getUid, (null, null)))) + warehouses = rows.map(r => + toDashboardWarehouse( + r.into(USER_WAREHOUSE), + ownerOf(r.get(USER.NAME), r.get(USER.AVATAR)) + ) + ) ) } @@ -204,7 +200,9 @@ class WarehouseResource(client: LakekeeperClient, enabled: Boolean) extends Lazy } throw new WebApplicationException(e.getMessage, 500) } - toDashboardWarehouse(row, resolveOwners(Seq(uid)).getOrElse(uid, (null, null))) + // The caller owns what they just created, and SessionUser already carries their + // display info -- no lookup needed. + toDashboardWarehouse(row, ownerOf(current_user.getUser)) } @DELETE From a0a0e295f27db9a8070d350fe16dbbdd773e9ef2 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:33:48 -0700 Subject: [PATCH 3/3] refactor(amber): collapse blank owner fields with StringUtils.trimToNull --- .../resource/dashboard/user/warehouse/WarehouseResource.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 cabb4c5caaf..cb196ceb2f7 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,6 +21,7 @@ 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 @@ -67,7 +68,7 @@ object WarehouseResource { private type Owner = (String, String) private def ownerOf(name: String, avatar: String): Owner = - (Option(name).filter(_.nonEmpty).orNull, Option(avatar).filter(_.nonEmpty).orNull) + (StringUtils.trimToNull(name), StringUtils.trimToNull(avatar)) private def ownerOf(user: User): Owner = ownerOf(user.getName, user.getAvatar)