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..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,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 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)