Skip to content

Commit 5f1f427

Browse files
amvanbarenamvanbaren
authored
Merge pull request #1214 from amvanbaren/databasesearch-complexity
Reduce DatabaseSearchService complexity
2 parents 2c6ce9c + e800379 commit 5f1f427

File tree

2 files changed

+105
-74
lines changed

2 files changed

+105
-74
lines changed

‎server/src/main/java/org/eclipse/openvsx/search/DatabaseSearchService.java‎

Lines changed: 62 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@
2222
import org.springframework.data.elasticsearch.core.SearchHits;
2323
import org.springframework.data.elasticsearch.core.SearchHitsImpl;
2424
import org.springframework.data.elasticsearch.core.TotalHitsRelation;
25+
import org.springframework.data.util.Streamable;
2526
import org.springframework.scheduling.annotation.Async;
2627
import org.springframework.stereotype.Component;
2728

2829
import java.util.*;
29-
import java.util.stream.Collectors;
3030
import java.util.stream.Stream;
3131

3232
import static org.eclipse.openvsx.cache.CacheService.CACHE_AVERAGE_REVIEW_RATING;
@@ -57,50 +57,29 @@ public boolean isEnabled() {
5757
@Cacheable(CACHE_DATABASE_SEARCH)
5858
@CacheEvict(value = CACHE_AVERAGE_REVIEW_RATING, allEntries = true)
5959
public SearchHits<ExtensionSearch> search(ISearchService.Options options) {
60-
// grab all extensions
6160
var matchingExtensions = repositories.findAllActiveExtensions();
61+
matchingExtensions = excludeByNamespace(options, matchingExtensions);
62+
matchingExtensions = excludeByTargetPlatform(options, matchingExtensions);
63+
matchingExtensions = excludeByCategory(options, matchingExtensions);
64+
matchingExtensions = excludeByQueryString(options, matchingExtensions);
6265

63-
// no extensions in the database
64-
if (matchingExtensions.isEmpty()) {
65-
return new SearchHitsImpl<>(0,TotalHitsRelation.OFF, 0f, null, null, Collections.emptyList(), null, null, null);
66-
}
67-
68-
// exlude namespaces
69-
if(options.namespacesToExclude() != null) {
70-
for(var namespaceToExclude : options.namespacesToExclude()) {
71-
matchingExtensions = matchingExtensions.filter(extension -> !extension.getNamespace().getName().equals(namespaceToExclude));
72-
}
73-
}
74-
75-
// filter target platform
76-
if(TargetPlatform.isValid(options.targetPlatform())) {
77-
matchingExtensions = matchingExtensions.filter(extension -> extension.getVersions().stream().anyMatch(ev -> ev.getTargetPlatform().equals(options.targetPlatform())));
78-
}
79-
80-
// filter category
81-
if (options.category() != null) {
82-
matchingExtensions = matchingExtensions.filter(extension -> {
83-
var latest = repositories.findLatestVersion(extension, null, false, true);
84-
return latest.getCategories().stream().anyMatch(category -> category.equalsIgnoreCase(options.category()));
85-
});
86-
}
66+
var sortedExtensions = sortExtensions(options, matchingExtensions);
67+
var totalHits = sortedExtensions.size();
68+
sortedExtensions = applyPaging(options, sortedExtensions);
69+
var searchHits = sortedExtensions.stream()
70+
.map(extensionSearch -> new SearchHit<>(null, null, null, 0.0f, null, null, null, null, null, null, extensionSearch))
71+
.toList();
8772

88-
// filter text
89-
if (options.queryString() != null) {
90-
matchingExtensions = matchingExtensions.filter(extension -> {
91-
var latest = repositories.findLatestVersion(extension, null, false, true);
92-
return extension.getName().toLowerCase().contains(options.queryString().toLowerCase())
93-
|| extension.getNamespace().getName().contains(options.queryString().toLowerCase())
94-
|| (latest.getDescription() != null && latest.getDescription()
95-
.toLowerCase().contains(options.queryString().toLowerCase()))
96-
|| (latest.getDisplayName() != null && latest.getDisplayName()
97-
.toLowerCase().contains(options.queryString().toLowerCase()));
98-
});
99-
}
73+
return new SearchHitsImpl<>(totalHits, TotalHitsRelation.OFF, 0f, null, null, searchHits, null, null, null);
74+
}
10075

101-
// need to perform the sortBy ()
102-
// 'relevance' | 'timestamp' | 'rating' | 'downloadCount';
76+
private List<ExtensionSearch> applyPaging(Options options, List<ExtensionSearch> sortedExtensions) {
77+
var endIndex = Math.min(sortedExtensions.size(), options.requestedOffset() + options.requestedSize());
78+
var startIndex = Math.min(endIndex, options.requestedOffset());
79+
return sortedExtensions.subList(startIndex, endIndex);
80+
}
10381

82+
private List<ExtensionSearch> sortExtensions(Options options, Streamable<Extension> matchingExtensions) {
10483
Stream<ExtensionSearch> searchEntries;
10584
if("relevance".equals(options.sortBy()) || "rating".equals(options.sortBy())) {
10685
var searchStats = new SearchStats(repositories);
@@ -113,42 +92,63 @@ public SearchHits<ExtensionSearch> search(ISearchService.Options options) {
11392
});
11493
}
11594

116-
var comparators = new HashMap<>(Map.of(
95+
var comparators = Map.of(
11796
"relevance", new RelevanceComparator(),
11897
"timestamp", new TimestampComparator(),
11998
"rating", new RatingComparator(),
12099
"downloadCount", new DownloadedCountComparator()
121-
));
100+
);
122101

123102
var comparator = comparators.get(options.sortBy());
124-
if(comparator != null) {
125-
searchEntries = searchEntries.sorted(comparator);
103+
if ("desc".equals(options.sortOrder())) {
104+
comparator = comparator.reversed();
126105
}
127106

128-
var sortedExtensions = searchEntries.collect(Collectors.toList());
107+
return searchEntries.sorted(comparator).toList();
108+
}
129109

130-
// need to do sortOrder
131-
// 'asc' | 'desc';
132-
if ("desc".equals(options.sortOrder())) {
133-
// reverse the order
134-
Collections.reverse(sortedExtensions);
110+
private Streamable<Extension> excludeByQueryString(Options options, Streamable<Extension> matchingExtensions) {
111+
if(options.queryString() == null) {
112+
return matchingExtensions;
135113
}
136114

137-
// Paging
138-
var totalHits = sortedExtensions.size();
139-
var endIndex = Math.min(sortedExtensions.size(), options.requestedOffset() + options.requestedSize());
140-
var startIndex = Math.min(endIndex, options.requestedOffset());
141-
sortedExtensions = sortedExtensions.subList(startIndex, endIndex);
115+
return matchingExtensions.filter(extension -> {
116+
var latest = repositories.findLatestVersion(extension, null, false, true);
117+
return extension.getName().toLowerCase().contains(options.queryString().toLowerCase())
118+
|| extension.getNamespace().getName().contains(options.queryString().toLowerCase())
119+
|| (latest.getDescription() != null && latest.getDescription()
120+
.toLowerCase().contains(options.queryString().toLowerCase()))
121+
|| (latest.getDisplayName() != null && latest.getDisplayName()
122+
.toLowerCase().contains(options.queryString().toLowerCase()));
123+
});
124+
}
142125

143-
List<SearchHit<ExtensionSearch>> searchHits;
144-
if (sortedExtensions.isEmpty()) {
145-
searchHits = Collections.emptyList();
146-
} else {
147-
// client is interested only in the extension IDs
148-
searchHits = sortedExtensions.stream().map(extensionSearch -> new SearchHit<>(null, null, null, 0.0f, null, null, null, null, null, null, extensionSearch)).collect(Collectors.toList());
126+
private Streamable<Extension> excludeByCategory(Options options, Streamable<Extension> matchingExtensions) {
127+
if(options.category() == null) {
128+
return matchingExtensions;
149129
}
150130

151-
return new SearchHitsImpl<>(totalHits, TotalHitsRelation.OFF, 0f, null, null, searchHits, null, null, null);
131+
return matchingExtensions.filter(extension -> {
132+
var latest = repositories.findLatestVersion(extension, null, false, true);
133+
return latest.getCategories().stream().anyMatch(category -> category.equalsIgnoreCase(options.category()));
134+
});
135+
}
136+
137+
private Streamable<Extension> excludeByTargetPlatform(Options options, Streamable<Extension> matchingExtensions) {
138+
if(!TargetPlatform.isValid(options.targetPlatform())) {
139+
return matchingExtensions;
140+
}
141+
142+
return matchingExtensions.filter(extension -> extension.getVersions().stream().anyMatch(ev -> ev.getTargetPlatform().equals(options.targetPlatform())));
143+
}
144+
145+
private Streamable<Extension> excludeByNamespace(Options options, Streamable<Extension> matchingExtensions) {
146+
if(options.namespacesToExclude() == null) {
147+
return matchingExtensions;
148+
}
149+
150+
var namespacesToExclude = List.of(options.namespacesToExclude());
151+
return matchingExtensions.filter(extension -> !namespacesToExclude.contains(extension.getNamespace().getName()));
152152
}
153153

154154
@Override

‎server/src/test/java/org/eclipse/openvsx/search/DatabaseSearchServiceTest.java‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void testCategory() {
5050
var ext3 = mockExtension("openshift", 4.0, 100, 0, "redhat", List.of("Snippets", "Other"));
5151
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3)));
5252

53-
var searchOptions = new ISearchService.Options(null, "Programming Languages", TargetPlatform.NAME_UNIVERSAL, 50, 0, null, null, false, null);
53+
var searchOptions = searchOptions(null, "Programming Languages",50, 0, null, null);
5454
var result = search.search(searchOptions);
5555
// should find two extensions
5656
assertThat(result.getTotalHits()).isEqualTo(2);
@@ -63,7 +63,7 @@ void testRelevance() {
6363
var ext3 = mockExtension("openshift", 1.0, 100, 10, "redhat", List.of("Snippets", "Other"));
6464
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3)));
6565

