From a15a9587b87b754814526c8d51cd6ff422d88b66 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:45:12 +0800 Subject: [PATCH] refactor: optimize user query using index (#5396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core /milestone 2.13.x #### What this PR does / why we need it: 使用索引机制优化用户查询以提高性能 #### Does this PR introduce a user-facing change? ```release-note 使用索引机制优化用户查询以提高性能 ``` --- .../run/halo/app/core/extension/User.java | 2 + .../extension/endpoint/StatsEndpoint.java | 27 +++-- .../core/extension/endpoint/UserEndpoint.java | 108 +++++++----------- .../DefaultInitializationStateGetter.java | 23 ++-- .../run/halo/app/infra/SchemeInitializer.java | 22 +++- .../extension/endpoint/UserEndpointTest.java | 62 +--------- .../infra/InitializationStateGetterTest.java | 6 +- .../modules/system/users/UserList.vue | 8 +- 8 files changed, 102 insertions(+), 156 deletions(-) diff --git a/api/src/main/java/run/halo/app/core/extension/User.java b/api/src/main/java/run/halo/app/core/extension/User.java index a3c89c9cc..1d796b3eb 100644 --- a/api/src/main/java/run/halo/app/core/extension/User.java +++ b/api/src/main/java/run/halo/app/core/extension/User.java @@ -33,6 +33,8 @@ public class User extends AbstractExtension { public static final String VERSION = "v1alpha1"; public static final String KIND = "User"; + public static final String USER_RELATED_ROLES_INDEX = "roles"; + public static final String ROLE_NAMES_ANNO = "rbac.authorization.halo.run/role-names"; public static final String EMAIL_TO_VERIFY = "halo.run/email-to-verify"; diff --git a/application/src/main/java/run/halo/app/core/extension/endpoint/StatsEndpoint.java b/application/src/main/java/run/halo/app/core/extension/endpoint/StatsEndpoint.java index 2102d0bb4..6bb1ab295 100644 --- a/application/src/main/java/run/halo/app/core/extension/endpoint/StatsEndpoint.java +++ b/application/src/main/java/run/halo/app/core/extension/endpoint/StatsEndpoint.java @@ -1,6 +1,5 @@ package run.halo.app.core.extension.endpoint; -import static java.lang.Boolean.parseBoolean; import static org.springdoc.core.fn.builders.apiresponse.Builder.responseBuilder; import static run.halo.app.extension.index.query.QueryFactory.and; import static run.halo.app.extension.index.query.QueryFactory.equal; @@ -17,10 +16,10 @@ import run.halo.app.core.extension.Counter; import run.halo.app.core.extension.User; import run.halo.app.core.extension.content.Post; import run.halo.app.extension.ListOptions; -import run.halo.app.extension.MetadataUtil; import run.halo.app.extension.PageRequestImpl; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.router.selector.FieldSelector; +import run.halo.app.extension.router.selector.LabelSelector; /** * Stats endpoint. @@ -61,18 +60,18 @@ public class StatsEndpoint implements CustomEndpoint { stats.setUpvotes(stats.getUpvotes() + counter.getUpvote()); return stats; }) - .flatMap(stats -> client.list(User.class, - user -> { - var labels = MetadataUtil.nullSafeLabels(user); - return user.getMetadata().getDeletionTimestamp() == null - && !parseBoolean(labels.getOrDefault(User.HIDDEN_USER_LABEL, "false")); - }, - null) - .count() - .map(count -> { - stats.setUsers(count.intValue()); - return stats; - })) + .flatMap(stats -> { + var listOptions = new ListOptions(); + listOptions.setLabelSelector(LabelSelector.builder() + .notEq(User.HIDDEN_USER_LABEL, "true") + .build() + ); + listOptions.setFieldSelector( + FieldSelector.of(isNull("metadata.deletionTimestamp"))); + return client.listBy(User.class, listOptions, PageRequestImpl.ofSize(1)) + .doOnNext(result -> stats.setUsers((int) result.getTotal())) + .thenReturn(stats); + }) .flatMap(stats -> { var listOptions = new ListOptions(); listOptions.setFieldSelector(FieldSelector.of( diff --git a/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java b/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java index 2650839de..b56a6147d 100644 --- a/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java +++ b/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java @@ -1,7 +1,6 @@ package run.halo.app.core.extension.endpoint; import static io.swagger.v3.oas.annotations.media.Schema.RequiredMode.REQUIRED; -import static java.util.Comparator.comparing; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static org.apache.commons.lang3.StringUtils.defaultIfBlank; import static org.springdoc.core.fn.builders.apiresponse.Builder.responseBuilder; @@ -11,8 +10,12 @@ import static org.springdoc.core.fn.builders.requestbody.Builder.requestBodyBuil import static org.springdoc.core.fn.builders.schema.Builder.schemaBuilder; import static org.springframework.web.reactive.function.server.RequestPredicates.contentType; import static run.halo.app.extension.ListResult.generateGenericClass; +import static run.halo.app.extension.index.query.QueryFactory.and; +import static run.halo.app.extension.index.query.QueryFactory.contains; +import static run.halo.app.extension.index.query.QueryFactory.equal; +import static run.halo.app.extension.index.query.QueryFactory.or; import static run.halo.app.extension.router.QueryParamBuildUtil.buildParametersFromType; -import static run.halo.app.extension.router.selector.SelectorUtil.labelAndFieldSelectorToPredicate; +import static run.halo.app.extension.router.selector.SelectorUtil.labelAndFieldSelectorToListOptions; import static run.halo.app.security.authorization.AuthorityUtils.authoritiesToRoles; import com.fasterxml.jackson.core.type.TypeReference; @@ -25,9 +28,7 @@ import io.swagger.v3.oas.annotations.media.ArraySchema; import io.swagger.v3.oas.annotations.media.Schema; import java.security.Principal; import java.time.Duration; -import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; @@ -37,7 +38,6 @@ import java.util.Objects; import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Predicate; import java.util.stream.Collectors; import lombok.Data; import lombok.RequiredArgsConstructor; @@ -76,12 +76,14 @@ import run.halo.app.core.extension.service.AttachmentService; import run.halo.app.core.extension.service.EmailVerificationService; import run.halo.app.core.extension.service.RoleService; import run.halo.app.core.extension.service.UserService; -import run.halo.app.extension.Comparators; +import run.halo.app.extension.ListOptions; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; import run.halo.app.extension.MetadataUtil; +import run.halo.app.extension.PageRequestImpl; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.router.IListRequest; +import run.halo.app.extension.router.selector.FieldSelector; import run.halo.app.infra.AnonymousUserConst; import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; import run.halo.app.infra.SystemSetting; @@ -676,7 +678,7 @@ public class UserEndpoint implements CustomEndpoint { } - public class ListRequest extends IListRequest.QueryListRequest { + public static class ListRequest extends IListRequest.QueryListRequest { private final ServerWebExchange exchange; @@ -703,63 +705,38 @@ public class UserEndpoint implements CustomEndpoint { implementation = String.class, example = "creationTimestamp,desc")) public Sort getSort() { - return SortResolver.defaultInstance.resolve(exchange); + var sort = SortResolver.defaultInstance.resolve(exchange); + sort = sort.and(Sort.by("metadata.creationTimestamp", "metadata.name").descending()); + return sort; } /** - * Converts query parameters to user predicate. - * - * @return user predicate to filter users + * Converts query parameters to list options. */ - public Predicate toPredicate() { - Predicate keywordPredicate = user -> { - var keyword = getKeyword(); - if (StringUtils.isBlank(keyword)) { - return true; - } - var username = user.getMetadata().getName(); - var displayName = user.getSpec().getDisplayName(); - return StringUtils.containsIgnoreCase(displayName, keyword) - || keyword.equalsIgnoreCase(username); - }; + public ListOptions toListOptions() { + var listOptions = + labelAndFieldSelectorToListOptions(getLabelSelector(), getFieldSelector()); - Predicate rolePredicate = user -> { - var roleName = getRole(); - if (StringUtils.isBlank(roleName)) { - return true; - } - var roleNamesAnno = MetadataUtil.nullSafeAnnotations(user) - .get(User.ROLE_NAMES_ANNO); - if (StringUtils.isBlank(roleNamesAnno)) { - return false; - } - Set roleNames = JsonUtils.jsonToObject(roleNamesAnno, - new TypeReference<>() { - }); - return roleNames.contains(roleName); - }; - return keywordPredicate - .and(rolePredicate) - .and(labelAndFieldSelectorToPredicate(getLabelSelector(), getFieldSelector())); - } - - public Comparator toComparator() { - var sort = getSort(); - var ctOrder = sort.getOrderFor("creationTimestamp"); - List> comparators = new ArrayList<>(); - if (ctOrder != null) { - Comparator comparator = - comparing(user -> user.getMetadata().getCreationTimestamp()); - if (ctOrder.isDescending()) { - comparator = comparator.reversed(); - } - comparators.add(comparator); + var fieldQuery = listOptions.getFieldSelector().query(); + if (StringUtils.isNotBlank(getKeyword())) { + fieldQuery = and( + fieldQuery, + or( + contains("spec.displayName", getKeyword()), + equal("metadata.name", getKeyword()) + ) + ); } - comparators.add(Comparators.compareCreationTimestamp(false)); - comparators.add(Comparators.compareName(true)); - return comparators.stream() - .reduce(Comparator::thenComparing) - .orElse(null); + + if (StringUtils.isNotBlank(getRole())) { + fieldQuery = and( + fieldQuery, + equal(User.USER_RELATED_ROLES_INDEX, getRole()) + ); + } + + listOptions.setFieldSelector(FieldSelector.of(fieldQuery)); + return listOptions; } } @@ -770,15 +747,12 @@ public class UserEndpoint implements CustomEndpoint { Mono list(ServerRequest request) { return Mono.just(request) .map(UserEndpoint.ListRequest::new) - .flatMap(listRequest -> { - var predicate = listRequest.toPredicate(); - var comparator = listRequest.toComparator(); - return client.list(User.class, - predicate, - comparator, - listRequest.getPage(), - listRequest.getSize()); - }) + .flatMap(listRequest -> client.listBy(User.class, listRequest.toListOptions(), + PageRequestImpl.of( + listRequest.getPage(), listRequest.getSize(), + listRequest.getSort() + ) + )) .flatMap(this::toListedUser) .flatMap(listResult -> ServerResponse.ok().bodyValue(listResult)); } diff --git a/application/src/main/java/run/halo/app/infra/DefaultInitializationStateGetter.java b/application/src/main/java/run/halo/app/infra/DefaultInitializationStateGetter.java index fea5c5a70..fafed520a 100644 --- a/application/src/main/java/run/halo/app/infra/DefaultInitializationStateGetter.java +++ b/application/src/main/java/run/halo/app/infra/DefaultInitializationStateGetter.java @@ -1,6 +1,7 @@ package run.halo.app.infra; import static org.apache.commons.lang3.BooleanUtils.isTrue; +import static run.halo.app.extension.index.query.QueryFactory.isNull; import java.util.concurrent.atomic.AtomicBoolean; import lombok.RequiredArgsConstructor; @@ -8,8 +9,11 @@ import org.springframework.stereotype.Component; import reactor.core.publisher.Mono; import run.halo.app.core.extension.User; import run.halo.app.extension.ConfigMap; -import run.halo.app.extension.MetadataUtil; +import run.halo.app.extension.ListOptions; +import run.halo.app.extension.PageRequestImpl; import run.halo.app.extension.ReactiveExtensionClient; +import run.halo.app.extension.router.selector.FieldSelector; +import run.halo.app.extension.router.selector.LabelSelector; /** *

A cache that caches system setup state.

@@ -50,16 +54,15 @@ public class DefaultInitializationStateGetter implements InitializationStateGett } private Mono hasUser() { - return client.list(User.class, - user -> { - var labels = MetadataUtil.nullSafeLabels(user); - return isNotTrue(labels.get("halo.run/hidden-user")); - }, null, 1, 10) + var listOptions = new ListOptions(); + listOptions.setLabelSelector(LabelSelector.builder() + .notEq(User.HIDDEN_USER_LABEL, "true") + .build() + ); + listOptions.setFieldSelector( + FieldSelector.of(isNull("metadata.deletionTimestamp"))); + return client.listBy(User.class, listOptions, PageRequestImpl.ofSize(1)) .map(result -> result.getTotal() > 0) .defaultIfEmpty(false); } - - static boolean isNotTrue(String value) { - return !Boolean.parseBoolean(value); - } } diff --git a/application/src/main/java/run/halo/app/infra/SchemeInitializer.java b/application/src/main/java/run/halo/app/infra/SchemeInitializer.java index 8fb6349a9..57cf54e61 100644 --- a/application/src/main/java/run/halo/app/infra/SchemeInitializer.java +++ b/application/src/main/java/run/halo/app/infra/SchemeInitializer.java @@ -4,6 +4,7 @@ import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; import static run.halo.app.extension.index.IndexAttributeFactory.multiValueAttribute; import static run.halo.app.extension.index.IndexAttributeFactory.simpleAttribute; +import com.fasterxml.jackson.core.type.TypeReference; import java.util.Set; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; @@ -44,9 +45,11 @@ import run.halo.app.core.extension.notification.Subscription; import run.halo.app.extension.ConfigMap; import run.halo.app.extension.DefaultSchemeManager; import run.halo.app.extension.DefaultSchemeWatcherManager; +import run.halo.app.extension.MetadataUtil; import run.halo.app.extension.Secret; import run.halo.app.extension.index.IndexSpec; import run.halo.app.extension.index.IndexSpecRegistryImpl; +import run.halo.app.infra.utils.JsonUtils; import run.halo.app.migration.Backup; import run.halo.app.plugin.extensionpoint.ExtensionDefinition; import run.halo.app.plugin.extensionpoint.ExtensionPointDefinition; @@ -69,7 +72,24 @@ public class SchemeInitializer implements ApplicationListener { + indexSpecs.add(new IndexSpec() + .setName("spec.displayName") + .setIndexFunc( + simpleAttribute(User.class, user -> user.getSpec().getDisplayName()))); + indexSpecs.add(new IndexSpec() + .setName(User.USER_RELATED_ROLES_INDEX) + .setIndexFunc(multiValueAttribute(User.class, user -> { + var roleNamesAnno = MetadataUtil.nullSafeAnnotations(user) + .get(User.ROLE_NAMES_ANNO); + if (StringUtils.isBlank(roleNamesAnno)) { + return Set.of(); + } + return JsonUtils.jsonToObject(roleNamesAnno, + new TypeReference<>() { + }); + }))); + }); schemeManager.register(ReverseProxy.class); schemeManager.register(Setting.class); schemeManager.register(AnnotationSetting.class); diff --git a/application/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java b/application/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java index af2dc8c7b..f33612406 100644 --- a/application/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java +++ b/application/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java @@ -2,10 +2,8 @@ package run.halo.app.core.extension.endpoint; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.mock; @@ -18,11 +16,9 @@ import static org.springframework.test.web.reactive.server.WebTestClient.bindToR import static run.halo.app.extension.GroupVersionKind.fromExtension; import java.time.Instant; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -48,6 +44,7 @@ import run.halo.app.core.extension.service.RoleService; import run.halo.app.core.extension.service.UserService; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; +import run.halo.app.extension.PageRequest; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.extension.exception.ExtensionNotFoundException; import run.halo.app.infra.SystemConfigurableEnvironmentFetcher; @@ -91,7 +88,7 @@ class UserEndpointTest { @Test void shouldListEmptyUsersWhenNoUsers() { - when(client.list(same(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(same(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.just(ListResult.emptyResult())); bindToRouterFunction(endpoint.endpoint()) @@ -113,7 +110,7 @@ class UserEndpointTest { ); var expectResult = new ListResult<>(users); when(roleService.list(anySet())).thenReturn(Flux.empty()); - when(client.list(same(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(same(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.just(expectResult)); bindToRouterFunction(endpoint.endpoint()) @@ -141,35 +138,11 @@ class UserEndpointTest { } } """, User.class); - var unexpectedUser1 = - JsonUtils.jsonToObject(""" - { - "apiVersion": "v1alpha1", - "kind": "User", - "metadata": { - "name": "admin", - "annotations": { - "rbac.authorization.halo.run/role-names": "[\\"super-role\\"]" - } - } - } - """, User.class); - var unexpectedUser2 = - JsonUtils.jsonToObject(""" - { - "apiVersion": "v1alpha1", - "kind": "User", - "metadata": { - "name": "joey", - "annotations": {} - } - } - """, User.class); var users = List.of( expectUser ); var expectResult = new ListResult<>(users); - when(client.list(same(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(same(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.just(expectResult)); when(roleService.list(anySet())).thenReturn(Flux.empty()); @@ -178,24 +151,14 @@ class UserEndpointTest { .get().uri("/users?role=guest") .exchange() .expectStatus().isOk(); - - verify(client).list(same(User.class), argThat( - predicate -> predicate.test(expectUser) - && !predicate.test(unexpectedUser1) - && !predicate.test(unexpectedUser2)), - any(), anyInt(), anyInt()); } @Test void shouldSortUsersWhenCreationTimestampSet() { var expectUser = createUser("fake-user-2", "expected display name"); - var unexpectedUser1 = - createUser("fake-user-1", "first fake display name"); - var unexpectedUser2 = - createUser("fake-user-3", "second fake display name"); var expectResult = new ListResult<>(List.of(expectUser)); - when(client.list(same(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(same(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.just(expectResult)); when(roleService.list(anySet())).thenReturn(Flux.empty()); @@ -204,21 +167,6 @@ class UserEndpointTest { .get().uri("/users?sort=creationTimestamp,desc") .exchange() .expectStatus().isOk(); - - verify(client).list(same(User.class), any(), argThat(comparator -> { - var now = Instant.now(); - var users = new ArrayList<>(List.of( - createUser("fake-user-a", now), - createUser("fake-user-b", now.plusSeconds(1)), - createUser("fake-user-c", now.plusSeconds(2)) - )); - users.sort(comparator); - return Objects.deepEquals(users, List.of( - createUser("fake-user-c", now.plusSeconds(2)), - createUser("fake-user-b", now.plusSeconds(1)), - createUser("fake-user-a", now) - )); - }), anyInt(), anyInt()); } User createUser(String name) { diff --git a/application/src/test/java/run/halo/app/infra/InitializationStateGetterTest.java b/application/src/test/java/run/halo/app/infra/InitializationStateGetterTest.java index b5dc381cd..ab7badaca 100644 --- a/application/src/test/java/run/halo/app/infra/InitializationStateGetterTest.java +++ b/application/src/test/java/run/halo/app/infra/InitializationStateGetterTest.java @@ -1,7 +1,6 @@ package run.halo.app.infra; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -19,6 +18,7 @@ import run.halo.app.core.extension.User; import run.halo.app.extension.ConfigMap; import run.halo.app.extension.ListResult; import run.halo.app.extension.Metadata; +import run.halo.app.extension.PageRequest; import run.halo.app.extension.ReactiveExtensionClient; /** @@ -37,7 +37,7 @@ class InitializationStateGetterTest { @Test void userInitialized() { - when(client.list(eq(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(eq(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.empty()); initializationStateGetter.userInitialized() .as(StepVerifier::create) @@ -52,7 +52,7 @@ class InitializationStateGetterTest { user.getSpec().setDisplayName("fake-hidden-user"); ListResult listResult = new ListResult<>(List.of(user)); - when(client.list(eq(User.class), any(), any(), anyInt(), anyInt())) + when(client.listBy(eq(User.class), any(), any(PageRequest.class))) .thenReturn(Mono.just(listResult)); initializationStateGetter.userInitialized() .as(StepVerifier::create) diff --git a/ui/console-src/modules/system/users/UserList.vue b/ui/console-src/modules/system/users/UserList.vue index 8bcda7a79..83cbd922e 100644 --- a/ui/console-src/modules/system/users/UserList.vue +++ b/ui/console-src/modules/system/users/UserList.vue @@ -116,11 +116,11 @@ const { return data.items; }, refetchInterval(data) { - const deletingUsers = data?.filter( + const hasDeletingData = data?.some( (user) => !!user.user.metadata.deletionTimestamp ); - return deletingUsers?.length ? 1000 : false; + return hasDeletingData ? 1000 : false; }, onSuccess() { selectedUser.value = undefined; @@ -347,11 +347,11 @@ onMounted(() => { }, { label: t('core.user.filters.sort.items.create_time_desc'), - value: 'creationTimestamp,desc', + value: 'metadata.creationTimestamp,desc', }, { label: t('core.user.filters.sort.items.create_time_asc'), - value: 'creationTimestamp,asc', + value: 'metadata.creationTimestamp,asc', }, ]" />