geonetwork / core-geonetwork

GeoNetwork is a catalog application to manage spatially referenced resources. It provides powerful metadata editing and search functions as well as an interactive web map viewer. It is currently used in numerous Spatial Data Infrastructure initiatives across the world.
http://geonetwork-opensource.org/
GNU General Public License v2.0
432 stars 489 forks source link

Add support for subdomains in emails - unit test xsl. #7268

Open sebr72 opened 1 year ago

sebr72 commented 1 year ago

Currently utility-tpl.xsl only support a subset of all possible email addresses. I would like to add emails with subdomains (e.g.: myname@admin.mycompany.com)

I have a fix but it is difficult to make sure this won't introduce new regressions. So I would to unit test a set of valid and invalid email addresses. Mainly the code need to change here.

I'll gladly implement the fix and tests. Could you recommend the preferred way for me to unit tests this xsl ?

jodygarnett commented 1 year ago

Thanks @sebr72 I assume you wish to setup a new regex for:

    <xsl:analyze-string select="$string"
                        regex="([\w\.]+)@([a-zA-Z_]+?\.[a-zA-Z]{{2,3}})">

Because schema plugins were initially just XML content there is not a strong tradition of unit testing; and a strong reliance on PR reviews. I recommend making your PR and asking for a review.

I expected to find a test that validates a document and count the number of errors or search for a specific error message.

Here is an example:

    public void testPasses() throws Exception {
        final Element testMetadata = Xml.loadStream(SchematronRulesIsoTest.class.getResourceAsStream(INSPIRE_VALID_ISO19139_XML));
        Element results = Xml.transform(testMetadata, getSchematronXsl(), params);
        assertEquals(0, countFailures(results));
    }
sebr72 commented 1 year ago

Thanks @jodygarnett . I think it is almost impossible to review a regex (especially a long one), and even harder to make sure you won't introduce errors with code affecting it in the future. So I'll definitely test it.

ianwallen commented 1 year ago

There is an email checker in the HNAP plugin which uses a regular expressions along with a reference for the source of the regular expression. (https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address which is based on the RFC

https://github.com/metadata101/iso19139.ca.HNAP/blob/76260c313522d2a9399b449c6b6bf07a4a39fb35/src/main/java/ca/gc/schema/iso19139hnap/util/XslUtilHnap.java#L260-L276