66-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, "relevance", false, null);
66+
var searchOptions = searchOptions(null, null, 50, 0, null, "relevance");
6767
var result = search.search(searchOptions);
6868
// should find all extensions but order should be different
6969
assertThat(result.getTotalHits()).isEqualTo(3);
@@ -81,7 +81,7 @@ void testReverse() {
8181
var ext2 = mockExtension("java", 4.0, 100, 0, "redhat", List.of("Snippets", "Programming Languages"));
8282
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2)));
8383

84-
var searchOptions = new ISearchService.Options(null, "Programming Languages", TargetPlatform.NAME_UNIVERSAL, 50, 0, "desc", null, false, null);
84+
var searchOptions = searchOptions(null, "Programming Languages", 50, 0, "desc", null);
8585
var result = search.search(searchOptions);
8686
// should find two extensions
8787
assertThat(result.getTotalHits()).isEqualTo(2);
@@ -103,7 +103,7 @@ void testSimplePageSize() {
103103
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4, ext5, ext6, ext7)));
104104

105105
var pageSizeItems = 5;
106-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, pageSizeItems, 0, null, null, false, null);
106+
var searchOptions = searchOptions(null, null, pageSizeItems, 0, null, null);
107107

108108
var result = search.search(searchOptions);
109109
// 7 total hits
@@ -131,7 +131,7 @@ void testPages() {
131131
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4, ext5, ext6, ext7)));
132132

