Skip to content

Commit

Permalink
Small fix for web archive results beyond first page + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
latonv committed Jan 26, 2024
1 parent 490f27a commit 2b23159
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 7 deletions.
5 changes: 5 additions & 0 deletions src/data-source/collection-browser-data-source-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ export interface CollectionBrowserDataSourceInterface
*/
refreshLetterCounts(): void;

/**
* Returns the current page size of the data source.
*/
getPageSize(): number;

/**
* Changes the page size used by the data source, discarding any previously-fetched pages.
*
Expand Down
50 changes: 46 additions & 4 deletions src/data-source/collection-browser-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
SortField,
SORT_OPTIONS,
} from '../models';
import type { PageSpecifierParams } from './models';
import { FACETLESS_PAGE_ELEMENTS, type PageSpecifierParams } from './models';
import type { CollectionBrowserDataSourceInterface } from './collection-browser-data-source-interface';
import type { CollectionBrowserSearchInterface } from './collection-browser-query-state';
import { sha1 } from '../utils/sha1';
Expand All @@ -31,10 +31,21 @@ import { log } from '../utils/log';
export class CollectionBrowserDataSource
implements CollectionBrowserDataSourceInterface
{
/**
* All pages of tile models that have been fetched so far, indexed by their page
* number (with the first being page 1).
*/
private pages: Record<string, TileModel[]> = {};

/**
* Tile offset to apply when looking up tiles in the pages, in order to maintain
* page alignment after tiles are removed.
*/
private offset = 0;

/**
* Total number of tile models stored in this data source's pages
*/
private numTileModels = 0;

/**
Expand All @@ -48,10 +59,21 @@ export class CollectionBrowserDataSource
*/
private previousQueryKey: string = '';

/**
* Whether the initial page of search results for the current query state
* is presently being fetched.
*/
private searchResultsLoading = false;

/**
* Whether the facets (aggregations) for the current query state are
* presently being fetched.
*/
private facetsLoading = false;

/**
* Whether further query changes should be ignored and not trigger fetches
*/
private suppressFetches = false;

/**
Expand Down Expand Up @@ -265,6 +287,13 @@ export class CollectionBrowserDataSource
return Object.values(this.pages).flat().indexOf(tile);
}

/**
* @inheritdoc
*/
getPageSize(): number {
return this.pageSize;
}

/**
* @inheritdoc
*/
Expand Down Expand Up @@ -294,11 +323,15 @@ export class CollectionBrowserDataSource
this._initialSearchCompleteResolver = res;
});

const shouldFetchFacets =
!this.host.suppressFacets &&
!FACETLESS_PAGE_ELEMENTS.includes(this.host.profileElement!);

// Fire the initial page & facet requests
this.queryInitialized = true;
await Promise.all([
this.doInitialPageFetch(),
this.host.suppressFacets ? null : this.fetchFacets(),
shouldFetchFacets ? this.fetchFacets() : null,
]);

// Resolve the `initialSearchComplete` promise for this search
Expand Down Expand Up @@ -876,7 +909,7 @@ export class CollectionBrowserDataSource

// Batch multiple initial page requests together if needed (e.g., can request
// pages 1 and 2 together in a single request).
const numPages = pageNumber === 1 ? numInitialPages : 1;
let numPages = pageNumber === 1 ? numInitialPages : 1;
const numRows = this.pageSize * numPages;

// if a fetch is already in progress for this query and page, don't fetch again
Expand Down Expand Up @@ -990,7 +1023,16 @@ export class CollectionBrowserDataSource
}
}

