Licenses on XML files

[Anthony Baker <ab...@pivotal.io>]

I was adding licenses to XML files this weekend and noticed a problem.  This code expects a certain structure to the xml doc that doesn’t allow for a comment:

 /**
   * Build <code>cache</code> element map for given <cod>doc</code>'s
   * schemaLocation for {@link CacheXml#NAMESPACE}.
   *
   * @param doc
   *          {@link Document} to parse schema for.
   * @return Element map
   * @throws IOException
   * @throws ParserConfigurationException
   * @throws SAXException
   * @throws XPathExpressionException
   * @since 8.1
   */
  public static LinkedHashMap<String, CacheElement> buildElementMap(final Document doc) throws IOException, XPathExpressionException, SAXException, ParserConfigurationException {
    final Map<String, List<String>> schemaLocationMap = XmlUtils.buildSchemaLocationMap(
        getAttribute(doc.getFirstChild(), W3C_XML_SCHEMA_INSTANCE_ATTRIBUTE_SCHEMA_LOCATION, W3C_XML_SCHEMA_INSTANCE_NS_URI));

    final LinkedHashMap<String, CacheElement> elementMap = new LinkedHashMap<String, CacheElement>();

    buildElementMapCacheType(elementMap, resolveSchema(schemaLocationMap, CacheXml.NAMESPACE));

    // if we are ever concerned with the order of extensions or children process them here.

    return elementMap;
  }

If you add a license header to CacheElementJUnitTest.xml and run CacheElementJUnitTest you will get an NPE.


So two questions:

1) Do all of our test xml files need license headers (it does make the exclusion list for RAT simple)?
2) Is the code above correct?

Thanks,
Anthony

Re: Licenses on XML files

[Niall Pemberton <ni...@gmail.com>]

On Mon, Dec 7, 2015 at 5:35 PM, Anthony Baker <ab...@pivotal.io> wrote:

> I was adding licenses to XML files this weekend and noticed a problem.
> This code expects a certain structure to the xml doc that doesn’t allow for
> a comment:
>
>  /**
>    * Build <code>cache</code> element map for given <cod>doc</code>'s
>    * schemaLocation for {@link CacheXml#NAMESPACE}.
>    *
>    * *@param* doc
>    *          {@link Document} to parse schema for.
>    * *@return* Element map
>    * *@throws* IOException
>    * *@throws* ParserConfigurationException
>    * *@throws* SAXException
>    * *@throws* XPathExpressionException
>    * *@since* 8.1
>    */
>   *public* *static* LinkedHashMap<String, CacheElement> buildElementMap(
> *final* Document doc) *throws* IOException, XPathExpressionException,
> SAXException, ParserConfigurationException {
>     *final* Map<String, List<String>> schemaLocationMap = XmlUtils.
> *buildSchemaLocationMap*(
>         *getAttribute*(doc.getFirstChild(),
> *W3C_XML_SCHEMA_INSTANCE_ATTRIBUTE_SCHEMA_LOCATION*,
> *W3C_XML_SCHEMA_INSTANCE_NS_URI*));
>
>     *final* LinkedHashMap<String, CacheElement> elementMap = *new*
> LinkedHashMap<String, CacheElement>();
>
>     *buildElementMapCacheType*(elementMap, *resolveSchema*(
> schemaLocationMap, CacheXml.*NAMESPACE*));
>
>     // if we are ever concerned with the order of extensions or children
> process them here.
>
>     *return* elementMap;
>   }
>
> If you add a license header to CacheElementJUnitTest.xml and run
> CacheElementJUnitTest you will get an NPE.
>
>
> So two questions:
>
> 1) Do all of our test xml files need license headers (it does make the
> exclusion list for RAT simple)?
>

It *reduces* the noise level if the RAT exclusion list is as small as
possible - so where it makes no difference, then I would say yes add them.
But I wouldn't get hung up on every little test file that can easily be
justified under the "contains no creative content" category.

Niall



> 2) Is the code above correct?
>
> Thanks,
> Anthony
>
>

Re: Licenses on XML files

[Jacob Barrett <jb...@pivotal.io>]

The code should be looking for the first element, not first node. Alternately the DOM parser can be configured to ignore comments. 



—

Jacob Barrett 
Manager 
GemFire Advanced Customer Engineering (ACE) 
Pivotal

jbarrett@pivotal.io 
503-533-3763

For immediate support please contact Pivotal Support at http://support.pivotal.io/

On Mon, Dec 7, 2015 at 2:34 PM, Dan Smith <ds...@pivotal.io> wrote:

>> 2) Is the code above correct?
>>
> So, the NPE is because doc.getFirstChild() will return the comment.
> It looks like this code is used within the context of extracting elements
> from an xml file we generate. So with the current geode code it will never
> see a comment, but it might still be worth just fixing the code to be more
> flexible. We do allow xml generating extensions to be registered, so this
> code could potentially cause NPEs if an extension generates xml with a
> comment.
> -Dan

Re: Licenses on XML files

[Dan Smith <ds...@pivotal.io>]

> 2) Is the code above correct?
>

So, the NPE is because doc.getFirstChild() will return the comment.

It looks like this code is used within the context of extracting elements
from an xml file we generate. So with the current geode code it will never
see a comment, but it might still be worth just fixing the code to be more
flexible. We do allow xml generating extensions to be registered, so this
code could potentially cause NPEs if an extension generates xml with a
comment.

-Dan