133133
var pageSizeItems = 2;
134-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, pageSizeItems, 4, null, null, false, null);
134+
var searchOptions = searchOptions(null, null, pageSizeItems, 4, null, null);
135135
var result = search.search(searchOptions);
136136

137137
// 7 total hits
@@ -152,7 +152,7 @@ void testQueryStringPublisherName() {
152152
var ext4 = mockExtension("foo", 4.0, 100, 0, "bar", List.of("Other"));
153153
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
154154

155-
var searchOptions = new ISearchService.Options("redhat", null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, null, false, null);
155+
var searchOptions = searchOptions("redhat", null, 50, 0, null, null);
156156
var result = search.search(searchOptions);
157157
// namespace finding
158158
assertThat(result.getTotalHits()).isEqualTo(3);
@@ -172,7 +172,7 @@ void testQueryStringExtensionName() {
172172
var ext4 = mockExtension("foo", 4.0, 100, 0, "bar", List.of("Other"));
173173
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
174174

175-
var searchOptions = new ISearchService.Options("openshift", null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, null, false, null);
175+
var searchOptions = searchOptions("openshift", null, 50, 0, null, null);
176176
var result = search.search(searchOptions);
177177
// extension name finding
178178
assertThat(result.getTotalHits()).isEqualTo(1);
@@ -192,7 +192,7 @@ void testQueryStringDescription() {
192192
var ext4 = mockExtension("foo", 4.0, 100, 0, "bar", List.of("Other"));
193193
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
194194

195-
var searchOptions = new ISearchService.Options("my custom desc", null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, null, false, null);
195+
var searchOptions = searchOptions("my custom desc", null, 50, 0, null, null);
196196
var result = search.search(searchOptions);
197197
// custom description
198198
assertThat(result.getTotalHits()).isEqualTo(1);
@@ -212,7 +212,7 @@ void testQueryStringDisplayName() {
212212
var ext4 = mockExtension("foo", 4.0, 100, 0, "bar", List.of("Other"));
213213
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
214214

215-
var searchOptions = new ISearchService.Options("Red Hat", null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, null, false, null);
215+
var searchOptions = searchOptions("Red Hat", null, 50, 0, null, null);
216216
var result = search.search(searchOptions);
217217

218218
// custom displayname
@@ -235,7 +235,7 @@ void testSortByTimeStamp() {
235235
ext4.getVersions().get(0).setTimestamp(LocalDateTime.parse("2021-10-06T00:00"));
236236
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
237237

238-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, "timestamp", false, null);
238+
var searchOptions = searchOptions(null, null, 50, 0, null, "timestamp");
239239
var result = search.search(searchOptions);
240240
// all extensions should be there
241241
assertThat(result.getTotalHits()).isEqualTo(4);
@@ -256,7 +256,7 @@ void testSortByDownloadCount() {
256256
var ext4 = mockExtension("foo", 4.0, 100, 500, "bar", List.of("Other"));
257257
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
258258

259-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, "downloadCount", false, null);
259+
var searchOptions = searchOptions(null, null, 50, 0, null, "downloadCount");
260260
var result = search.search(searchOptions);
261261
// all extensions should be there
262262
assertThat(result.getTotalHits()).isEqualTo(4);
@@ -277,7 +277,7 @@ void testSortByRating() {
277277
var ext4 = mockExtension("foo", 1.0, 1, 0, "bar", List.of("Other"));
278278
Mockito.when(repositories.findAllActiveExtensions()).thenReturn(Streamable.of(List.of(ext1, ext2, ext3, ext4)));
279279

280-
var searchOptions = new ISearchService.Options(null, null, TargetPlatform.NAME_UNIVERSAL, 50, 0, null, "rating", false, null);
280+
var searchOptions = searchOptions(null, null, 50, 0, null, "rating");
281281
var result = search.search(searchOptions);
282282
// all extensions should be there
283283
assertThat(result.getTotalHits()).isEqualTo(4);
@@ -292,6 +292,37 @@ void testSortByRating() {
292292

293293
// ---------- UTILITY ----------//
294294

295+
private ISearchService.Options searchOptions(
296+
String queryString,
297+
String category,
298+
Integer requestedSize,
299+
Integer requestedOffset,
300+
String sortOrder,
301+
String sortBy
302+
) {
303+
if(requestedSize == null) {
304+
requestedSize = 18;
305+
}
306+
if(requestedOffset == null) {
307+
requestedOffset = 0;
308+
}
309+
if(sortBy == null) {
310+
sortBy = "relevance";
311+
}
312+
313+
return new ISearchService.Options(
314+
queryString,
315+
category,
316+
null,
317+
requestedSize,
318+
requestedOffset,
319+
sortOrder,
320+
sortBy,
321+
false,
322+
null
323+
);
324+
}
325+
295326
long getIdFromExtensionHits(List<SearchHit<ExtensionSearch>> hits, int index) {
296327
return hits.get(index).getContent().getId();
297328
}

0 commit comments

Comments
 (0)