Skip to content

Commit f62119e

Browse files
srl295srl295
authored
CLDR-18423 search: feature and robustness improvements (#4611)
1 parent 19f7d4a commit f62119e

File tree

4 files changed

+121
-28
lines changed

4 files changed

+121
-28
lines changed

‎tools/cldr-apps/src/main/java/org/unicode/cldr/web/SearchManager.java‎

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@
1010
import java.util.Set;
1111
import java.util.TreeSet;
1212
import java.util.concurrent.Callable;
13+
import java.util.concurrent.ExecutionException;
1314
import java.util.concurrent.Future;
1415
import java.util.concurrent.TimeUnit;
16+
import java.util.concurrent.TimeoutException;
17+
import java.util.logging.Level;
1518
import java.util.logging.Logger;
1619
import org.eclipse.microprofile.openapi.annotations.media.Schema;
1720
import org.unicode.cldr.util.CLDRFile;
@@ -117,6 +120,7 @@ synchronized void addResult(SearchResult r) {
117120

118121
/** Mark the search as complete. */
119122
synchronized void complete() {
123+
logger.finest(() -> "completing..");
120124
if (isComplete || !isOngoing) return;
121125
isComplete = true;
122126
isOngoing = false;
@@ -167,20 +171,22 @@ private class Search implements Callable<Search> {
167171
public void begin() {
168172
// TODO: could look up cached results here.
169173

170-
this.future = SurveyThreadManager.getExecutorService().submit(this);
174+
future = SurveyThreadManager.getExecutorService().submit(this);
171175
}
172176

173177
final CLDRLocale EN = CLDRLocale.getInstance("en");
174178

175179
@Override
176180
public Search call() throws Exception {
181+
logger.finest("begin call()");
177182
// first, simple search of exact values
178183
addExactMatches(locale);
179-
180184
CLDRLocale cLoc = CLDRLocale.getInstance(locale);
181185
CLDRLocale parLoc = cLoc.getParent();
182186
while (response.isEmpty() && parLoc != null) {
183-
addExactMatches(parLoc.getBaseName());
187+
final String parLocName = parLoc.getBaseName();
188+
logger.finest(() -> " trying " + parLocName);
189+
addExactMatches(parLocName);
184190
parLoc = parLoc.getParent();
185191
}
186192

@@ -192,37 +198,40 @@ public Search call() throws Exception {
192198
addExactMatches(EN.getBaseName());
193199
}
194200

201+
logger.finest(() -> "completing");
195202
// All done (for now!)
196203
response.complete();
197204

198205
return this;
199206
}
200207

201208
private void addCodes(final String locale) {
209+
logger.finest(() -> "AddCodes " + locale);
202210
final CLDRFile resolvedFile = factory.make(locale, true);
211+
logger.finest(() -> "AddCodes made resolved " + locale);
212+
203213
for (final String x : resolvedFile.fullIterable()) {
214+
final PathHeader ph = phf.fromPath(x);
215+
if (!ph.getSurveyToolStatus().visible()) continue; // skip invisible paths
216+
204217
// match exact xpath
205218
if (x.equals(request.value)) {
206219
response.addResult(new SearchResult(x, "Exact XPath", locale));
207220
return;
208221
}
209222

210-
final PathHeader ph = phf.fromPath(x);
211-
if (ph.getCode().equalsIgnoreCase(request.value)) {
212-
response.addResult(new SearchResult(x, "code: " + ph.getCode(), locale));
213-
return;
214-
}
215-
}
216-
// now try partial
217-
for (final String x : resolvedFile.fullIterable()) {
218223
// match partial xpath
219224
if (x.startsWith(request.value)) {
220225
response.addResult(new SearchResult(x, "Partial XPath", locale));
221226
return;
222227
}
223228

229+
if (ph.getCode().equalsIgnoreCase(request.value)) {
230+
response.addResult(new SearchResult(x, "code: " + ph.getCode(), locale));
231+
return;
232+
}
233+
224234
// partial code
225-
final PathHeader ph = phf.fromPath(x);
226235
if (ph.getCode().contains(request.value)) {
227236
response.addResult(new SearchResult(x, "code: " + ph.getCode(), locale));
228237
return;
@@ -231,14 +240,20 @@ private void addCodes(final String locale) {
231240
}
232241

233242
private CLDRFile addExactMatches(final String locale) {
243+
logger.finest(() -> "AEM " + locale + " on " + factory);
234244
CLDRFile file = factory.make(locale, false);
245+
logger.finest(() -> "AEM called make on " + locale);
235246
Set<String> xresult = new TreeSet<>();
236247
file.getPathsWithValue(request.value, "", null, xresult);
237248
for (final String xpath : xresult) {
238249
// Skip if this isn’t found "here"
239250
if (!file.isHere(xpath)) {
240251
continue;
241252
}
253+
254+
final PathHeader ph = phf.fromPath(xpath);
255+
if (!ph.getSurveyToolStatus().visible()) continue; // skip invisible paths
256+
242257
// Add incrementally. A user may get a partial result if they request before we are
243258
// done.
244259
response.addResult(new SearchResult(xpath, request.value, locale));
@@ -294,11 +309,35 @@ public SearchResponse newSearch(final SearchRequest request, final String locale
294309
*
295310
* @param token
296311
* @return
312+
* @throws TimeoutException
313+
* @throws ExecutionException
314+
* @throws InterruptedException
297315
*/
298-
public SearchResponse getSearch(final String token) {
299-
Search s = (Search) searches.getIfPresent(token);
300-
if (s == null) return null;
301-
return s.response;
316+
public SearchResponse getSearch(final String token) throws ExecutionException {
317+
final Search s = (Search) searches.getIfPresent(token);
318+
if (s == null) return null; // not present
319+
try {
320+
return s.future.get(1, TimeUnit.SECONDS).response;
321+
} catch (InterruptedException | TimeoutException e) {
322+
// the exception here isn't interesting
323+
logger.log(
324+
Level.FINEST,
325+
() ->
326+
String.format(
327+
"Search %s was interrupted or timeout, but will return best available result: %s",
328+
token, e.toString()));
329+
return s.response;
330+
} catch (ExecutionException e) {
331+
logger.log(
332+
Level.WARNING,
333+
e,
334+
() ->
335+
String.format(
336+
"Search %s had exception %s",
337+
token, e.getCause().getMessage()));
338+
s.response.complete(); // mark as complete
339+
throw e;
340+
}
302341
}
303342

304343
/**

‎tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/SearchAPI.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.unicode.cldr.web.api;
22

3+
import java.util.concurrent.ExecutionException;
34
import javax.ws.rs.Consumes;
45
import javax.ws.rs.DELETE;
56
import javax.ws.rs.GET;
@@ -121,11 +122,16 @@ public Response searchStatus(
121122

122123
final SearchManager searchManager = SearchAPIHelper.getSearchManager();
123124

124-
SearchResponse search = searchManager.getSearch(token);
125-
if (search == null) {
126-
return Response.status(Status.NOT_FOUND).build();
125+
try {
126+
final SearchResponse search = searchManager.getSearch(token);
127+
if (search == null) {
128+
return Response.status(Status.NOT_FOUND).build();
129+
}
130+
return Response.ok(search).build();
131+
} catch (ExecutionException e) {
132+
// error was thrown searching.
133+
return Response.status(500, e.getCause().getMessage()).build();
127134
}
128-
return Response.ok(search).build();
129135
}
130136

131137
@Path("/status/{token}")

‎tools/cldr-apps/src/test/java/org/unicode/cldr/web/TestSearchManager.java‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
import static org.junit.jupiter.api.Assertions.assertFalse;
66
import static org.junit.jupiter.api.Assertions.assertNotNull;
77
import static org.junit.jupiter.api.Assertions.assertNull;
8+
import static org.junit.jupiter.api.Assertions.assertThrows;
89
import static org.junit.jupiter.api.Assertions.assertTrue;
910

1011
import com.google.common.collect.ImmutableSet;
1112
import java.io.IOException;
1213
import java.util.Collections;
1314
import java.util.Set;
1415
import java.util.TreeSet;
16+
import java.util.concurrent.ExecutionException;
1517
import org.junit.jupiter.api.AfterAll;
1618
import org.junit.jupiter.api.BeforeAll;
1719
import org.junit.jupiter.api.Test;
@@ -34,7 +36,7 @@ private static void teardown() throws IOException {
3436
}
3537

3638
@Test
37-
void TestMaltese() throws InterruptedException {
39+
void TestMaltese() throws InterruptedException, ExecutionException {
3840
final String XPATH =
3941
"//ldml/dates/calendars/calendar[@type=\"gregorian\"]/months/monthContext[@type=\"format\"]/monthWidth[@type=\"wide\"]/month[@type=\"3\"]";
4042
final String searchText = "Marzu";
@@ -81,6 +83,39 @@ void TestCode() throws InterruptedException {
8183
);
8284
}
8385

86+
@Test
87+
void TestCodeBF() throws InterruptedException {
88+
final String XPATH = "//ldml/localeDisplayNames/territories/territory[@type=\"BF\"]";
89+
final String searchText = "BF";
90+
final String locale = "ff_Adlm_BF";
91+
assertAll(
92+
"looking for " + searchText + " in " + locale,
93+
() ->
94+
testOneSearchResult(
95+
XPATH,
96+
searchText,
97+
locale,
98+
locale,
99+
"code: " + searchText) // should match in the locale
100+
);
101+
}
102+
103+
@Test
104+
void TestCodeXJ() throws InterruptedException {
105+
final String searchText = "XJ";
106+
final String locale = "root"; // won't find XJ
107+
assertAll(
108+
"looking for " + searchText + " in " + locale,
109+
() -> testNoResult(searchText, locale));
110+
}
111+
112+
@Test
113+
void TestExceptionInSearch() throws InterruptedException {
114+
final String searchText = "XJ";
115+
final String locale = "mul"; // will throw
116+
assertThrows(ExecutionException.class, () -> testNoResult(searchText, locale));
117+
}
118+
84119
@Test
85120
void TestKorean() throws InterruptedException {
86121
final String XPATH =
@@ -98,7 +133,7 @@ private void testOneSearchResult(
98133
final String searchText,
99134
final String locale,
100135
final String resultLocale)
101-
throws InterruptedException {
136+
throws InterruptedException, ExecutionException {
102137
testOneSearchResult(XPATH, searchText, locale, resultLocale, searchText);
103138
}
104139

@@ -108,21 +143,21 @@ private void testOneSearchResult(
108143
final String locale,
109144
final String resultLocale,
110145
final String expectContext)
111-
throws InterruptedException {
146+
throws InterruptedException, ExecutionException {
112147
final Set<SearchResult> r =
113148
ImmutableSet.of(new SearchResult(XPATH, expectContext, resultLocale));
114149

115150
testSearchResult(searchText, locale, r);
116151
}
117152

118153
private void testNoResult(final String searchText, final String locale)
119-
throws InterruptedException {
154+
throws InterruptedException, ExecutionException {
120155
testSearchResult(searchText, locale, Collections.emptySet());
121156
}
122157

123158
private void testSearchResult(
124159
final String searchText, final String locale, Set<SearchResult> expected)
125-
throws InterruptedException {
160+
throws InterruptedException, ExecutionException {
126161
// Create a SearchManager over disk files
127162
assertNotNull(mgr);
128163
final String searchContext = searchText + " in " + locale;
@@ -133,7 +168,7 @@ private void testSearchResult(
133168
assertNotNull(r0.token);
134169
// can't assume it has not already completed.
135170

136-
final int PATIENCE = 40; // x .1 seconds = 4 seconds
171+
final int PATIENCE = 6000; // x .1 seconds
137172
for (int n = 0; n < PATIENCE; n++) {
138173
final SearchResponse r1 = mgr.getSearch(r0.token);
139174
if (r1.isOngoing) {
@@ -148,6 +183,12 @@ private void testSearchResult(
148183
final Set<SearchResult> results = new TreeSet<SearchResult>();
149184
for (final SearchResult result : r1.getResults()) {
150185
results.add(result);
186+
assertFalse(
187+
result.xpath.contains("/identity"),
188+
() ->
189+
String.format(
190+
"Did not expect /identity xpaths present in %s - %s",
191+
result.xpath, searchContext));
151192
}
152193
if (expected.isEmpty()) {
153194
assertEquals(
@@ -201,7 +242,7 @@ private void testSearchResult(
201242
}
202243

203244
@Test
204-
void TestApi() {
245+
void TestApi() throws ExecutionException {
205246
// Create a SearchManager over disk files
206247
SearchManager mgr = SearchManager.forFactory(CLDRConfig.getInstance().getCldrFactory());
207248
assertNotNull(mgr);

‎tools/cldr-code/src/main/java/org/unicode/cldr/util/PathHeader.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,14 @@ public enum SurveyToolStatus {
9292
* Changes are allowed as READ_WRITE, but field is always displayed as LTR, even in RTL
9393
* locales (used for patterns).
9494
*/
95-
LTR_ALWAYS
95+
LTR_ALWAYS;
96+
97+
/**
98+
* @returns true if visible in surveytool
99+
*/
100+
public final boolean visible() {
101+
return (this != DEPRECATED && this != HIDE);
102+
}
96103
}
97104

98105
private static final EnumNames<SectionId> SectionIdNames = new EnumNames<>();

0 commit comments

Comments
 (0)