mirror of https://github.com/halo-dev/halo
Refactor the response structure of Extension list API (#2244)
#### What type of PR is this? /kind improvement /kind api-change /area core /milestone 2.0 #### What this PR does / why we need it: This PR is refactoring the response structure of Extension list API as follows: ```json { "page": 0, "size": 0, "total": 1, "items": [ { "spec": { "displayName": "Administrator", "email": "admin@halo.run", "password": "{bcrypt}$2a$10$/YveWyuf9vyYrHE3fiToI.bGBy5Hgs1eViRvKzU7Kl982la5NSwWO", "registeredAt": "2022-06-17T09:35:47.237625514Z", "twoFactorAuthEnabled": false, "disabled": false }, "apiVersion": "v1alpha1", "kind": "User", "metadata": { "name": "admin", "annotations": { "user.halo.run/roles": "[\"super-role\"]", "rbac.authorization.halo.run/role-names": "[\"second-super-role\",\"super-role\"]" }, "version": 3077, "creationTimestamp": "2022-06-17T09:35:47.367919552Z" } } ], "first": true, "last": true, "hasNext": false, "hasPrevious": false } ``` Instead of items only. #### Which issue(s) this PR fixes: Fixes # #### Special notes for your reviewer: Steps to test: 1. Start Halo server 2. Request <http://localhost:8090/swagger-ui.html> from browser and you might be redirected to login page 3. Login with your username and password 4. Try to request the list endpoints and see the result. #### Does this PR introduce a user-facing change? ```release-note None ```pull/2247/head
parent
a8db2e5e4b
commit
1cbd3c74e3
|
@ -5,9 +5,6 @@ import java.util.Comparator;
|
|||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.function.Predicate;
|
||||
import org.springframework.data.domain.Page;
|
||||
import org.springframework.data.domain.PageRequest;
|
||||
import org.springframework.data.support.PageableExecutionUtils;
|
||||
import org.springframework.util.Assert;
|
||||
import run.halo.app.extension.store.ExtensionStoreClient;
|
||||
|
||||
|
@ -53,14 +50,19 @@ public class DefaultExtensionClient implements ExtensionClient {
|
|||
}
|
||||
|
||||
@Override
|
||||
public <E extends Extension> Page<E> page(Class<E> type, Predicate<E> predicate,
|
||||
public <E extends Extension> ListResult<E> list(Class<E> type, Predicate<E> predicate,
|
||||
Comparator<E> comparators, int page, int size) {
|
||||
var pageable = PageRequest.of(page, size);
|
||||
var all = list(type, predicate, comparators);
|
||||
var total = all.size();
|
||||
var content =
|
||||
all.stream().limit(pageable.getPageSize()).skip(pageable.getOffset()).toList();
|
||||
return PageableExecutionUtils.getPage(content, pageable, () -> total);
|
||||
var extensions = list(type, predicate, comparators);
|
||||
var extensionStream = extensions.stream();
|
||||
if (page > 0) {
|
||||
extensionStream = extensionStream.skip(((long) (page - 1)) * (long) size);
|
||||
}
|
||||
if (size > 0) {
|
||||
extensionStream = extensionStream.limit(size);
|
||||
}
|
||||
var content = extensionStream.toList();
|
||||
|
||||
return new ListResult<>(page, size, extensions.size(), content);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -4,7 +4,6 @@ import java.util.Comparator;
|
|||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.function.Predicate;
|
||||
import org.springframework.data.domain.Page;
|
||||
|
||||
/**
|
||||
* ExtensionClient is an interface which contains some operations on Extension instead of
|
||||
|
@ -35,9 +34,9 @@ public interface ExtensionClient {
|
|||
* @param page is page number which starts from 0.
|
||||
* @param size is page size.
|
||||
* @param <E> is Extension type.
|
||||
* @return a page of Extensions.
|
||||
* @return a list of Extensions.
|
||||
*/
|
||||
<E extends Extension> Page<E> page(Class<E> type, Predicate<E> predicate,
|
||||
<E extends Extension> ListResult<E> list(Class<E> type, Predicate<E> predicate,
|
||||
Comparator<E> comparator, int page, int size);
|
||||
|
||||
/**
|
||||
|
|
|
@ -8,6 +8,8 @@ import io.swagger.v3.oas.annotations.enums.ParameterIn;
|
|||
import java.net.URI;
|
||||
import java.time.Instant;
|
||||
import java.util.Objects;
|
||||
import net.bytebuddy.ByteBuddy;
|
||||
import net.bytebuddy.description.type.TypeDescription;
|
||||
import org.springdoc.webflux.core.fn.SpringdocRouteBuilder;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.lang.NonNull;
|
||||
|
@ -67,10 +69,9 @@ public class ExtensionRouterFunctionFactory {
|
|||
.parameter(parameterBuilder().in(ParameterIn.QUERY)
|
||||
.name("sort")
|
||||
.description("Sort by some fields. Like metadata.name,desc"))
|
||||
|
||||
.response(responseBuilder().responseCode("200")
|
||||
.description("Response " + scheme.plural())
|
||||
.implementationArray(scheme.type())))
|
||||
.implementation(generateListResultClass())))
|
||||
.POST(createHandler.pathPattern(), createHandler,
|
||||
builder -> builder.operationId("Create" + gvk)
|
||||
.description("Create " + gvk)
|
||||
|
@ -194,12 +195,18 @@ public class ExtensionRouterFunctionFactory {
|
|||
@Override
|
||||
@NonNull
|
||||
public Mono<ServerResponse> handle(@NonNull ServerRequest request) {
|
||||
int page = request.queryParam("page")
|
||||
.filter(StringUtils::hasLength)
|
||||
.map(Integer::parseUnsignedInt).orElse(0);
|
||||
int size = request.queryParam("size")
|
||||
.filter(StringUtils::hasLength)
|
||||
.map(Integer::parseUnsignedInt).orElse(0);
|
||||
// TODO Resolve predicate and comparator from request
|
||||
var extensions = client.list(scheme.type(), null, null);
|
||||
var listResult = client.list(scheme.type(), null, null, page, size);
|
||||
return ServerResponse
|
||||
.ok()
|
||||
.contentType(MediaType.APPLICATION_JSON)
|
||||
.bodyValue(extensions);
|
||||
.bodyValue(listResult);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -315,4 +322,16 @@ public class ExtensionRouterFunctionFactory {
|
|||
}
|
||||
|
||||
}
|
||||
|
||||
private Class<?> generateListResultClass() {
|
||||
var generic =
|
||||
TypeDescription.Generic.Builder.parameterizedType(ListResult.class, scheme.type())
|
||||
.build();
|
||||
return new ByteBuddy()
|
||||
.subclass(generic)
|
||||
.name(scheme.groupVersionKind().kind() + "List")
|
||||
.make()
|
||||
.load(this.getClass().getClassLoader())
|
||||
.getLoaded();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,87 @@
|
|||
package run.halo.app.extension;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||
import com.fasterxml.jackson.annotation.JsonProperty;
|
||||
import io.swagger.v3.oas.annotations.media.Schema;
|
||||
import java.util.Collections;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import lombok.Data;
|
||||
import org.springframework.data.util.Streamable;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
@Data
|
||||
public class ListResult<T> implements Streamable<T> {
|
||||
|
||||
@Schema(description = "Page number, starts from 1. If not set or equal to 0, it means no "
|
||||
+ "pagination.", required = true)
|
||||
private final int page;
|
||||
|
||||
@Schema(description = "Size of each page. If not set or equal to 0, it means no pagination.",
|
||||
required = true)
|
||||
private final int size;
|
||||
|
||||
@Schema(description = "Total elements.", required = true)
|
||||
private final long total;
|
||||
|
||||
@Schema(description = "A chunk of items.", required = true)
|
||||
private final List<T> items;
|
||||
|
||||
public ListResult(int page, int size, long total, List<T> items) {
|
||||
Assert.isTrue(total >= 0, "Total elements must be greater than or equal to 0");
|
||||
if (page < 0) {
|
||||
page = 0;
|
||||
}
|
||||
if (size < 0) {
|
||||
size = 0;
|
||||
}
|
||||
if (items == null) {
|
||||
items = Collections.emptyList();
|
||||
}
|
||||
this.page = page;
|
||||
this.size = size;
|
||||
this.total = total;
|
||||
this.items = items;
|
||||
}
|
||||
|
||||
public ListResult(List<T> items) {
|
||||
this(0, 0, items.size(), items);
|
||||
}
|
||||
|
||||
@Schema(description = "Indicates whether current page is the first page.", required = true)
|
||||
public boolean isFirst() {
|
||||
return !hasPrevious();
|
||||
}
|
||||
|
||||
@Schema(description = "Indicates whether current page is the last page.", required = true)
|
||||
public boolean isLast() {
|
||||
return !hasNext();
|
||||
}
|
||||
|
||||
@Schema(description = "Indicates whether current page has previous page.", required = true)
|
||||
@JsonProperty("hasNext")
|
||||
public boolean hasNext() {
|
||||
if (page <= 0) {
|
||||
return false;
|
||||
}
|
||||
var totalPages = size == 0 ? 1 : (int) Math.ceil((double) total / (double) size);
|
||||
return page < totalPages;
|
||||
}
|
||||
|
||||
@Schema(description = "Indicates whether current page has previous page.", required = true)
|
||||
@JsonProperty("hasPrevious")
|
||||
public boolean hasPrevious() {
|
||||
return page > 1;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Iterator<T> iterator() {
|
||||
return items.iterator();
|
||||
}
|
||||
|
||||
@Override
|
||||
@JsonIgnore
|
||||
public boolean isEmpty() {
|
||||
return Streamable.super.isEmpty();
|
||||
}
|
||||
}
|
|
@ -29,8 +29,6 @@ import org.junit.jupiter.api.extension.ExtendWith;
|
|||
import org.mockito.InjectMocks;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
import org.springframework.data.domain.PageImpl;
|
||||
import org.springframework.data.domain.PageRequest;
|
||||
import run.halo.app.extension.exception.SchemeNotFoundException;
|
||||
import run.halo.app.extension.store.ExtensionStore;
|
||||
import run.halo.app.extension.store.ExtensionStoreClient;
|
||||
|
@ -114,7 +112,7 @@ class DefaultExtensionClientTest {
|
|||
assertThrows(SchemeNotFoundException.class,
|
||||
() -> client.list(UnRegisteredExtension.class, null, null));
|
||||
assertThrows(SchemeNotFoundException.class,
|
||||
() -> client.page(UnRegisteredExtension.class, null, null, 0, 10));
|
||||
() -> client.list(UnRegisteredExtension.class, null, null, 0, 10));
|
||||
assertThrows(SchemeNotFoundException.class,
|
||||
() -> client.fetch(UnRegisteredExtension.class, "fake"));
|
||||
assertThrows(SchemeNotFoundException.class, () ->
|
||||
|
@ -192,23 +190,27 @@ class DefaultExtensionClientTest {
|
|||
createExtensionStore("fake-03")));
|
||||
|
||||
// without filter and sorter.
|
||||
var fakes = client.page(FakeExtension.class, null, null, 0, 10);
|
||||
assertEquals(new PageImpl<>(List.of(fake1, fake2, fake3), PageRequest.of(0, 10), 3), fakes);
|
||||
var fakes = client.list(FakeExtension.class, null, null, 1, 10);
|
||||
assertEquals(new ListResult<>(1, 10, 3, List.of(fake1, fake2, fake3)), fakes);
|
||||
|
||||
// out of page range
|
||||
fakes = client.page(FakeExtension.class, null, null, 100, 10);
|
||||
assertEquals(new PageImpl<>(emptyList(), PageRequest.of(100, 10), 3), fakes);
|
||||
fakes = client.list(FakeExtension.class, null, null, 100, 10);
|
||||
assertEquals(new ListResult<>(100, 10, 3, emptyList()), fakes);
|
||||
|
||||
// with filter only
|
||||
fakes =
|
||||
client.page(FakeExtension.class, fake -> "fake-03".equals(fake.getMetadata().getName()),
|
||||
null, 0, 10);
|
||||
assertEquals(new PageImpl<>(List.of(fake3), PageRequest.of(0, 10), 1), fakes);
|
||||
client.list(FakeExtension.class, fake -> "fake-03".equals(fake.getMetadata().getName()),
|
||||
null, 1, 10);
|
||||
assertEquals(new ListResult<>(1, 10, 1, List.of(fake3)), fakes);
|
||||
|
||||
// with sorter only
|
||||
fakes = client.page(FakeExtension.class, null,
|
||||
reverseOrder(comparing(fake -> fake.getMetadata().getName())), 0, 10);
|
||||
assertEquals(new PageImpl<>(List.of(fake3, fake2, fake1), PageRequest.of(0, 10), 3), fakes);
|
||||
fakes = client.list(FakeExtension.class, null,
|
||||
reverseOrder(comparing(fake -> fake.getMetadata().getName())), 1, 10);
|
||||
assertEquals(new ListResult<>(1, 10, 3, List.of(fake3, fake2, fake1)), fakes);
|
||||
|
||||
// without page
|
||||
fakes = client.list(FakeExtension.class, null, null, 0, 0);
|
||||
assertEquals(new ListResult<>(0, 0, 3, List.of(fake1, fake2, fake3)), fakes);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -3,7 +3,8 @@ package run.halo.app.extension;
|
|||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.ArgumentMatchers.anyInt;
|
||||
import static org.mockito.ArgumentMatchers.same;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.List;
|
||||
|
@ -38,7 +39,10 @@ class ExtensionListHandlerTest {
|
|||
var listHandler = new ExtensionListHandler(scheme, client);
|
||||
var serverRequest = MockServerRequest.builder().build();
|
||||
final var fake = new FakeExtension();
|
||||
when(client.list(eq(FakeExtension.class), any(), any())).thenReturn(List.of(fake));
|
||||
// when(client.list(same(FakeExtension.class), any(), any())).thenReturn(List.of(fake));
|
||||
var fakeListResult = new ListResult<>(0, 0, 1, List.of(fake));
|
||||
when(client.list(same(FakeExtension.class), any(), any(), anyInt(), anyInt()))
|
||||
.thenReturn(fakeListResult);
|
||||
|
||||
var responseMono = listHandler.handle(serverRequest);
|
||||
|
||||
|
@ -47,7 +51,7 @@ class ExtensionListHandlerTest {
|
|||
assertEquals(HttpStatus.OK, response.statusCode());
|
||||
assertEquals(MediaType.APPLICATION_JSON, response.headers().getContentType());
|
||||
assertTrue(response instanceof EntityResponse<?>);
|
||||
assertEquals(List.of(fake), ((EntityResponse<?>) response).entity());
|
||||
assertEquals(fakeListResult, ((EntityResponse<?>) response).entity());
|
||||
})
|
||||
.verifyComplete();
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue