Commit 6d36cc7c authored by Jean-Baptiste Nizet's avatar Jean-Baptiste Nizet
Browse files

feat: display a resume of the page on top, limit results to 10000 to avoid exception

parent e2de48e2
......@@ -12,6 +12,11 @@ import org.springframework.data.domain.Page;
* @param <T> the type of data in this page
*/
public final class PageDTO<T> {
/**
* The maximum number of results that Elasticsearch accepts to paginate.
*/
public static final int MAX_RESULTS = 10_000;
/**
* The elements in this page. Its size is less than or equal to {@link #size}
*/
......@@ -50,7 +55,7 @@ public final class PageDTO<T> {
page.getNumber(),
page.getSize(),
page.getTotalElements(),
page.getTotalPages());
Math.min(page.getTotalPages(), MAX_RESULTS / page.getSize()));
}
public static <T, R> PageDTO<R> fromPage(Page<T> page, Function<T, R> mapper) {
......@@ -58,7 +63,7 @@ public final class PageDTO<T> {
page.getNumber(),
page.getSize(),
page.getTotalElements(),
page.getTotalPages());
Math.min(page.getTotalPages(), MAX_RESULTS / page.getSize()));
}
public List<T> getContent() {
......@@ -80,4 +85,8 @@ public final class PageDTO<T> {
public int getTotalPages() {
return totalPages;
}
public int getMaxResults() {
return MAX_RESULTS;
}
}
package fr.inra.urgi.rare.exception;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.ResponseStatus;
/**
* Exception thrown to signal a bad request (status 400)
* @author JB Nizet
*/
@ResponseStatus(HttpStatus.BAD_REQUEST)
public class BadRequestException extends RuntimeException {
public BadRequestException(String message) {
super(message);
}
}
......@@ -8,6 +8,8 @@ import fr.inra.urgi.rare.dao.RareAggregation;
import fr.inra.urgi.rare.dao.SearchRefinements;
import fr.inra.urgi.rare.domain.GeneticResource;
import fr.inra.urgi.rare.dto.AggregatedPageDTO;
import fr.inra.urgi.rare.dto.PageDTO;
import fr.inra.urgi.rare.exception.BadRequestException;
import org.springframework.data.domain.PageRequest;
import org.springframework.util.MultiValueMap;
import org.springframework.web.bind.annotation.GetMapping;
......@@ -24,6 +26,11 @@ public class SearchController {
public static final int PAGE_SIZE = 20;
/**
* The maximum number of results that Elasticsearch accepts to paginate.
*/
public static final int MAX_RESULTS = PageDTO.MAX_RESULTS;
private GeneticResourceDao geneticResourceDao;
public SearchController(GeneticResourceDao geneticResourceDao) {
......@@ -49,6 +56,8 @@ public class SearchController {
@RequestParam("page") Optional<Integer> page,
@RequestParam MultiValueMap<String, String> parameters) {
boolean aggregate = agg.orElse(false);
int requestedPage = page.orElse(0);
validatePage(requestedPage);
return AggregatedPageDTO.fromPage(geneticResourceDao.search(query,
aggregate,
createRefinementsFromParameters(parameters),
......@@ -67,4 +76,11 @@ public class SearchController {
return builder.build();
}
private void validatePage(int requestedPage) {
int maxPage = MAX_RESULTS / PAGE_SIZE;
if (requestedPage >= maxPage) {
throw new BadRequestException("The requested page is too high. It must be less than " + maxPage);
}
}
}
......@@ -56,6 +56,7 @@ class SearchControllerTest {
mockMvc.perform(get("/api/genetic-resources").param("query", query))
.andExpect(status().isOk())
.andExpect(jsonPath("$.number").value(0))
.andExpect(jsonPath("$.maxResults").value(SearchController.MAX_RESULTS))
.andExpect(jsonPath("$.content[0].identifier").value(resource.getId()))
.andExpect(jsonPath("$.content[0].name").value(resource.getName()))
.andExpect(jsonPath("$.content[0].description").value(resource.getDescription()))
......@@ -127,4 +128,15 @@ class SearchControllerTest {
.param(RareAggregation.MATERIAL.getName(), "m1"))
.andExpect(status().isOk());
}
@Test
void shouldThrowIfPageTooLarge() throws Exception {
int page = SearchController.MAX_RESULTS / SearchController.PAGE_SIZE;
String query = "pauca";
mockMvc.perform(get("/api/genetic-resources")
.param("query", query)
.param("page", Integer.toString(page)))
.andExpect(status().isBadRequest());
}
}
import { BrowserModule } from '@angular/platform-browser';
import { NgModule } from '@angular/core';
import { LOCALE_ID, NgModule } from '@angular/core';
import { RouterModule } from '@angular/router';
import { HttpClientModule } from '@angular/common/http';
import { ReactiveFormsModule } from '@angular/forms';
......@@ -11,6 +11,10 @@ import { SearchComponent } from './search/search.component';
import { GeneticResourcesComponent } from './genetic-resources/genetic-resources.component';
import { GeneticResourceComponent } from './genetic-resource/genetic-resource.component';
import { NgbPaginationModule } from '@ng-bootstrap/ng-bootstrap';
import { registerLocaleData } from '@angular/common';
import localeFr from '@angular/common/locales/fr';
registerLocaleData(localeFr);
@NgModule({
declarations: [
......@@ -27,7 +31,9 @@ import { NgbPaginationModule } from '@ng-bootstrap/ng-bootstrap';
HttpClientModule,
NgbPaginationModule.forRoot()
],
providers: [],
providers: [
{ provide: LOCALE_ID, useValue: 'fr-FR' }
],
bootstrap: [AppComponent]
})
export class AppModule { }
<div class="mt-5">
<!-- if there are results to display -->
<div *ngIf="geneticResources?.content?.length; else noResults">
<div class="mt-4" *ngFor="let geneticResource of geneticResources.content">
<rare-genetic-resource [geneticResource]="geneticResource"></rare-genetic-resource>
</div>
<!-- if there are results to display -->
<div *ngIf="geneticResources.content.length; else noResults">
<div id="resume" class="text-muted small mb-3">
Résultats {{ firstResultIndex | number }} à {{ lastResultIndex | number }}
sur {{ geneticResources.totalElements | number }}
<ng-container *ngIf="resultLimited">(limités à {{ geneticResources.maxResults | number }})</ng-container>
</div>
<div class="mb-4" *ngFor="let geneticResource of geneticResources.content">
<rare-genetic-resource [geneticResource]="geneticResource"></rare-genetic-resource>
</div>
<!-- else we display a simple message -->
<ng-template #noResults>
<div id="no-results">Pas de résultat.</div>
</ng-template>
</div>
<!-- else we display a simple message -->
<ng-template #noResults>
<div id="no-results">Pas de résultat.</div>
</ng-template>
import { TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { ReactiveFormsModule } from '@angular/forms';
import { ComponentTester } from 'ngx-speculoos';
import { ComponentTester, speculoosMatchers } from 'ngx-speculoos';
import { GeneticResourcesComponent } from './genetic-resources.component';
import { GeneticResourceComponent } from '../genetic-resource/genetic-resource.component';
import { toGeneticResource, toSinglePage } from '../models/test-model-generators';
import { LOCALE_ID } from '@angular/core';
import { registerLocaleData } from '@angular/common';
import localeFr from '@angular/common/locales/fr';
import { GeneticResourceModel } from '../models/genetic-resource.model';
describe('GeneticResourcesComponent', () => {
......@@ -19,24 +23,25 @@ describe('GeneticResourcesComponent', () => {
}
get noResults() {
return this.nativeElement.querySelector('#no-results');
return this.element('#no-results');
}
}
beforeEach(() => TestBed.configureTestingModule({
imports: [ReactiveFormsModule],
declarations: [GeneticResourcesComponent, GeneticResourceComponent]
}));
it('should display no results if null', () => {
const tester = new GeneticResourcesComponentTester();
get resume() {
return this.element('#resume');
}
}
// given no results
tester.detectChanges();
beforeEach(() => {
registerLocaleData(localeFr);
TestBed.configureTestingModule({
imports: [ReactiveFormsModule],
declarations: [GeneticResourcesComponent, GeneticResourceComponent],
providers: [
{ provide: LOCALE_ID, useValue: 'fr-FR' }
]
});
// then it should display a message
expect(tester.results.length).toBe(0);
expect(tester.noResults).not.toBeNull();
jasmine.addMatchers(speculoosMatchers);
});
it('should display no results if empty', () => {
......@@ -49,6 +54,7 @@ describe('GeneticResourcesComponent', () => {
// then it should display a message
expect(tester.results.length).toBe(0);
expect(tester.resume).toBeNull();
expect(tester.noResults).not.toBeNull();
});
......@@ -56,7 +62,7 @@ describe('GeneticResourcesComponent', () => {
const tester = new GeneticResourcesComponentTester();
const component = tester.componentInstance;
// given no results
// given two results
const bacteria1 = toGeneticResource('Bacteria1');
const bacteria2 = toGeneticResource('Bacteria2');
component.geneticResources = toSinglePage([bacteria1, bacteria2]);
......@@ -65,9 +71,37 @@ describe('GeneticResourcesComponent', () => {
// then it should display each result
expect(tester.noResults).toBeNull();
expect(tester.results.length).toBe(2);
const result1 = tester.results[0].componentInstance as GeneticResourceComponent;
expect(result1.geneticResource).toBe(bacteria1);
const result2 = tester.results[1].componentInstance as GeneticResourceComponent;
expect(result2.geneticResource).toBe(bacteria2);
expect(tester.resume).toContainText('Résultats 1 à 2 sur 2');
expect(tester.resume).not.toContainText('limités');
});
it('should display limited results in resume, and format numbers in French', () => {
const tester = new GeneticResourcesComponentTester();
const component = tester.componentInstance;
// given results
const content: Array<GeneticResourceModel> = [];
for (let i = 0; i < 20; i++) {
content.push(toGeneticResource(`Bacteria ${i}`));
}
// in page 200 on a limited number of pages
component.geneticResources = toSinglePage(content);
component.geneticResources.totalElements = 12000;
component.geneticResources.totalPages = 500;
component.geneticResources.number = 200;
tester.detectChanges();
// then it should display each result
expect(tester.noResults).toBeNull();
expect(tester.results.length).toBe(20);
expect(tester.resume).toContainText('Résultats 4\u00a0001 à 4\u00a0020 sur 12\u00a0000 (limités à 10\u00a0000)');
});
});
......@@ -13,4 +13,15 @@ export class GeneticResourcesComponent {
@Input() geneticResources: Page<GeneticResourceModel>;
get firstResultIndex() {
return (this.geneticResources.number * this.geneticResources.size) + 1
}
get lastResultIndex() {
return this.firstResultIndex + this.geneticResources.content.length - 1;
}
get resultLimited() {
return this.geneticResources.totalElements > this.geneticResources.maxResults;
}
}
......@@ -4,4 +4,5 @@ export interface Page<T> {
size: number;
totalElements: number;
totalPages: number;
maxResults: number;
}
......@@ -7,7 +7,8 @@ export function toSinglePage<T>(content: Array<T>): Page<T> {
number: 0,
size: 20,
totalElements: content.length,
totalPages: 1
totalPages: 1,
maxResults: 10000
};
}
......@@ -17,7 +18,8 @@ export function toSecondPage<T>(content: Array<T>): Page<T> {
number: 1,
size: 20,
totalElements: 20 + content.length,
totalPages: 2
totalPages: 2,
maxResults: 10000
};
}
......
......@@ -9,14 +9,14 @@
</form>
</div>
<!-- results -->
<div class="mt-5" *ngIf="results">
<div class="mt-3" *ngIf="results">
<rare-genetic-resources [geneticResources]="results"></rare-genetic-resources>
</div>
<!-- pagination -->
<div class="d-flex justify-content-center mt-5" *ngIf="results && results.totalPages > 1">
<!-- we add 1 to the page because ngb-pagination is 1 based -->
<ngb-pagination *ngIf="results" [page]="results.number + 1" (pageChange)="navigateToPage($event)"
[collectionSize]="results.totalElements"
[collectionSize]="collectionSize()"
[pageSize]="results.size"
[maxSize]="5"
[boundaryLinks]="true"
......
......@@ -13,6 +13,7 @@ import { GeneticResourcesComponent } from '../genetic-resources/genetic-resource
import { GeneticResourceComponent } from '../genetic-resource/genetic-resource.component';
import { SearchService } from '../search.service';
import { toGeneticResource, toSecondPage, toSinglePage } from '../models/test-model-generators';
import { GeneticResourceModel } from '../models/genetic-resource.model';
class SearchComponentTester extends ComponentTester<SearchComponent> {
constructor() {
......@@ -181,6 +182,32 @@ describe('SearchComponent', () => {
expect(paginationComponent.pageCount).toBe(2);
});
it('should limit pagination to 500 pages, even if more results', () => {
// given a component
const tester = new SearchComponentTester();
const component = tester.componentInstance;
// when it has results
const content: Array<GeneticResourceModel> = [];
for (let i = 0; i < 20; i++) {
content.push(toGeneticResource(`Bacteria ${i}`));
}
// in page 200 on a limited number of pages
component.results = toSinglePage(content);
component.results.totalElements = 12000;
component.results.totalPages = 500;
component.results.number = 200;
tester.detectChanges();
// then it should limit the pagination to 500 pages in the pagination
// and a pagination with one page
expect(tester.pagination).not.toBeNull();
const paginationComponent = tester.pagination.componentInstance as NgbPagination;
expect(paginationComponent.page).toBe(201);
expect(paginationComponent.pageCount).toBe(500);
});
it('should hide results and pagination on a new search', () => {
// given a component
const tester = new SearchComponentTester();
......
......@@ -62,6 +62,10 @@ export class SearchComponent implements OnInit {
this.search(requestedPage);
}
collectionSize() {
return Math.min(this.results.totalElements, this.results.maxResults);
}
private search(page?: number) {
this.router.navigate(['.'], {
relativeTo: this.route,
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment