From d0d4288734679a61154b1fe25be0860e52bbfa38 Mon Sep 17 00:00:00 2001
From: Sebastian Stenzel <sebastian.stenzel@skymatic.de>
Date: Wed, 6 Sep 2023 11:27:50 +0200
Subject: [PATCH] fix n+1 select in `/api/vaults/{vaultId}/members`

---
 .../org/cryptomator/hub/api/VaultResource.java    | 14 ++++++--------
 .../org/cryptomator/hub/entities/VaultAccess.java | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
index 2fced67e4..95be03fa0 100644
--- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
+++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
@@ -128,17 +128,15 @@ public List<VaultDto> getAllVaults() {
 	@VaultRole(VaultAccess.Role.OWNER) // may throw 403
 	@Transactional
 	@Produces(MediaType.APPLICATION_JSON)
-	@Operation(summary = "list vault members", description = "list all users that this vault has been shared with")
+	@Operation(summary = "list vault members", description = "list all users or groups that this vault has been shared with directly (not inherited via group membership)")
 	@APIResponse(responseCode = "200")
 	@APIResponse(responseCode = "403", description = "not a vault owner")
-	public List<MemberDto> getMembers(@PathParam("vaultId") UUID vaultId) {
-		var vault = Vault.<Vault>findById(vaultId); // should always be found, since @VaultRole filter would have triggered
-
-		return vault.directMembers.stream().map(authority -> {
-			VaultAccess access = VaultAccess.findById(new VaultAccess.Id(vaultId, authority.id)); // TODO: inefficient, would be better if role is already part of the query result
-			if (authority instanceof User u) {
+	public List<MemberDto> getDirectMembers(@PathParam("vaultId") UUID vaultId) {
+		return VaultAccess.forVault(vaultId).map(access -> {
+			// TODO switch to switch expressions, once we can make Authority sealed
+			if (access.authority instanceof User u) {
 				return MemberDto.fromEntity(u, access.role);
-			} else if (authority instanceof Group g) {
+			} else if (access.authority instanceof Group g) {
 				return MemberDto.fromEntity(g, access.role);
 			} else {
 				throw new IllegalStateException();
diff --git a/backend/src/main/java/org/cryptomator/hub/entities/VaultAccess.java b/backend/src/main/java/org/cryptomator/hub/entities/VaultAccess.java
index b445cac14..132040b84 100644
--- a/backend/src/main/java/org/cryptomator/hub/entities/VaultAccess.java
+++ b/backend/src/main/java/org/cryptomator/hub/entities/VaultAccess.java
@@ -1,6 +1,7 @@
 package org.cryptomator.hub.entities;
 
 import io.quarkus.hibernate.orm.panache.PanacheEntityBase;
+import io.quarkus.panache.common.Parameters;
 import jakarta.persistence.Column;
 import jakarta.persistence.Embeddable;
 import jakarta.persistence.EmbeddedId;
@@ -10,14 +11,24 @@
 import jakarta.persistence.JoinColumn;
 import jakarta.persistence.ManyToOne;
 import jakarta.persistence.MapsId;
+import jakarta.persistence.NamedQuery;
 import jakarta.persistence.Table;
 
 import java.io.Serializable;
 import java.util.Objects;
 import java.util.UUID;
+import java.util.stream.Stream;
 
 @Entity
 @Table(name = "vault_access")
+@NamedQuery(name = "VaultAccess.forVault",
+		query = """
+				SELECT va
+				FROM VaultAccess va
+				INNER JOIN FETCH va.vault
+				INNER JOIN FETCH va.authority
+				WHERE va.id.vaultId = :vaultId
+				""")
 public class VaultAccess extends PanacheEntityBase {
 
 	@EmbeddedId
@@ -49,6 +60,10 @@ public enum Role {
 		OWNER
 	}
 
+	public static Stream<VaultAccess> forVault(UUID vaultId) {
+		return find("#VaultAccess.forVault", Parameters.with("vaultId", vaultId)).stream();
+	}
+
 	@Embeddable
 	public static class Id implements Serializable {