Add support for OSM JSON format

[mmd-osm]

Following the discussion in openstreetmap/openstreetmap-website#5474 (comment), I'm adding OSM JSON support to this package.

Summary of changes:

1. Moved XML parsing functions getChangesets, getNodes, getWays, getRelations and getTags inside L.OSM.XMLParser
2. Create a similar object attribute L.OSM.JSONParser with the identically named functions to parse JSON
3. buildFeatures decides based on element.version attribute, which parser to use

Unit test changes:

1. Added json fixtures
2. Fixed XML fixtures, replaced <node/> by <node></node> (see commit messages for details)
3. L.OSM.DataLayer unit test is now executed twice, once for XML and once for JSON
4. Bug fix: incorrect feature count in layers(osm).length.should.eq(...); was also fixed

The bug fix was required to keep xml and json unit tests in sync.

For code review I recommend split view + hide spaces in GitHub.

[tomhughes]

I'm not really keen on the structure of this in terms of how it is broken down into commits, especially how we essentially have two fixup commits for the JSON parser.

I'd suggest having one commit to fix the XML tests, then one to introduce the JSON parser in it's final form and then one to add JSON tests (though that could equally be included with the JSON parser addition).

Even better would be to refactor the XML parser along the same lines as the JSON parser after fixing the XML tests and before adding the JSON parser but that's obviously not essential so I wouldn't insist on that.

[tomhughes]

Presumably this is relying on data either being a JSON object (that has a version member) or an XML object (that doesn't) but that seems quite fragile... Could we use instanceof instead to check the type of the data?

[mmd-osm]

Agreed, data.version is quite fragile, and I also don't really like this approach. I need to check for some alternatives.

[mmd-osm]

I didn't have much luck with instanceof, (data instanceof Document) fails in the test environment with ReferenceError: Document is not defined.

typeof data === 'object' didn't work either, because both JSON and XML would match the condition.

(data.constructor.name === "Document") would be another option, also not very nice.

https://developer.mozilla.org/en-US/docs/Web/API/Document/documentElement might be better than checking data.version?

    const parser = (data.documentElement ? L.OSM.XMLParser : L.OSM.JSONParser);

This last option at least passes the unit tests.

jquery isXMLDoc has a more complex logic, which at one point also checks documentElement: https://github.com/jquery/jquery/blob/098591e6fd3222e64b59af92c8849f5d8963d43c/src/core.js#L313-L320

[tomhughes]

Using data instanceof XMLDocument seems to work for me? Both with my suggested test parsing technique and debugging the current code with the data layer on. JSON is tricky because it just returns an Object rather than anything JSON specific...

[mmd-osm]

I'm giving data instanceof XMLDocument a try on GH actions, maybe my local environment has some issue.

[tomhughes]

Ah it works with DOMParser in the browser but not with jsdom which returns something that seems to inherit from Document but which is a plain object?

[tomhughes]

So if we add jsdom-global to the modules then this seems to work:

> require('jsdom-global')()
[Function: cleanup]
> parser = new window.DOMParser();
DOMParser {}
> data = parser.parseFromString("<osm><node/><node/></osm>", "text/xml");
Document { location: [Getter/Setter] }
> data instanceof Document
true

Unlike a true browser it returns Document not XMLDocument but instanceof Document works for both.

[mmd-osm]

Funny, I was just reading the exact same proposal here: https://stackoverflow.com/questions/42649700/using-domparser-in-javascript-testing-with-mocha-and-jsdom

I'll give it a try...

[mmd-osm]

Even though data was created from an XML file, it's not an instance of the same Document that's being used for instanceof:

[mmd-osm]

Ah, there was a missing global.Document = window.Document;

I think it's working now:

[mmd-osm]

Thank you for reviewing the code. I think it's easier to create a new PR to fix the existing XML tests, then continue with the JSON part...

[mmd-osm]

By the way, I rebased this PR on #45, that's why 0ed5313 shows up as commit here as well. Once #45 is merged, I'll do another rebase on master.

[tomhughes]

I think this looks good now, thanks.