Skip to content

Commit 658c16b

Browse files
Paurikova2Paurikova2
authored
ZCU-PUB/Partial match for community/collection search (#1232)
* added searching by query * removed trailing spaces * Fix dc.title field scoping in search queries * Fix typo: efectiveSort -> effectiveSort in dso-selector component * fix(dso-selector): query normalization and slash escaping * fix query normalization and tests * test: add regression tests, fix query processing * refactor(tests): consolidate redundant query processing test cases * Bypass query rewriting for internal Solr field queries * added partial word match also for new item browsing collection * Add partial matching to item creation * fix(dso-selector): derive hasNextPage from currentPage < totalPages * removed unwanted comments * created constraint from dc.title * removed solr and lucene from comments and methods names
1 parent 73f21e6 commit 658c16b

4 files changed

Lines changed: 262 additions & 27 deletions

File tree

‎src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.spec.ts‎

Lines changed: 68 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,32 +11,48 @@ import { createPaginatedList } from '../../../testing/utils.test';
1111
import { Collection } from '../../../../core/shared/collection.model';
1212
import { DSpaceObjectType } from '../../../../core/shared/dspace-object-type.model';
1313
import { NotificationsService } from '../../../notifications/notifications.service';
14+
import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
1415

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

1920
let collectionService;
20-
let collection;
21-
21+
let dsoNameService: jasmine.SpyObj<DSONameService>;
2222
let notificationsService: NotificationsService;
2323

24+
function createCollection(id: string, name: string): Collection {
25+
return Object.assign(new Collection(), { id, name });
26+
}
27+
28+
const collectionTest = createCollection('col-test', 'test');
29+
const collectionTestSuite = createCollection('col-suite', 'test suite');
30+
const collectionCollection = createCollection('col-collection', 'collection');
31+
2432
beforeEach(waitForAsync(() => {
25-
collection = Object.assign(new Collection(), {
26-
id: 'authorized-collection'
27-
});
28-
collectionService = jasmine.createSpyObj('collectionService', {
29-
getAuthorizedCollection: createSuccessfulRemoteDataObject$(createPaginatedList([collection])),
30-
getAuthorizedCollectionByEntityType: createSuccessfulRemoteDataObject$(createPaginatedList([collection]))
31-
});
33+
dsoNameService = jasmine.createSpyObj('dsoNameService', ['getName']);
34+
dsoNameService.getName.and.callFake((dso: any) => dso?.name ?? '');
35+
3236
notificationsService = jasmine.createSpyObj('notificationsService', ['error']);
37+
38+
// Use callFake so createSuccessfulRemoteDataObject$ is called lazily at spy invocation time
39+
// (not at setup time), avoiding issues with environment not being available during beforeEach.
40+
collectionService = jasmine.createSpyObj('collectionService', ['getAuthorizedCollection', 'getAuthorizedCollectionByEntityType']);
41+
collectionService.getAuthorizedCollection.and.callFake(() =>
42+
createSuccessfulRemoteDataObject$(createPaginatedList([collectionTest]))
43+
);
44+
collectionService.getAuthorizedCollectionByEntityType.and.callFake(() =>
45+
createSuccessfulRemoteDataObject$(createPaginatedList([collectionTest]))
46+
);
47+
3348
TestBed.configureTestingModule({
3449
declarations: [AuthorizedCollectionSelectorComponent, VarDirective],
3550
imports: [TranslateModule.forRoot(), RouterTestingModule.withRoutes([])],
3651
providers: [
3752
{ provide: SearchService, useValue: {} },
3853
{ provide: CollectionDataService, useValue: collectionService },
3954
{ provide: NotificationsService, useValue: notificationsService },
55+
{ provide: DSONameService, useValue: dsoNameService },
4056
],
4157
schemas: [NO_ERRORS_SCHEMA]
4258
}).compileComponents();
@@ -51,24 +67,60 @@ describe('AuthorizedCollectionSelectorComponent', () => {
5167

5268
describe('search', () => {
5369
describe('when has no entity type', () => {
54-
it('should call getAuthorizedCollection and return the authorized collection in a SearchResult', (done) => {
55-
component.search('', 1).subscribe((resultRD) => {
70+
it('should call getAuthorizedCollection and return the collection wrapped in a SearchResult', (done) => {
71+
component.search('', 1).subscribe((resultRD) => {
5672
expect(collectionService.getAuthorizedCollection).toHaveBeenCalled();
57-
expect(resultRD.payload.page.length).toEqual(1);
58-
expect(resultRD.payload.page[0].indexableObject).toEqual(collection);
73+
expect(resultRD.payload.page.length).toEqual(1);
74+
expect(resultRD.payload.page[0].indexableObject).toEqual(collectionTest);
5975
done();
6076
});
6177
});
6278
});
6379

6480
describe('when has entity type', () => {
65-
it('should call getAuthorizedCollectionByEntityType and return the authorized collection in a SearchResult', (done) => {
66-
component.entityType = 'test';
81+
it('should call getAuthorizedCollectionByEntityType and return the collection wrapped in a SearchResult', (done) => {
82+
component.entityType = 'Publication';
6783
fixture.detectChanges();
6884
component.search('', 1).subscribe((resultRD) => {
6985
expect(collectionService.getAuthorizedCollectionByEntityType).toHaveBeenCalled();
7086
expect(resultRD.payload.page.length).toEqual(1);
71-
expect(resultRD.payload.page[0].indexableObject).toEqual(collection);
87+
expect(resultRD.payload.page[0].indexableObject).toEqual(collectionTest);
88+
done();
89+
});
90+
});
91+
});
92+
93+
describe('title prefix filtering', () => {
94+
beforeEach(() => {
95+
// Override to return all three collections so we can test client-side filtering
96+
collectionService.getAuthorizedCollection.and.callFake(() =>
97+
createSuccessfulRemoteDataObject$(
98+
createPaginatedList([collectionTest, collectionTestSuite, collectionCollection])
99+
)
100+
);
101+
});
102+
103+
it('should return all collections when query is empty', (done) => {
104+
component.search('', 1).subscribe((resultRD) => {
105+
expect(resultRD.payload.page.length).toEqual(3);
106+
done();
107+
});
108+
});
109+
110+
it('should return only collections whose title starts with the query', (done) => {
111+
component.search('test', 1).subscribe((resultRD) => {
112+
const names = resultRD.payload.page.map((r: any) => r.indexableObject.name);
113+
expect(names).toEqual(['test', 'test suite']);
114+
expect(names).not.toContain('collection');
115+
done();
116+
});
117+
});
118+
119+
it('should be case-insensitive', (done) => {
120+
component.search('TEST', 1).subscribe((resultRD) => {
121+
const names = resultRD.payload.page.map((r: any) => r.indexableObject.name);
122+
expect(names).toContain('test');
123+
expect(names).toContain('test suite');
72124
done();
73125
});
74126
});

‎src/app/shared/dso-selector/dso-selector/authorized-collection-selector/authorized-collection-selector.component.ts‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ import { DSpaceObject } from '../../../../core/shared/dspace-object.model';
1111
import { buildPaginatedList, PaginatedList } from '../../../../core/data/paginated-list.model';
1212
import { followLink } from '../../../utils/follow-link-config.model';
1313
import { RemoteData } from '../../../../core/data/remote-data';
14-
import { hasValue } from '../../../empty.util';
14+
import { hasNoValue, hasValue, isNotEmpty } from '../../../empty.util';
1515
import { NotificationsService } from '../../../notifications/notifications.service';
1616
import { TranslateService } from '@ngx-translate/core';
1717
import { Collection } from '../../../../core/shared/collection.model';
1818
import { DSONameService } from '../../../../core/breadcrumbs/dso-name.service';
1919
import { FindListOptions } from '../../../../core/data/find-list-options.model';
20+
import { NotificationType } from '../../../notifications/models/notification-type';
21+
import { ListableNotificationObject } from '../../../object-list/listable-notification-object/listable-notification-object.model';
22+
import { LISTABLE_NOTIFICATION_OBJECT } from '../../../object-list/listable-notification-object/listable-notification-object.resource-type';
2023

2124
@Component({
2225
selector: 'ds-authorized-collection-selector',
@@ -74,9 +77,50 @@ export class AuthorizedCollectionSelectorComponent extends DSOSelectorComponent
7477
}
7578
return searchListService$.pipe(
7679
getFirstCompletedRemoteData(),
77-
map((rd) => Object.assign(new RemoteData(null, null, null, null), rd, {
78-
payload: hasValue(rd.payload) ? buildPaginatedList(rd.payload.pageInfo, rd.payload.page.map((col) => Object.assign(new CollectionSearchResult(), { indexableObject: col }))) : null,
79-
}))
80+
map((rd) => {
81+
if (!hasValue(rd.payload)) {
82+
return Object.assign(new RemoteData(null, null, null, null), rd, { payload: null });
83+
}
84+
let searchResults = rd.payload.page.map((col) =>
85+
Object.assign(new CollectionSearchResult(), { indexableObject: col })
86+
);
87+
if (isNotEmpty(query)) {
88+
const lowerQuery = query.trim().toLowerCase();
89+
searchResults = searchResults.filter((result) => {
90+
const name = this.dsoNameService.getName(result.indexableObject);
91+
return hasValue(name) && name.toLowerCase().startsWith(lowerQuery);
92+
});
93+
}
94+
return Object.assign(new RemoteData(null, null, null, null), rd, {
95+
payload: buildPaginatedList(rd.payload.pageInfo, searchResults),
96+
});
97+
})
8098
);
8199
}
100+
101+
/**
102+
* Override updateList to derive hasNextPage from page-based pagination
103+
* (currentPage < totalPages) instead of totalElements, because client-side
104+
* filtering makes totalElements unreliable for next-page detection.
105+
*/
106+
updateList(rd: RemoteData<PaginatedList<SearchResult<DSpaceObject>>>) {
107+
this.loading = false;
108+
const currentEntries = this.listEntries$.getValue();
109+
if (rd.hasSucceeded) {
110+
if (hasNoValue(currentEntries)) {
111+
this.listEntries$.next(rd.payload.page);
112+
} else {
113+
this.listEntries$.next([...currentEntries, ...rd.payload.page]);
114+
}
115+
// Use page-based check: currentPage is 0-based, totalPages is 1-based
116+
const pageInfo = rd.payload.pageInfo;
117+
this.hasNextPage = hasValue(pageInfo) && pageInfo.currentPage < (pageInfo.totalPages - 1);
118+
} else {
119+
this.listEntries$.next([
120+
...(hasNoValue(currentEntries) ? [] : this.listEntries$.getValue()),
121+
new ListableNotificationObject(NotificationType.Error, 'dso-selector.results-could-not-be-retrieved', LISTABLE_NOTIFICATION_OBJECT.value)
122+
]);
123+
this.hasNextPage = false;
124+
}
125+
}
82126
}

‎src/app/shared/dso-selector/dso-selector/dso-selector.component.spec.ts‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ describe('DSOSelectorComponent', () => {
132132

133133
expect(searchService.search).toHaveBeenCalledWith(
134134
jasmine.objectContaining({
135-
query: undefined,
135+
query: '',
136136
sort: jasmine.objectContaining({
137137
field: 'dc.title',
138138
direction: SortDirection.ASC,
@@ -158,6 +158,84 @@ describe('DSOSelectorComponent', () => {
158158
});
159159
});
160160

161+
describe('query processing', () => {
162+
beforeEach(() => {
163+
spyOn(searchService, 'search').and.callThrough();
164+
});
165+
166+
describe('for COMMUNITY/COLLECTION types', () => {
167+
beforeEach(() => {
168+
component.types = [DSpaceObjectType.COMMUNITY];
169+
});
170+
171+
it('should create title field query with escaping and wildcards', () => {
172+
component.search('test+query [with] special:chars/paths', 1);
173+
174+
expect(searchService.search).toHaveBeenCalledWith(
175+
jasmine.objectContaining({
176+
query: 'dc.title:("test\\+query" AND "\\[with\\]" AND special\\:chars\\/paths*)'
177+
}),
178+
null,
179+
true
180+
);
181+
});
182+
183+
it('should pass through internal resource ID queries unchanged', () => {
184+
const resourceIdQuery = component.getCurrentDSOQuery();
185+
component.search(resourceIdQuery, 1);
186+
187+
expect(searchService.search).toHaveBeenCalledWith(
188+
jasmine.objectContaining({
189+
query: resourceIdQuery
190+
}),
191+
null,
192+
true
193+
);
194+
});
195+
});
196+
197+
describe('for ITEM types', () => {
198+
beforeEach(() => {
199+
component.types = [DSpaceObjectType.ITEM];
200+
});
201+
202+
it('should pass through queries unchanged', () => {
203+
component.search('test query', 1);
204+
205+
expect(searchService.search).toHaveBeenCalledWith(
206+
jasmine.objectContaining({
207+
query: 'test query'
208+
}),
209+
null,
210+
true
211+
);
212+
});
213+
});
214+
215+
describe('edge cases', () => {
216+
beforeEach(() => {
217+
component.types = [DSpaceObjectType.COMMUNITY];
218+
});
219+
220+
it('should treat whitespace-only query as empty and apply default sort', () => {
221+
component.sort = new SortOptions('dc.title', SortDirection.ASC);
222+
component.search(' ', 1);
223+
224+
expect(searchService.search).toHaveBeenCalledWith(
225+
jasmine.objectContaining({
226+
query: '',
227+
sort: jasmine.objectContaining({
228+
field: 'dc.title',
229+
direction: SortDirection.ASC,
230+
}),
231+
}),
232+
null,
233+
true
234+
);
235+
});
236+
});
237+
});
238+
161239
describe('when search returns an error', () => {
162240
beforeEach(() => {
163241
spyOn(searchService, 'search').and.returnValue(createFailedRemoteDataObject$());

‎src/app/shared/dso-selector/dso-selector/dso-selector.component.ts‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
120120
*/
121121
@ViewChildren('listEntryElement') listElements: QueryList<ElementRef>;
122122

123+
/**
124+
* Field used for title-based prefix queries
125+
*/
126+
protected readonly TITLE_FIELD = 'dc.title';
127+
123128
/**
124129
* Time to wait before sending a search request to the server when a user types something
125130
*/
@@ -205,8 +210,10 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
205210
} else {
206211
this.listEntries$.next([...currentEntries, ...rd.payload.page]);
207212
}
208-
// Check if there are more pages available after the current one
209-
this.hasNextPage = rd.payload.totalElements > this.listEntries$.getValue().length;
213+
// Check if the server reports a next page, using page-based comparison so that
214+
// client-side filtering (which reduces list length without changing totalPages)
215+
// does not cause repeated fetching past the server's last page.
216+
this.hasNextPage = rd.payload.currentPage < rd.payload.totalPages;
210217
} else {
211218
this.listEntries$.next([...(hasNoValue(currentEntries) ? [] : this.listEntries$.getValue()), new ListableNotificationObject(NotificationType.Error, 'dso-selector.results-could-not-be-retrieved', LISTABLE_NOTIFICATION_OBJECT.value)]);
212219
this.hasNextPage = false;
@@ -227,16 +234,34 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
227234
* @param useCache Whether or not to use the cache
228235
*/
229236
search(query: string, page: number, useCache: boolean = true): Observable<RemoteData<PaginatedList<SearchResult<DSpaceObject>>>> {
230-
// default sort is only used when there is not query
231-
let efectiveSort = query ? null : this.sort;
237+
const rawQuery = query ?? '';
238+
const trimmedQuery = rawQuery.trim();
239+
const hasQuery = isNotEmpty(trimmedQuery);
240+
241+
// default sort is only used when there is no query
242+
let effectiveSort = hasQuery ? null : this.sort;
243+
244+
let processedQuery = trimmedQuery;
245+
if (isNotEmpty(trimmedQuery)) {
246+
// Bypass query rewriting for internal field queries (e.g. search.resourceid:<uuid>)
247+
const isInternalFieldQuery = /^\w[\w.]*:/.test(trimmedQuery);
248+
if (isInternalFieldQuery ) {
249+
processedQuery = trimmedQuery;
250+
} else if (this.types.includes(DSpaceObjectType.COMMUNITY) || this.types.includes(DSpaceObjectType.COLLECTION)) {
251+
processedQuery = this.buildTitlePrefixQuery(trimmedQuery);
252+
} else {
253+
processedQuery = trimmedQuery;
254+
}
255+
}
256+
232257
return this.searchService.search(
233258
new PaginatedSearchOptions({
234-
query: query,
259+
query: processedQuery,
235260
dsoTypes: this.types,
236261
pagination: Object.assign({}, this.defaultPagination, {
237262
currentPage: page
238263
}),
239-
sort: efectiveSort
264+
sort: effectiveSort
240265
}),
241266
null,
242267
useCache,
@@ -301,6 +326,42 @@ export class DSOSelectorComponent implements OnInit, OnDestroy {
301326
}
302327
}
303328

329+
/**
330+
* Builds a dc.title partial matching query with wildcard support.
331+
* Single term: dc.title:term*
332+
* Multiple terms: dc.title:("term1" AND term2*)
333+
* @param query The raw user input query
334+
* @returns The processed query string with dc.title prefix matching, or the original query if empty
335+
*/
336+
protected buildTitlePrefixQuery(query: string): string {
337+
if (hasValue(query) && query.trim().length > 0) {
338+
const trimmedQuery = query.trim();
339+
const escapedQuery = this.escapeQuerySpecialCharacters(trimmedQuery);
340+
const terms = escapedQuery.split(/\s+/).filter(term => term.length > 0);
341+
342+
if (terms.length === 1) {
343+
return `${this.TITLE_FIELD}:${terms[0]}*`;
344+
} else {
345+
const allButLast = terms.slice(0, -1).map(term => `"${term}"`).join(' AND ');
346+
const lastTerm = terms[terms.length - 1];
347+
return `${this.TITLE_FIELD}:(${allButLast} AND ${lastTerm}*)`;
348+
}
349+
}
350+
return query;
351+
}
352+
353+
/**
354+
* Escapes special query characters in user input to prevent syntax errors
355+
* @param query The user input query to escape
356+
* @returns The escaped query string
357+
*/
358+
private escapeQuerySpecialCharacters(query: string): string {
359+
// Escape special characters used in query syntax
360+
return query.replace(/[+\-!(){}[\]^"~*?:\\\/]/g, '\\$&')
361+
.replace(/&&/g, '\\&&')
362+
.replace(/\|\|/g, '\\||');
363+
}
364+
304365
getName(listableObject: ListableObject): string {
305366
return hasValue((listableObject as SearchResult<DSpaceObject>).indexableObject) ?
306367
this.dsoNameService.getName((listableObject as SearchResult<DSpaceObject>).indexableObject) : null;

0 commit comments

Comments
 (0)