Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,48 @@ import { createPaginatedList } from '../../../testing/utils.test';
import { Collection } from '../../../../core/shared/collection.model';
import { DSpaceObjectType } from '../../../../core/shared/dspace-object-type.model';
import { NotificationsService } from '../../../notifications/notifications.service';
import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';

describe('AuthorizedCollectionSelectorComponent', () => {
let component: AuthorizedCollectionSelectorComponent;
let fixture: ComponentFixture<AuthorizedCollectionSelectorComponent>;

let collectionService;
let collection;

let dsoNameService: jasmine.SpyObj<DSONameService>;
let notificationsService: NotificationsService;

function createCollection(id: string, name: string): Collection {
return Object.assign(new Collection(), { id, name });
}

const collectionTest = createCollection('col-test', 'test');
const collectionTestSuite = createCollection('col-suite', 'test suite');
const collectionCollection = createCollection('col-collection', 'collection');

beforeEach(waitForAsync(() => {
collection = Object.assign(new Collection(), {
id: 'authorized-collection'
});
collectionService = jasmine.createSpyObj('collectionService', {
getAuthorizedCollection: createSuccessfulRemoteDataObject$(createPaginatedList([collection])),
getAuthorizedCollectionByEntityType: createSuccessfulRemoteDataObject$(createPaginatedList([collection]))
});
dsoNameService = jasmine.createSpyObj('dsoNameService', ['getName']);
dsoNameService.getName.and.callFake((dso: any) => dso?.name ?? '');

notificationsService = jasmine.createSpyObj('notificationsService', ['error']);

// Use callFake so createSuccessfulRemoteDataObject$ is called lazily at spy invocation time
// (not at setup time), avoiding issues with environment not being available during beforeEach.
collectionService = jasmine.createSpyObj('collectionService', ['getAuthorizedCollection', 'getAuthorizedCollectionByEntityType']);
collectionService.getAuthorizedCollection.and.callFake(() =>
createSuccessfulRemoteDataObject$(createPaginatedList([collectionTest]))
);
collectionService.getAuthorizedCollectionByEntityType.and.callFake(() =>
createSuccessfulRemoteDataObject$(createPaginatedList([collectionTest]))
);

TestBed.configureTestingModule({
declarations: [AuthorizedCollectionSelectorComponent, VarDirective],
imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])],
providers: [
{ provide: SearchService, useValue: {} },
{ provide: CollectionDataService, useValue: collectionService },
{ provide: NotificationsService, useValue: notificationsService },
{ provide: DSONameService, useValue: dsoNameService },
],
schemas: [NO_ERRORS_SCHEMA]
}).compileComponents();
Expand All @@ -51,24 +67,60 @@ describe('AuthorizedCollectionSelectorComponent', () => {

describe('search', () => {
describe('when has no entity type', () => {
it('should call getAuthorizedCollection and return the authorized collection in a SearchResult', (done) => {
component.search('', 1).subscribe((resultRD) => {
it('should call getAuthorizedCollection and return the collection wrapped in a SearchResult', (done) => {
component.search('', 1).subscribe((resultRD) => {
expect(collectionService.getAuthorizedCollection).toHaveBeenCalled();
expect(resultRD.payload.page.length).toEqual(1);
expect(resultRD.payload.page[0].indexableObject).toEqual(collection);
expect(resultRD.payload.page.length).toEqual(1);
expect(resultRD.payload.page[0].indexableObject).toEqual(collectionTest);
done();
});
});
});

describe('when has entity type', () => {
it('should call getAuthorizedCollectionByEntityType and return the authorized collection in a SearchResult', (done) => {
component.entityType = 'test';
it('should call getAuthorizedCollectionByEntityType and return the collection wrapped in a SearchResult', (done) => {
component.entityType = 'Publication';
fixture.detectChanges();
component.search('', 1).subscribe((resultRD) => {
expect(collectionService.getAuthorizedCollectionByEntityType).toHaveBeenCalled();
expect(resultRD.payload.page.length).toEqual(1);
expect(resultRD.payload.page[0].indexableObject).toEqual(collection);
expect(resultRD.payload.page[0].indexableObject).toEqual(collectionTest);
done();
});
});
});

describe('title prefix filtering', () => {
beforeEach(() => {
// Override to return all three collections so we can test client-side filtering
collectionService.getAuthorizedCollection.and.callFake(() =>
createSuccessfulRemoteDataObject$(
createPaginatedList([collectionTest, collectionTestSuite, collectionCollection])
)
);
});

it('should return all collections when query is empty', (done) => {
component.search('', 1).subscribe((resultRD) => {
expect(resultRD.payload.page.length).toEqual(3);
done();
});
});

it('should return only collections whose title starts with the query', (done) => {
component.search('test', 1).subscribe((resultRD) => {
const names = resultRD.payload.page.map((r: any) => r.indexableObject.name);
expect(names).toEqual(['test', 'test suite']);
expect(names).not.toContain('collection');
done();
});
});

it('should be case-insensitive', (done) => {
component.search('TEST', 1).subscribe((resultRD) => {
const names = resultRD.payload.page.map((r: any) => r.indexableObject.name);
expect(names).toContain('test');
expect(names).toContain('test suite');
done();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ import { DSpaceObject } from '../../../../core/shared/dspace-object.model';
import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
import { followLink } from '../../../utils/follow-link-config.model';
import { RemoteData } from '../../../../core/data/remote-data';
import { hasValue } from '../../../empty.util';
import { hasNoValue, hasValue, isNotEmpty } from '../../../empty.util';
import { NotificationsService } from '../../../notifications/notifications.service';
import { TranslateService } from '@ngx-translate/core';
import { Collection } from '../../../../core/shared/collection.model';
import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
import { FindListOptions } from '../../../../core/data/find-list-options.model';
import { NotificationType } from '../../../notifications/models/notification-type';
import { ListableNotificationObject } from '../../../object-list/listable-notification-object/listable-notification-object.model';
import { LISTABLE_NOTIFICATION_OBJECT } from '../../../object-list/listable-notification-object/listable-notification-object.resource-type';

@Component({
selector: 'ds-authorized-collection-selector',
Expand Down Expand Up @@ -74,9 +77,50 @@ export class AuthorizedCollectionSelectorComponent extends DSOSelectorComponent
}
return searchListService$.pipe(
getFirstCompletedRemoteData(),
map((rd) => Object.assign(new RemoteData(null, null, null, null), rd, {
payload: hasValue(rd.payload) ? buildPaginatedList(rd.payload.pageInfo, rd.payload.page.map((col) => Object.assign(new CollectionSearchResult(), { indexableObject: col }))) : null,
}))
map((rd) => {
if (!hasValue(rd.payload)) {
return Object.assign(new RemoteData(null, null, null, null), rd, { payload: null });
}
let searchResults = rd.payload.page.map((col) =>
Object.assign(new CollectionSearchResult(), { indexableObject: col })
);
if (isNotEmpty(query)) {
const lowerQuery = query.trim().toLowerCase();
searchResults = searchResults.filter((result) => {
const name = this.dsoNameService.getName(result.indexableObject);
return hasValue(name) && name.toLowerCase().startsWith(lowerQuery);
});
}
return Object.assign(new RemoteData(null, null, null, null), rd, {
payload: buildPaginatedList(rd.payload.pageInfo, searchResults),
});
})
);
}

/**
* Override updateList to derive hasNextPage from page-based pagination
* (currentPage < totalPages) instead of totalElements, because client-side
* filtering makes totalElements unreliable for next-page detection.
*/
updateList(rd: RemoteData<PaginatedList<SearchResult<DSpaceObject>>>) {
this.loading = false;
const currentEntries = this.listEntries$.getValue();
if (rd.hasSucceeded) {
if (hasNoValue(currentEntries)) {
this.listEntries$.next(rd.payload.page);
} else {
this.listEntries$.next([...currentEntries, ...rd.payload.page]);
}
// Use page-based check: currentPage is 0-based, totalPages is 1-based
const pageInfo = rd.payload.pageInfo;
this.hasNextPage = hasValue(pageInfo) && pageInfo.currentPage < (pageInfo.totalPages - 1);
} else {
this.listEntries$.next([
...(hasNoValue(currentEntries) ? [] : this.listEntries$.getValue()),
new ListableNotificationObject(NotificationType.Error, 'dso-selector.results-could-not-be-retrieved', LISTABLE_NOTIFICATION_OBJECT.value)
]);
this.hasNextPage = false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe('DSOSelectorComponent', () => {

expect(searchService.search).toHaveBeenCalledWith(
jasmine.objectContaining({
query: undefined,
query: '',
sort: jasmine.objectContaining({
field: 'dc.title',
direction: SortDirection.ASC,
Expand All @@ -158,6 +158,84 @@ describe('DSOSelectorComponent', () => {
});
});

describe('query processing', () => {
beforeEach(() => {
spyOn(searchService, 'search').and.callThrough();
});

describe('for COMMUNITY/COLLECTION types', () => {
beforeEach(() => {
component.types = [DSpaceObjectType.COMMUNITY];
});

it('should create title field query with escaping and wildcards', () => {
component.search('test+query [with] special:chars/paths', 1);

expect(searchService.search).toHaveBeenCalledWith(
jasmine.objectContaining({
query: 'dc.title:("test\\+query" AND "\\[with\\]" AND special\\:chars\\/paths*)'
}),
null,
true
);
});

it('should pass through internal resource ID queries unchanged', () => {
const resourceIdQuery = component.getCurrentDSOQuery();
component.search(resourceIdQuery, 1);

expect(searchService.search).toHaveBeenCalledWith(
jasmine.objectContaining({
query: resourceIdQuery
}),
null,
true
);
});
});

describe('for ITEM types', () => {
beforeEach(() => {
component.types = [DSpaceObjectType.ITEM];
});

it('should pass through queries unchanged', () => {
component.search('test query', 1);

expect(searchService.search).toHaveBeenCalledWith(
jasmine.objectContaining({
query: 'test query'
}),
null,
true
);
});
});

describe('edge cases', () => {
beforeEach(() => {
component.types = [DSpaceObjectType.COMMUNITY];
});

it('should treat whitespace-only query as empty and apply default sort', () => {
component.sort = new SortOptions('dc.title', SortDirection.ASC);
component.search(' ', 1);

expect(searchService.search).toHaveBeenCalledWith(
jasmine.objectContaining({
query: '',
sort: jasmine.objectContaining({
field: 'dc.title',
direction: SortDirection.ASC,
}),
}),
null,
true
);
});
});
});

describe('when search returns an error', () => {
beforeEach(() => {
spyOn(searchService, 'search').and.returnValue(createFailedRemoteDataObject$());
Expand Down
73 changes: 67 additions & 6 deletions src/app/shared/dso-selector/dso-selector/dso-selector.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
*/
@ViewChildren('listEntryElement') listElements: QueryList<ElementRef>;

/**
* The Solr/Lucene field used for title-based prefix queries
*/
protected readonly TITLE_FIELD = 'dc.title';

/**
* Time to wait before sending a search request to the server when a user types something
*/
Expand Down Expand Up @@ -205,8 +210,10 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
} else {
this.listEntries$.next([...currentEntries, ...rd.payload.page]);
}
// Check if there are more pages available after the current one
this.hasNextPage = rd.payload.totalElements > this.listEntries$.getValue().length;
// Check if the server reports a next page, using page-based comparison so that
// client-side filtering (which reduces list length without changing totalPages)
// does not cause repeated fetching past the server's last page.
this.hasNextPage = rd.payload.currentPage < rd.payload.totalPages;
} else {
this.listEntries$.next([...(hasNoValue(currentEntries) ? [] : this.listEntries$.getValue()), new ListableNotificationObject(NotificationType.Error, 'dso-selector.results-could-not-be-retrieved', LISTABLE_NOTIFICATION_OBJECT.value)]);
this.hasNextPage = false;
Expand All @@ -227,16 +234,34 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
* @param useCache Whether or not to use the cache
*/
search(query: string, page: number, useCache: boolean = true): Observable<RemoteData<PaginatedList<SearchResult<DSpaceObject>>>> {
// default sort is only used when there is not query
let efectiveSort = query ? null : this.sort;
const rawQuery = query ?? '';
const trimmedQuery = rawQuery.trim();
const hasQuery = isNotEmpty(trimmedQuery);

// default sort is only used when there is no query
let effectiveSort = hasQuery ? null : this.sort;

let processedQuery = trimmedQuery;
if (isNotEmpty(trimmedQuery)) {
// Bypass query rewriting for internal Solr field queries (e.g. search.resourceid:<uuid>)
const isInternalSolrQuery = /^\w[\w.]*:/.test(trimmedQuery);
if (isInternalSolrQuery) {
processedQuery = trimmedQuery;
} else if (this.types.includes(DSpaceObjectType.COMMUNITY) || this.types.includes(DSpaceObjectType.COLLECTION)) {
processedQuery = this.buildTitlePrefixQuery(trimmedQuery);
} else {
processedQuery = trimmedQuery;
}
}

return this.searchService.search(
new PaginatedSearchOptions({
query: query,
query: processedQuery,
dsoTypes: this.types,
pagination: Object.assign({}, this.defaultPagination, {
currentPage: page
}),
sort: efectiveSort
sort: effectiveSort
}),
null,
useCache,
Expand Down Expand Up @@ -301,6 +326,42 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
}
}

/**
* Builds a dc.title partial matching query with wildcard support.
* Single term: dc.title:term*
* Multiple terms: dc.title:("term1" AND term2*)
* @param query The raw user input query
* @returns The processed query string with dc.title prefix matching, or the original query if empty
*/
protected buildTitlePrefixQuery(query: string): string {
if (hasValue(query) && query.trim().length > 0) {
const trimmedQuery = query.trim();
const escapedQuery = this.escapeLuceneSpecialCharacters(trimmedQuery);
const terms = escapedQuery.split(/\s+/).filter(term => term.length > 0);

if (terms.length === 1) {
return `${this.TITLE_FIELD}:${terms[0]}*`;
} else {
const allButLast = terms.slice(0, -1).map(term => `"${term}"`).join(' AND ');
const lastTerm = terms[terms.length - 1];
return `${this.TITLE_FIELD}:(${allButLast} AND ${lastTerm}*)`;
}
}
return query;
}

/**
* Escapes special Lucene/Solr characters in user input to prevent query syntax errors
* @param query The user input query to escape
* @returns The escaped query string
*/
private escapeLuceneSpecialCharacters(query: string): string {
// Escape special Lucene characters: + - && || ! ( ) { } [ ] ^ " ~ * ? : \ /
return query.replace(/[+\-!(){}[\]^"~*?:\\\/]/g, '\\$&')
.replace(/&&/g, '\\&&')
.replace(/\|\|/g, '\\||');
}

getName(listableObject: ListableObject): string {
return hasValue((listableObject as SearchResult<DSpaceObject>).indexableObject) ?
this.dsoNameService.getName((listableObject as SearchResult<DSpaceObject>).indexableObject) : null;
Expand Down
Loading