Skip to content

Commit 0b284df

Browse files
authored
Merge pull request #65 from salesforce/fix-validator
fix LabelValidator to return more detail
2 parents dde2420 + 7890fd1 commit 0b284df

File tree

5 files changed

+98
-45
lines changed

5 files changed

+98
-45
lines changed

src/main/java/com/force/i18n/LocaleUtils.java

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
/*
1+
/*
22
* Copyright (c) 2017, salesforce.com, inc.
33
* All rights reserved.
4-
* Licensed under the BSD 3-Clause license.
4+
* Licensed under the BSD 3-Clause license.
55
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77

88
package com.force.i18n;
99

1010
import java.util.List;
1111
import java.util.Locale;
12+
import java.util.Locale.LanguageRange;
1213
import java.util.concurrent.ConcurrentHashMap;
1314
import java.util.concurrent.ConcurrentMap;
1415

@@ -60,29 +61,34 @@ public Locale getLocaleFromDbString(String value) {
6061
return getLocaleByIsoCode(value);
6162
}
6263

63-
/**
64-
* HTTP and Java do not use the same locale stuff
65-
* HTTP would say "de-de", but Java would want "de_DE". This handles those kinds of
66-
* weirdness things (like "de-de;q=0.8")
67-
* @param str the HTTP language input
68-
* @return the java locale from the http language input.
69-
*/
70-
public Locale getLocaleFromHttpInput(String str) {
71-
if ("*".equals(str)) return null; // Invalid
72-
int semiIndex = str.indexOf(';');
73-
String locale = str;
74-
if (semiIndex > 0) {
75-
// We have a "quality" rating. Ignore it. It's useless
76-
locale = str.substring(0,semiIndex);
77-
}
78-
// OK, we should have "de" or "de-de";
79-
if (locale.length() == 2) {
80-
return new Locale.Builder().setLanguage(locale.toLowerCase()).build();
81-
} else if (locale.length() == 5) {
82-
if (locale.charAt(2) != '-') return null;
83-
return new Locale.Builder().setLanguage(locale.substring(0,2).toLowerCase()).setRegion(locale.substring(3,5).toUpperCase()).build();
84-
} else {
85-
return null;
86-
}
87-
}
64+
/**
65+
* Parses an "Accept-Language" header value as defined in
66+
* <a href="https://www.rfc-editor.org/rfc/rfc9110.html#name-accept-language">RFC 9110</a> and returns the first
67+
* value.
68+
*
69+
* <p>HTTP and Java handle locales differently. For example, HTTP might use {@code "de-de,de;q=0.8"}, while Java expects
70+
* "de_DE". This method addresses such discrepancies.
71+
*
72+
* @param str
73+
* The HTTP language input, which should follow the "language-range" format of the "Accept-Language"
74+
* header as defined in <a href="https://www.rfc-editor.org/rfc/rfc9110.html#name-accept-language">RFC
75+
* 9110</a>, except for the wildcard '*'. If the wildcard '*' is provided, this method will return
76+
* {@code null}.
77+
* @return The Java locale derived from the HTTP language input. If {@code str} contains multiple languages, only
78+
* the first value is considered, and the rest are ignored. Returns {@code null} if an invalid {@code str}
79+
* is provided.
80+
*/
81+
public Locale getLocaleFromHttpInput(String str) {
82+
if (str == null || "*".equals(str)) return null; // Invalid
83+
84+
try {
85+
final List<LanguageRange> ranges = Locale.LanguageRange.parse(str);
86+
if (ranges != null && !ranges.isEmpty()) {
87+
return Locale.forLanguageTag(ranges.get(0).getRange());
88+
}
89+
} catch (IllegalArgumentException ignore) {
90+
// do nothing. let below to return null
91+
}
92+
return null;
93+
}
8894
}

