You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2015/11/05 12:38:33 UTC
Re: svn commit: r1712699 - in /commons/proper/validator/trunk/src:
changes/changes.xml main/java/org/apache/commons/validator/routines/DomainValidator.java
test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
2015-11-05 1:42 GMT+01:00 <se...@apache.org>:
> Author: sebb
> Date: Thu Nov 5 00:42:37 2015
> New Revision: 1712699
>
> URL: http://svn.apache.org/viewvc?rev=1712699&view=rev
> Log:
> VALIDATOR-341 Make TLD list configurable
>
> Modified:
> commons/proper/validator/trunk/src/changes/changes.xml
>
> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
>
> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
>
> Modified: commons/proper/validator/trunk/src/changes/changes.xml
> URL:
> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/changes/changes.xml?rev=1712699&r1=1712698&r2=1712699&view=diff
>
> ==============================================================================
> --- commons/proper/validator/trunk/src/changes/changes.xml (original)
> +++ commons/proper/validator/trunk/src/changes/changes.xml Thu Nov 5
> 00:42:37 2015
> @@ -43,6 +43,10 @@ The <action> type attribute can be add,u
> <body>
>
> <release version="1.5.0" date="tba" description="tba">
> + <action issue="VALIDATOR-341" type="add" dev="sebb">
> + Make TLD list configurable;
> + both generic and country-code now support addition and removal
> + </action>
> <action issue="VALIDATOR-374" type="fix">
> Email Validator : .school domain is being rejected
> Add Unit test to show it has been fixed
>
> Modified:
> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java?rev=1712699&r1=1712698&r2=1712699&view=diff
>
> ==============================================================================
> ---
> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
> (original)
> +++
> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
> Thu Nov 5 00:42:37 2015
> @@ -63,6 +63,8 @@ import java.util.Locale;
> */
> public class DomainValidator implements Serializable {
>
> + private static final String[] EMPTY_STRING_ARRAY = new String[0];
> +
> private static final long serialVersionUID = -4407125112880174009L;
>
> // Regular expression strings for hostnames (derived from RFC2396 and
> RFC 1123)
> @@ -219,7 +221,8 @@ public class DomainValidator implements
> */
> public boolean isValidGenericTld(String gTld) {
> final String key =
> chompLeadingDot(unicodeToASCII(gTld).toLowerCase(Locale.ENGLISH));
> - return arrayContains(GENERIC_TLDS, key);
> + return (arrayContains(GENERIC_TLDS, key) ||
> arrayContains(GENERIC_TLDS_PLUS, key))
> + && !arrayContains(GENERIC_TLDS_MINUS, key);
> }
>
> /**
> @@ -231,7 +234,8 @@ public class DomainValidator implements
> */
> public boolean isValidCountryCodeTld(String ccTld) {
> final String key =
> chompLeadingDot(unicodeToASCII(ccTld).toLowerCase(Locale.ENGLISH));
> - return arrayContains(COUNTRY_CODE_TLDS, key);
> + return (arrayContains(COUNTRY_CODE_TLDS, key) ||
> arrayContains(COUNTRY_CODE_TLDS_PLUS, key))
> + && !arrayContains(COUNTRY_CODE_TLDS_MINUS, key);
> }
>
> /**
> @@ -1385,6 +1389,68 @@ public class DomainValidator implements
> "localhost", // RFC2606 defined
> };
>
> + // Additional arrays to supplement or override the built in ones.
> + // The PLUS arrays are valid keys, the MINUS arrays are invalid keys
> + // The arrays are marked volatile because they are not immutable
> +
> + // WARNING: this array MUST be sorted, otherwise it cannot be
> searched reliably using binary search
> + private static volatile String[] COUNTRY_CODE_TLDS_PLUS =
> EMPTY_STRING_ARRAY;
> +
> + // WARNING: this array MUST be sorted, otherwise it cannot be
> searched reliably using binary search
> + private static volatile String[] GENERIC_TLDS_PLUS =
> EMPTY_STRING_ARRAY;
> +
> + // WARNING: this array MUST be sorted, otherwise it cannot be
> searched reliably using binary search
> + private static volatile String[] COUNTRY_CODE_TLDS_MINUS =
> EMPTY_STRING_ARRAY;
> +
> + // WARNING: this array MUST be sorted, otherwise it cannot be
> searched reliably using binary search
> + private static volatile String[] GENERIC_TLDS_MINUS =
> EMPTY_STRING_ARRAY;
> +
> + enum ArrayType {
> + GENERIC_PLUS,
> + GENERIC_MINUS,
> + COUNTRY_CODE_PLUS,
> + COUNTRY_CODE_MINUS;
> + };
> +
> + // For use by unit test code
> + static void clearTLDOverrides() {
> + COUNTRY_CODE_TLDS_PLUS = EMPTY_STRING_ARRAY;
> + COUNTRY_CODE_TLDS_MINUS = EMPTY_STRING_ARRAY;
> + GENERIC_TLDS_PLUS = EMPTY_STRING_ARRAY;
> + GENERIC_TLDS_MINUS = EMPTY_STRING_ARRAY;
> + }
> + /**
> + * Update one of the TLD override arrays.
> + * This should normally only be done at program startup.
> + *
> + * To clear an override array, provide an empty array.
> + *
> + * @param table the table to update
> + * @param tlds the array of TLDs, must not be null
> + */
> + public static void updateTLDOverride(ArrayType table, String [] tlds)
> {
> + String [] copy = new String[tlds.length];
> + // Comparisons are always done with lower-case entries
> + for (int i = 0; i < tlds.length; i++) {
> + copy[i] = tlds[i].toLowerCase(Locale.ENGLISH);
> + }
> + Arrays.sort(copy);
> + switch(table) {
> + case COUNTRY_CODE_MINUS:
> + COUNTRY_CODE_TLDS_MINUS = copy;
> + break;
> + case COUNTRY_CODE_PLUS:
> + COUNTRY_CODE_TLDS_PLUS = copy;
> + break;
> + case GENERIC_MINUS:
> + GENERIC_TLDS_MINUS = copy;
> + break;
> + case GENERIC_PLUS:
> + GENERIC_TLDS_PLUS = copy;
> + break;
> + }
> + }
> +
> /**
> * Converts potentially Unicode input to punycode.
> * If conversion fails, returns the original input.
>
> Modified:
> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java?rev=1712699&r1=1712698&r2=1712699&view=diff
>
> ==============================================================================
> ---
> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
> (original)
> +++
> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
> Thu Nov 5 00:42:37 2015
> @@ -42,6 +42,8 @@ import java.util.TreeMap;
> import java.util.regex.Matcher;
> import java.util.regex.Pattern;
>
> +import org.apache.commons.validator.routines.DomainValidator.ArrayType;
> +
> import junit.framework.TestCase;
>
> /**
> @@ -55,6 +57,7 @@ public class DomainValidatorTest extends
>
> public void setUp() {
> validator = DomainValidator.getInstance();
> + DomainValidator.clearTLDOverrides();
> }
>
> public void testValidDomains() {
> @@ -296,6 +299,34 @@ public class DomainValidatorTest extends
> assertTrue(sorted);
> }
>
> + public void testUpdateCountryCode() {
> + assertFalse(validator.isValidCountryCodeTld("com")); // cannot be
> valid
> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_PLUS,
> new String[]{"com"});
> + assertTrue(validator.isValidCountryCodeTld("com")); // it is now!
> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
> new String[]{"com"});
> + assertFalse(validator.isValidCountryCodeTld("com")); // show that
> minus overrides the rest
> +
> + assertTrue(validator.isValidCountryCodeTld("ch"));
> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
> new String[]{"ch"});
> + assertFalse(validator.isValidCountryCodeTld("ch"));
> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
> new String[]{"xx"});
> + assertTrue(validator.isValidCountryCodeTld("ch"));
> + }
> +
> + public void testUpdateGeneric() {
> + assertFalse(validator.isValidGenericTld("ch")); // cannot be valid
> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_PLUS, new
> String[]{"ch"});
> + assertTrue(validator.isValidGenericTld("ch")); // it is now!
> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
> String[]{"ch"});
> + assertFalse(validator.isValidGenericTld("ch")); // show that
> minus overrides the rest
> +
> + assertTrue(validator.isValidGenericTld("com"));
> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
> String[]{"com"});
> + assertFalse(validator.isValidGenericTld("com"));
> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
> String[]{"xx"}); // change the minus list
> + assertTrue(validator.isValidGenericTld("com"));
> + }
> +
> // Download and process local copy of
> http://data.iana.org/TLD/tlds-alpha-by-domain.txt
> // Check if the internal TLD table is up to date
> // Check if the internal TLD tables have any spurious entries
>
>
>
I don't think it is a good idea to implement this based on static fields.
It would be better to make DomainValidator instances configurable via the
constructor. Has this been implemented this way, because other validators
use DomainValidator via Singleton access?
Benedikt
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: svn commit: r1712699 - in /commons/proper/validator/trunk/src:
changes/changes.xml main/java/org/apache/commons/validator/routines/DomainValidator.java
test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
Posted by sebb <se...@gmail.com>.
On 5 November 2015 at 15:58, sebb <se...@gmail.com> wrote:
> On 5 November 2015 at 11:38, Benedikt Ritter <br...@apache.org> wrote:
>> 2015-11-05 1:42 GMT+01:00 <se...@apache.org>:
>>
>>> Author: sebb
>>> Date: Thu Nov 5 00:42:37 2015
>>> New Revision: 1712699
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1712699&view=rev
>>> Log:
>>> VALIDATOR-341 Make TLD list configurable
>>>
<snip/>
>>>
>> I don't think it is a good idea to implement this based on static fields.
>> It would be better to make DomainValidator instances configurable via the
>> constructor. Has this been implemented this way, because other validators
>> use DomainValidator via Singleton access?
>
> Yes, the existing code uses static methods on the class.
Sorry, that's not exactly incorrect.
The methods are not static, however the calling code uses one of two
fixed instances (depending on whether they support local URLs or not).
So effectively the methods are static at present.
(is that hemi-static ?)
I don't think there is any need to allow different domain lists for
different instances.
Indeed I think that would be confusing.
So I think the approach is still OK.
However if you feel that there is a use case for supporting multiple
threads with different overrides, this needs to be fixed before the
next release.
It would mean a minor change to the API for the DomainValidator, but
all the validators that use the DomainValidator would also need to be
enhanced to add an option to allow a DV instance to be provided.
I think that would make the feature unusable.
It just occurs to me that it would be possible to disallow use of the
override once getInstance has been called.
This would guarantee that every thread got the same overrides.
I think I will add that check, as an accidental later update to the
list might prove difficult to debug and cause unnecessary queries on
the mailing lists.
(It will need to be overridden for unit tests, but that can be done
with a package-protected method)
> The patch just allows the user to update the static data which is used
> for the validation.
>
> In any case, I don't think it makes sense for different threads to
> have different base data - either "xyz" is a valid domain or it is
> not, and this applies for all users of the code, until such time as a
> new domain is registered (or dropped).
>
> So I cannot see a use case for using multiple instances.
>
> As the Javadoc says, the intention is to allow the user to update the
> base data set at the start of an application.
> The alternative is to wait for the next release of the class, or
> rebuild/replace the class locally.
>
> [The code uses a pre-sorted database - rather than parsing a separate
> datafile - because that speeds up startup.
> But that is orthogonal to the question of whether the class should be
> a singleton.]
>
>> Benedikt
>>
>>
>>
>> --
>> http://people.apache.org/~britter/
>> http://www.systemoutprintln.de/
>> http://twitter.com/BenediktRitter
>> http://github.com/britter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1712699 - in /commons/proper/validator/trunk/src:
changes/changes.xml main/java/org/apache/commons/validator/routines/DomainValidator.java
test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
Posted by sebb <se...@gmail.com>.
On 5 November 2015 at 11:38, Benedikt Ritter <br...@apache.org> wrote:
> 2015-11-05 1:42 GMT+01:00 <se...@apache.org>:
>
>> Author: sebb
>> Date: Thu Nov 5 00:42:37 2015
>> New Revision: 1712699
>>
>> URL: http://svn.apache.org/viewvc?rev=1712699&view=rev
>> Log:
>> VALIDATOR-341 Make TLD list configurable
>>
>> Modified:
>> commons/proper/validator/trunk/src/changes/changes.xml
>>
>> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
>>
>> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
>>
>> Modified: commons/proper/validator/trunk/src/changes/changes.xml
>> URL:
>> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/changes/changes.xml?rev=1712699&r1=1712698&r2=1712699&view=diff
>>
>> ==============================================================================
>> --- commons/proper/validator/trunk/src/changes/changes.xml (original)
>> +++ commons/proper/validator/trunk/src/changes/changes.xml Thu Nov 5
>> 00:42:37 2015
>> @@ -43,6 +43,10 @@ The <action> type attribute can be add,u
>> <body>
>>
>> <release version="1.5.0" date="tba" description="tba">
>> + <action issue="VALIDATOR-341" type="add" dev="sebb">
>> + Make TLD list configurable;
>> + both generic and country-code now support addition and removal
>> + </action>
>> <action issue="VALIDATOR-374" type="fix">
>> Email Validator : .school domain is being rejected
>> Add Unit test to show it has been fixed
>>
>> Modified:
>> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
>> URL:
>> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java?rev=1712699&r1=1712698&r2=1712699&view=diff
>>
>> ==============================================================================
>> ---
>> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
>> (original)
>> +++
>> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java
>> Thu Nov 5 00:42:37 2015
>> @@ -63,6 +63,8 @@ import java.util.Locale;
>> */
>> public class DomainValidator implements Serializable {
>>
>> + private static final String[] EMPTY_STRING_ARRAY = new String[0];
>> +
>> private static final long serialVersionUID = -4407125112880174009L;
>>
>> // Regular expression strings for hostnames (derived from RFC2396 and
>> RFC 1123)
>> @@ -219,7 +221,8 @@ public class DomainValidator implements
>> */
>> public boolean isValidGenericTld(String gTld) {
>> final String key =
>> chompLeadingDot(unicodeToASCII(gTld).toLowerCase(Locale.ENGLISH));
>> - return arrayContains(GENERIC_TLDS, key);
>> + return (arrayContains(GENERIC_TLDS, key) ||
>> arrayContains(GENERIC_TLDS_PLUS, key))
>> + && !arrayContains(GENERIC_TLDS_MINUS, key);
>> }
>>
>> /**
>> @@ -231,7 +234,8 @@ public class DomainValidator implements
>> */
>> public boolean isValidCountryCodeTld(String ccTld) {
>> final String key =
>> chompLeadingDot(unicodeToASCII(ccTld).toLowerCase(Locale.ENGLISH));
>> - return arrayContains(COUNTRY_CODE_TLDS, key);
>> + return (arrayContains(COUNTRY_CODE_TLDS, key) ||
>> arrayContains(COUNTRY_CODE_TLDS_PLUS, key))
>> + && !arrayContains(COUNTRY_CODE_TLDS_MINUS, key);
>> }
>>
>> /**
>> @@ -1385,6 +1389,68 @@ public class DomainValidator implements
>> "localhost", // RFC2606 defined
>> };
>>
>> + // Additional arrays to supplement or override the built in ones.
>> + // The PLUS arrays are valid keys, the MINUS arrays are invalid keys
>> + // The arrays are marked volatile because they are not immutable
>> +
>> + // WARNING: this array MUST be sorted, otherwise it cannot be
>> searched reliably using binary search
>> + private static volatile String[] COUNTRY_CODE_TLDS_PLUS =
>> EMPTY_STRING_ARRAY;
>> +
>> + // WARNING: this array MUST be sorted, otherwise it cannot be
>> searched reliably using binary search
>> + private static volatile String[] GENERIC_TLDS_PLUS =
>> EMPTY_STRING_ARRAY;
>> +
>> + // WARNING: this array MUST be sorted, otherwise it cannot be
>> searched reliably using binary search
>> + private static volatile String[] COUNTRY_CODE_TLDS_MINUS =
>> EMPTY_STRING_ARRAY;
>> +
>> + // WARNING: this array MUST be sorted, otherwise it cannot be
>> searched reliably using binary search
>> + private static volatile String[] GENERIC_TLDS_MINUS =
>> EMPTY_STRING_ARRAY;
>> +
>> + enum ArrayType {
>> + GENERIC_PLUS,
>> + GENERIC_MINUS,
>> + COUNTRY_CODE_PLUS,
>> + COUNTRY_CODE_MINUS;
>> + };
>> +
>> + // For use by unit test code
>> + static void clearTLDOverrides() {
>> + COUNTRY_CODE_TLDS_PLUS = EMPTY_STRING_ARRAY;
>> + COUNTRY_CODE_TLDS_MINUS = EMPTY_STRING_ARRAY;
>> + GENERIC_TLDS_PLUS = EMPTY_STRING_ARRAY;
>> + GENERIC_TLDS_MINUS = EMPTY_STRING_ARRAY;
>> + }
>> + /**
>> + * Update one of the TLD override arrays.
>> + * This should normally only be done at program startup.
>> + *
>> + * To clear an override array, provide an empty array.
>> + *
>> + * @param table the table to update
>> + * @param tlds the array of TLDs, must not be null
>> + */
>> + public static void updateTLDOverride(ArrayType table, String [] tlds)
>> {
>> + String [] copy = new String[tlds.length];
>> + // Comparisons are always done with lower-case entries
>> + for (int i = 0; i < tlds.length; i++) {
>> + copy[i] = tlds[i].toLowerCase(Locale.ENGLISH);
>> + }
>> + Arrays.sort(copy);
>> + switch(table) {
>> + case COUNTRY_CODE_MINUS:
>> + COUNTRY_CODE_TLDS_MINUS = copy;
>> + break;
>> + case COUNTRY_CODE_PLUS:
>> + COUNTRY_CODE_TLDS_PLUS = copy;
>> + break;
>> + case GENERIC_MINUS:
>> + GENERIC_TLDS_MINUS = copy;
>> + break;
>> + case GENERIC_PLUS:
>> + GENERIC_TLDS_PLUS = copy;
>> + break;
>> + }
>> + }
>> +
>> /**
>> * Converts potentially Unicode input to punycode.
>> * If conversion fails, returns the original input.
>>
>> Modified:
>> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
>> URL:
>> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java?rev=1712699&r1=1712698&r2=1712699&view=diff
>>
>> ==============================================================================
>> ---
>> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
>> (original)
>> +++
>> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java
>> Thu Nov 5 00:42:37 2015
>> @@ -42,6 +42,8 @@ import java.util.TreeMap;
>> import java.util.regex.Matcher;
>> import java.util.regex.Pattern;
>>
>> +import org.apache.commons.validator.routines.DomainValidator.ArrayType;
>> +
>> import junit.framework.TestCase;
>>
>> /**
>> @@ -55,6 +57,7 @@ public class DomainValidatorTest extends
>>
>> public void setUp() {
>> validator = DomainValidator.getInstance();
>> + DomainValidator.clearTLDOverrides();
>> }
>>
>> public void testValidDomains() {
>> @@ -296,6 +299,34 @@ public class DomainValidatorTest extends
>> assertTrue(sorted);
>> }
>>
>> + public void testUpdateCountryCode() {
>> + assertFalse(validator.isValidCountryCodeTld("com")); // cannot be
>> valid
>> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_PLUS,
>> new String[]{"com"});
>> + assertTrue(validator.isValidCountryCodeTld("com")); // it is now!
>> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
>> new String[]{"com"});
>> + assertFalse(validator.isValidCountryCodeTld("com")); // show that
>> minus overrides the rest
>> +
>> + assertTrue(validator.isValidCountryCodeTld("ch"));
>> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
>> new String[]{"ch"});
>> + assertFalse(validator.isValidCountryCodeTld("ch"));
>> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS,
>> new String[]{"xx"});
>> + assertTrue(validator.isValidCountryCodeTld("ch"));
>> + }
>> +
>> + public void testUpdateGeneric() {
>> + assertFalse(validator.isValidGenericTld("ch")); // cannot be valid
>> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_PLUS, new
>> String[]{"ch"});
>> + assertTrue(validator.isValidGenericTld("ch")); // it is now!
>> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
>> String[]{"ch"});
>> + assertFalse(validator.isValidGenericTld("ch")); // show that
>> minus overrides the rest
>> +
>> + assertTrue(validator.isValidGenericTld("com"));
>> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
>> String[]{"com"});
>> + assertFalse(validator.isValidGenericTld("com"));
>> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new
>> String[]{"xx"}); // change the minus list
>> + assertTrue(validator.isValidGenericTld("com"));
>> + }
>> +
>> // Download and process local copy of
>> http://data.iana.org/TLD/tlds-alpha-by-domain.txt
>> // Check if the internal TLD table is up to date
>> // Check if the internal TLD tables have any spurious entries
>>
>>
>>
> I don't think it is a good idea to implement this based on static fields.
> It would be better to make DomainValidator instances configurable via the
> constructor. Has this been implemented this way, because other validators
> use DomainValidator via Singleton access?
Yes, the existing code uses static methods on the class.
The patch just allows the user to update the static data which is used
for the validation.
In any case, I don't think it makes sense for different threads to
have different base data - either "xyz" is a valid domain or it is
not, and this applies for all users of the code, until such time as a
new domain is registered (or dropped).
So I cannot see a use case for using multiple instances.
As the Javadoc says, the intention is to allow the user to update the
base data set at the start of an application.
The alternative is to wait for the next release of the class, or
rebuild/replace the class locally.
[The code uses a pre-sorted database - rather than parsing a separate
datafile - because that speeds up startup.
But that is orthogonal to the question of whether the class should be
a singleton.]
> Benedikt
>
>
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org