Add pagination for long queries in CustomControllerClients#1809
Add pagination for long queries in CustomControllerClients#1809theghost5800 wants to merge 2 commits intomasterfrom
Conversation
JIRA:LMCROSSITXSADEPLOY-3432
theghost5800
force-pushed
the
addPagination
branch
from
eaa1300 to
0437da1
Compare
| String uri = uriPrefix + batchParamPrefix + String.join(",", batch); | ||
| return getListOfResources(responseMapper, uri); |
There was a problem hiding this comment.
Can you extract this to a method so the map is cleaner
| .toList(); | ||
| } | ||
|
|
||
| List<List<String>> splitIntoBatches(List<String> values, int fixedUriLength) { |
There was a problem hiding this comment.
Can you add access modifier or is it intentionally without one?
| Mockito.when(webClient.get()) | ||
| .thenReturn(requestHeadersUriSpec); | ||
| Mockito.when(requestHeadersUriSpec.uri(Mockito.anyString())) | ||
| .thenReturn(requestHeadersSpec); | ||
| Mockito.when(requestHeadersSpec.headers(Mockito.any(Consumer.class))) | ||
| .thenReturn(requestHeadersSpec); | ||
| Mockito.when(requestHeadersSpec.retrieve()) | ||
| .thenReturn(responseSpec); |
There was a problem hiding this comment.
This code is repeated in the methods below. Can you extract is to avoid duplication? The other test classes have similiar duplicatio. Can you do the same?
| } | ||
|
|
||
| private PaginationV3 addPageOfResources(String uri, ResourcesResponseMapper<?> responseMapper, Object... urlVariables) { | ||
| protected <T> List<T> getListOfResourcesInBatches(ResourcesResponseMapper<T> responseMapper, String uriPrefix, String batchParamPrefix, |
There was a problem hiding this comment.
can you decrease the number of params of this method? maybe merge uriPrefix and batchParamPrefix?
There was a problem hiding this comment.
I have merged them but if we want to keep old behaviour by passing urlVariables to be resolved inside webClient, then count of parameters will be still the same.
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| class AppBoundServiceInstanceNamesGetterTest { |
There was a problem hiding this comment.
why is this class called this way? I do not understand what is is actually testing from the name
There was a problem hiding this comment.
Name of the class is the same as original class by adding test suffix in the end. Test method names explains what exactly is testing.
| return buildResponseWithIncludedServiceInstancesAndPagination(serviceNames, null); | ||
| } | ||
|
|
||
| private String buildResponseWithIncludedServiceInstancesAndPagination(List<String> serviceNames, String nextPageHref) { |
There was a problem hiding this comment.
this method seems hard to read. can you extrac in some json and replace files or split to smaller readable methods?
| return getListOfResources(new ServiceKeysResponseMapper(), | ||
| SERVICE_KEYS_BY_METADATA_SELECTOR_URI + uriSuffix, | ||
| labelSelector); | ||
| String expandedUriPrefix = SERVICE_KEYS_BY_METADATA_SELECTOR_URI.replace("{value}", labelSelector) |
There was a problem hiding this comment.
why is it needed to manually replace this value? is it properly url encoded?
| String nextPage = nextPageHref == null ? "null" : "{\"href\":\"" + nextPageHref + "\"}"; | ||
| return "{" + "\"resources\":[" + " {" + " \"guid\":\"" + serviceKeyGuid + "\"," + " \"name\":\"" + keyName + "\"," | ||
| + " \"type\":\"key\"," + " \"created_at\":\"2024-01-01T00:00:00Z\"," + " \"updated_at\":\"2024-01-01T00:00:00Z\"," | ||
| + " \"metadata\":{\"labels\":{},\"annotations\":{}}," + " \"relationships\":{" | ||
| + " \"service_instance\":{\"data\":{\"guid\":\"" + serviceInstanceGuid + "\"}}" + " }" + " }" + "]," + "\"included\":{" | ||
| + " \"service_instances\":[" + " {" + " \"guid\":\"" + serviceInstanceGuid + "\"," + " \"name\":\"" + serviceName | ||
| + "\"," + " \"type\":\"managed\"," + " \"created_at\":\"2024-01-01T00:00:00Z\"," | ||
| + " \"updated_at\":\"2024-01-01T00:00:00Z\"," + " \"metadata\":{\"labels\":{},\"annotations\":{}}" + " }" + " ]" |
There was a problem hiding this comment.
extract this in a file
| return "{" + "\"resources\":[" + resources + "]," + "\"included\":{" + " \"service_instances\":[" + " {" + " \"guid\":\"" | ||
| + serviceInstanceGuid + "\"," + " \"name\":\"" + serviceName + "\"," + " \"type\":\"managed\"," | ||
| + " \"created_at\":\"2024-01-01T00:00:00Z\"," + " \"updated_at\":\"2024-01-01T00:00:00Z\"," | ||
| + " \"metadata\":{\"labels\":{},\"annotations\":{}}" + " }" + " ]" + "}," + "\"pagination\":{\"next\":null}" + "}"; |
There was a problem hiding this comment.
same, extract in file
| return "{" | ||
| + "\"resources\":[" | ||
| + " {" | ||
| + " \"guid\":\"" + UUID.randomUUID() + "\"," | ||
| + " \"created_at\":\"2024-01-01T00:00:00Z\"," | ||
| + " \"updated_at\":\"2024-01-01T00:00:00Z\"," | ||
| + " \"relationships\":{" | ||
| + " \"route\":{\"data\":{\"guid\":\"" + routeGuid + "\"}}," | ||
| + " \"service_instance\":{\"data\":{\"guid\":\"" + serviceInstanceGuid + "\"}}" | ||
| + " }" | ||
| + " }" | ||
| + "]," | ||
| + "\"pagination\":{\"next\":" + nextPage + "}" | ||
| + "}"; |
There was a problem hiding this comment.
same, extract in file?
|
3 participants
JIRA:LMCROSSITXSADEPLOY-3432