src/main/java/com/force/i18n/grammar/ArticledDeclension.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ protected void setString(ArticleForm form, String value) {
132132
this.value = intern(value);
133133
}
134134
@Override
135+
public int hashCode() {
136+
return Objects.hash(super.hashCode(), this.value);
137+
}
138+
@Override
139+
public boolean equals(Object other) {
140+
if (this == other) return true;
141+
if (other instanceof SimpleArticle) {
142+
return super.equals(other) && Objects.equals(this.value, ((SimpleArticle)other).value);
143+
}
144+
return false;
145+
}
146+
@Override
135147
public boolean validate(String name) {
136148
if (this.value == null) {
137149
HumanLanguage language = this.getDeclension().getLanguage();

src/main/java/com/force/i18n/grammar/parser/GrammaticalLabelFileParser.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ public enum ErrorType {
486486
public String getMessage(ErrorInfo ref) {
487487
String ret = (ref.getArguments() == null || ref.getArguments().length == 0) ? errorMessage
488488
: MessageFormat.format(errorMessage, ref.getArguments());
489-
return (ref == null) ? ret : ret + " at " + ref.toString();
489+
return String.format("%s at %s (%s:%d)", ret, ref.toString(), ref.file, ref.lineNumber);
490490
}
491491
}
492492

@@ -512,5 +512,23 @@ public class ErrorInfo extends LabelRef {
512512
public String getMessage() {
513513
return this.type.getMessage(this);
514514
}
515+
516+
@Override
517+
public int hashCode() {
518+
return Objects.hash(super.hashCode(), this.type, this.file, this.lineNumber);
519+
}
520+
521+
@Override
522+
public boolean equals(Object other) {
523+
if (this == other) return true;
524+
if (other instanceof ErrorInfo) {
525+
ErrorInfo o = (ErrorInfo)other;
526+
return super.equals(other)
527+
&& this.type == o.type
528+
&& Objects.equals(this.file, o.file)
529+
&& this.lineNumber == o.lineNumber;
530+
}
531+
return false;
532+
}
515533
}
516534
}

src/main/java/com/force/i18n/grammar/parser/LabelValidator.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
/*
1+
/*
22
* Copyright (c) 2017, salesforce.com, inc.
33
* All rights reserved.
4-
* Licensed under the BSD 3-Clause license.
4+
* Licensed under the BSD 3-Clause license.
55
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77

@@ -17,6 +17,7 @@
1717
import com.force.i18n.grammar.GrammaticalTerm.TermType;
1818
import com.force.i18n.grammar.impl.LanguageDeclensionFactory;
1919
import com.force.i18n.grammar.parser.GrammaticalLabelFileParser.AliasParam;
20+
import com.force.i18n.grammar.parser.GrammaticalLabelFileParser.ErrorInfo;
2021
import com.force.i18n.settings.MapPropertyFileData;
2122

2223
/**
@@ -72,11 +73,26 @@ public List<String> getDuplicates(HumanLanguage language) throws IOException {
7273
}
7374

7475
public List<String> getInvalidLabels(HumanLanguage language) throws Exception {
76+
return getInvalidLabels(language, true);
77+
}
78+
79+
/**
80+
* Validates labels for the speicified language.
81+
*
82+
* @param language
83+
* the language to validate labels
84+
* @param onlyLabelKeys
85+
* {@code true} to return only the label key that has an error;
86+
* {@code false} to return detailed error information including file name and line number.
87+
* @return a list of errors; returns an empty list if no errors are found.
88+
* @throws IOException if there was a problem while parsing
89+
*/
90+
public List<String> getInvalidLabels(HumanLanguage language, boolean onlyLabelKeys) throws IOException {
7591
List<String> result = new ArrayList<>();
7692
GrammaticalLabelFileParser parser = getParser(language, false);
7793
if (parser.getInvalidLabels() != null) {
78-
for (LabelReference ref : parser.getInvalidLabels()) {
79-
result.add(ref.toString());
94+
for (ErrorInfo ref : parser.getInvalidLabels()) {
95+
result.add(onlyLabelKeys ? ref.toString() : ref.getMessage());
8096
}
8197
}
8298
return result;
@@ -96,12 +112,6 @@ public List<String> validateReferencedCaseFormsExistTest(List<GrammaticalLabelSe
96112

97113
Set<String> inheritedNouns = ls.getDictionary().getAllInheritedTermNames(TermType.Noun);
98114

99-
// Unwrap the layers of falling back: NOTE, this only works one level deep which is fine for now.
100-
GrammaticalLabelSet main = ls;
101-
if (main instanceof GrammaticalLabelSetFallbackImpl) {
102-
main = ((GrammaticalLabelSetFallbackImpl)main).getOverlay();
103-
}
104-
105115
// Iterate through the label set
106116
for (String section : ls.sectionNames()) {
107117
Map<String, Object> sectionMap = ls.getSection(section);
@@ -160,7 +170,7 @@ public String renderLabel(HumanLanguage language, String text) throws IOExceptio
160170
* @param text the text of the label to parser
161171
* @param grammarOverride the XML set of terms to add to the label set while testing
162172
* @return the rendered label
163-
* @throws IOException if there is an issue reading from the label set descriptor passed into the constructor
173+
* @throws IOException if there is an issue reading from the label set descriptor passed into the constructor
164174
*/
165175
public String renderLabel(HumanLanguage language, String text, String grammarOverride) throws IOException {
166176
return getTestLabelSet(language, text, grammarOverride).getString("Test", "Test");
@@ -193,7 +203,4 @@ LanguageDictionary loadDictionaryNoThrow(GrammaticalLabelSetDescriptor dictDesc)
193203
throw new RuntimeException(e);
194204
}
195205
}
196-
197-
198-
199206
}

src/test/java/com/force/i18n/LocaleUtilsUnitTest.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
/*
1+
/*
22
* Copyright (c) 2017, salesforce.com, inc.
33
* All rights reserved.
4-
* Licensed under the BSD 3-Clause license.
4+
* Licensed under the BSD 3-Clause license.
55
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77

@@ -41,7 +41,17 @@ public void testGetLocaleByIsoCodeWithLanguageCountry() throws Exception {
4141
public void testGetLocaleByIsoCodeWithLanguageCountryVariant() throws Exception {
4242
final String isoCode = "ca_ES_PREEURO";
4343
final Locale actualLocale = LocaleUtils.get().getLocaleByIsoCode(isoCode);
44-
assertEquals("Expected getLocaleByIsoCode to return the Catalan Spain Locale with Euro variant",
44+
assertEquals("Expected getLocaleByIsoCode to return the Catalan Spain Locale with Euro variant",
4545
new Locale.Builder().setLanguage("ca").setRegion("ES").setVariant("PREEURO").build(), actualLocale);
4646
}
47+
48+
public void testGetLocaleFromHttpStrig() {
49+
assertNull(LocaleUtils.get().getLocaleFromHttpInput("*"));
50+
assertNull(LocaleUtils.get().getLocaleFromHttpInput(""));
51+
assertNull(LocaleUtils.get().getLocaleFromHttpInput(null));
52+
53+
assertEquals(Locale.ENGLISH, LocaleUtils.get().getLocaleFromHttpInput("en"));
54+
assertEquals(Locale.ENGLISH, LocaleUtils.get().getLocaleFromHttpInput("en;q=0.8"));
55+
assertEquals(Locale.US, LocaleUtils.get().getLocaleFromHttpInput("en-US,en;q=0.8"));
56+
}
4757
}

0 commit comments

Comments
 (0)