// Update the data source for each returned page
// Update the data source for each returned page.
// For loans and web archives, we must account for receiving more pages than we asked for.
if (
this.host.profileElement === 'lending' ||
this.host.profileElement === 'web_archives'
) {
numPages = Math.ceil(results.length / this.pageSize);
this.endOfDataReached = true;
if (this.activeOnHost) this.host.setTileCount(this.totalResults);
}
for (let i = 0; i < numPages; i += 1) {
const pageStartIndex = this.pageSize * i;
this.addTilesToDataSource(
Expand Down
9 changes: 9 additions & 0 deletions src/data-source/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,12 @@ export type PageSpecifierParams = {
*/
pageElements?: PageElementName[];
};

/**
* List of profile page elements that do not currently allow faceting
*/
export const FACETLESS_PAGE_ELEMENTS: PageElementName[] = [
'forum_posts',
'lending',
'web_archives',
];
31 changes: 31 additions & 0 deletions test/collection-browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,37 @@ describe('Collection Browser', () => {
expect(el.maxSelectedDate).not.to.exist;
});

it('correctly retrieves web archive hits', async () => {
const searchService = new MockSearchService();
const el = await fixture<CollectionBrowser>(
html`<collection-browser
.searchService=${searchService}
.withinProfile=${'@foo'}
.profileElement=${'web_archives'}
>
</collection-browser>`
);

el.baseQuery = 'web-archive';
await el.updateComplete;
await el.initialSearchComplete;
await nextTick();

console.log(
'\n\n*****\n\n*****\n\n',
el.dataSource.getAllPages(),
'\n\n*****\n\n*****\n\n'
);
expect(el.dataSource.totalResults, 'total results').to.equal(1);
expect(el.dataSource.getTileModelAt(0)?.title).to.equal(
'https://example.com'
);
expect(
el.dataSource.getTileModelAt(0)?.captureDates?.length,
'capture dates'
).to.equal(1);
});

it('shows temporarily unavailable message when facets suppressed', async () => {
const searchService = new MockSearchService();
const el = await fixture<CollectionBrowser>(
Expand Down
23 changes: 20 additions & 3 deletions test/data-source/collection-browser-data-source.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { expect, fixture } from '@open-wc/testing';
import { html } from 'lit';
import sinon from 'sinon';
import { ItemHit, SearchType } from '@internetarchive/search-service';
import { CollectionBrowserDataSource } from '../../src/data-source/collection-browser-data-source';
import { TileModel } from '../../src/models';
import type { CollectionBrowser } from '../../src/collection-browser';
import '../../src/collection-browser';
import { MockSearchService } from '../mocks/mock-search-service';

const dataPage: TileModel[] = [
new TileModel(
Expand Down Expand Up @@ -32,7 +34,7 @@ describe('Collection Browser Data Source', () => {
`);
});

it('can add and retrieve data pages', async () => {
it('can add and retrieve data pages', () => {
const dataSource = new CollectionBrowserDataSource(host);
dataSource.addPage(1, dataPage);

Expand All @@ -42,12 +44,13 @@ describe('Collection Browser Data Source', () => {
expect(dataSource.getPage(1)[1].identifier).to.equal('bar');
});

it('resets data when changing page size', async () => {
it('resets data when changing page size', () => {
const dataSource = new CollectionBrowserDataSource(host);
dataSource.addPage(1, dataPage);

dataSource.setPageSize(100);
expect(Object.keys(dataSource.getAllPages()).length).to.equal(0);
expect(dataSource.getPageSize()).to.equal(100);
});

it('can be installed on the host', async () => {
Expand All @@ -70,7 +73,21 @@ describe('Collection Browser Data Source', () => {
host.removeController(dataSource);
});

it('refreshes prefix filter counts', async () => {
it('can suppress further fetches', async () => {
host.searchService = new MockSearchService();

const pageFetchSpy = sinon.spy();
const dataSource = new CollectionBrowserDataSource(host);
dataSource.fetchPage = pageFetchSpy;

dataSource.addPage(1, dataPage);
dataSource.setFetchesSuppressed(true);
dataSource.handleQueryChange();

expect(pageFetchSpy.callCount).to.equal(0);
});

it('refreshes prefix filter counts', () => {
const dataSource = new CollectionBrowserDataSource(host);
dataSource.addPage(1, dataPage);

Expand Down
44 changes: 44 additions & 0 deletions test/mocks/mock-search-responses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
SearchServiceError,
TextHit,
} from '@internetarchive/search-service';
import { WebArchiveHit } from '@internetarchive/search-service/dist/src/models/hit-types/web-archive-hit';
import { SearchServiceErrorType } from '@internetarchive/search-service/dist/src/search-service-error';

export const getMockSuccessSingleResult: () => Result<
Expand Down Expand Up @@ -876,6 +877,49 @@ export const getMockSuccessWithParentCollections: () => Result<
},
});

export const getMockSuccessWithWebArchiveHits: () => Result<
SearchResponse,
SearchServiceError
> = () => ({
success: {
request: {
kind: 'hits',
clientParameters: {
user_query: 'web-archive',
sort: [],
},
backendRequests: {
primary: {
kind: 'hits',
finalized_parameters: {
user_query: 'web-archive',
sort: [],
},
},
},
},
rawResponse: {},
response: {
totalResults: 1,
returnedCount: 1,
results: [
new WebArchiveHit({
fields: {
url: 'https://example.com',
capture_dates: ['2010-01-02T03:04:05Z'],
__href__:
'https://web.archive.org/web/20100102030405/https%3A%2F%2Fexample.com',
},
}),
],
},
responseHeader: {
succeeded: true,
query_time: 0,
},
},
});

export const getMockErrorResult: () => Result<
SearchResponse,
SearchServiceError
Expand Down
2 changes: 2 additions & 0 deletions test/mocks/mock-search-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
getMockSuccessWithParentCollections,
getMockSuccessManyFields,
getMockSuccessNoResults,
getMockSuccessWithWebArchiveHits,
} from './mock-search-responses';

const responses: Record<
Expand All @@ -49,6 +50,7 @@ const responses: Record<
'default-sort-concise': getMockSuccessWithConciseDefaultSort,
'fav-sort': getMockSuccessWithDefaultFavSort,
'parent-collections': getMockSuccessWithParentCollections,
'web-archive': getMockSuccessWithWebArchiveHits,
'many-fields': getMockSuccessManyFields,
'no-results': getMockSuccessNoResults,
error: getMockErrorResult,
Expand Down

0 comments on commit 2b23159

Please sign in to comment.