diff --git a/lib/spatial_features/importers/kml.rb b/lib/spatial_features/importers/kml.rb index 8bd0e9df..5b29ea6d 100644 --- a/lib/spatial_features/importers/kml.rb +++ b/lib/spatial_features/importers/kml.rb @@ -6,6 +6,20 @@ class KML < Base # keys that may contain tags IMAGE_METADATA_KEYS = %w[pdfmaps_photos].freeze + # The elements that become features. A Placemark holds them directly or inside a + # MultiGeometry, and each one becomes a feature of its own. + GEOMETRY_TYPES = %w[Polygon LineString Point].freeze + GEOMETRY_SELECTOR = GEOMETRY_TYPES.join(', ').freeze + + # Matches what `GEOMETRY_SELECTOR` matches, for the path that scopes elements to one + # Placemark by counting how many Placemarks they sit inside. + GEOMETRY_SELF_TEST = GEOMETRY_TYPES.map {|type| "self::#{type}" }.join(' or ').freeze + + # Geometry that sits outside any Placemark, which imports with no name and no + # metadata. One pass over the document matches all of it. + UNPLACED_GEOMETRY_XPATH = + GEOMETRY_TYPES.map {|type| "//#{type}[not(ancestor::Placemark)]" }.join(' | ').freeze + # matches a coordinate pair with an optional altitude, including invalid altitudes like NaN # -118.1,50.9,NaN # -118.1,50.9,0 @@ -20,25 +34,62 @@ def initialize(data, base_dir: nil, **options) private def each_record(&block) - {'Polygon' => 'POLYGON', 'LineString' => 'LINE', 'Point' => 'POINT'}.each do |kml_type, sql_type| - kml_document.css(kml_type).each do |feature| - if (placemark = feature.ancestors('Placemark').first) - metadata = extract_metadata(placemark) - name = placemark.css('name').text - else - metadata = {} - end + kml_document.css('Placemark').each do |placemark| + metadata = extract_metadata(placemark) + importable_image_paths = images_from_metadata(metadata) + name = placemark.css('name').text + + geometries_in(placemark).each do |geometry| + # A hash of its own per feature, since each is stored on a separate record. + yield_feature(geometry, name, metadata.dup, importable_image_paths, &block) + end + end + + kml_document.xpath(UNPLACED_GEOMETRY_XPATH).each do |geometry| + yield_feature(geometry, nil, {}, [], &block) + end + end - next if blank_feature?(feature) + # Returns the geometry elements belonging to a Placemark. + # + # A Placemark inside another Placemark owns its own geometry, so the outer one has to + # leave it alone or the same element is read twice. Scoping by depth costs an upward + # walk per element, which is 7x the plain selector on a document holding six figures + # of geometry, so it is used only for a document that nests. + # + # @param placemark [Nokogiri::XML::Element] + # @return [Nokogiri::XML::NodeSet] the Polygon, LineString and Point elements whose + # nearest enclosing Placemark is this one. + def geometries_in(placemark) + return placemark.css(GEOMETRY_SELECTOR) unless nested_placemarks? + + depth = placemark.xpath('count(ancestor::Placemark)').to_i + 1 + placemark.xpath(".//*[#{GEOMETRY_SELF_TEST}][count(ancestor::Placemark) = #{depth}]") + end - geog = geom_from_kml(feature) - next if geog.blank? + # Returns true when any Placemark in the document holds another, which KML 2.2 does + # not allow. Read once per document. + def nested_placemarks? + return @nested_placemarks if defined?(@nested_placemarks) - importable_image_paths = images_from_metadata(metadata) + @nested_placemarks = kml_document.at_xpath('//Placemark//Placemark').present? + end - yield OpenStruct.new(geog: geog, name: name, metadata: metadata, importable_image_paths: importable_image_paths) - end - end + # Yields the feature built from a geometry element. + # + # @param geometry [Nokogiri::XML::Element] a Polygon, LineString or Point node. + # @param metadata [Hash] stored on the feature as it stands, so it must already have + # had its image keys removed. + # @yield [OpenStruct] nothing is yielded when the element holds no coordinates, or + # when PostGIS cannot read it. + def yield_feature(geometry, name, metadata, importable_image_paths, &block) + return if blank_feature?(geometry) + + geog = geom_from_kml(geometry) + return if geog.blank? + + block.call OpenStruct.new(geog: geog, name: name, metadata: metadata, + importable_image_paths: importable_image_paths) end def kml_document diff --git a/spec/fixtures/README.md b/spec/fixtures/README.md index a6d153dc..33624f97 100644 --- a/spec/fixtures/README.md +++ b/spec/fixtures/README.md @@ -17,9 +17,11 @@ The geometry is deliberately dull: squares on a regular grid, in NAD83 / UTM zon shapefiles and in round degrees for the KML. What matters in each fixture is its structure rather than its coordinates — a missing `.shx`, a `.shx` whose basename does not match the `.shp`, a `__MACOSX` decoy carrying a real extension, a single-vertex LineString, a null -geometry, a name longer than the name column. The specs assert on feature counts and on those -structures, so regenerating with different coordinates is safe, while changing a count, a -component filename, or a placemark name is not. +geometry, a name longer than the name column, a Placemark holding many polygons in one +`MultiGeometry`, a Placemark holding another Placemark. The specs assert on feature counts +and on those structures, so regenerating +with different coordinates is safe, while changing a count, a component filename, or a +placemark name is not. Regenerating rewrites the archives in place, and their entry timestamps change every run, so expect git to report them modified even when nothing about their contents has. Run the suite diff --git a/spec/fixtures/kml_file_with_multi_geometry_photos.kml b/spec/fixtures/kml_file_with_multi_geometry_photos.kml new file mode 100644 index 00000000..6029bdf1 --- /dev/null +++ b/spec/fixtures/kml_file_with_multi_geometry_photos.kml @@ -0,0 +1,21 @@ + + + + kml_file_with_multi_geometry_photos.kml + + Multi geometry photo folder + + Photo Poly + This is a description + + <img src="images/two_a.jpg"><img src="images/two_b.jpg"> + + + 50,60 50.5,60 50.5,60.5 50,60.5 50,60 + 51,60 51.5,60 51.5,60.5 51,60.5 51,60 + 52,60 52.5,60 52.5,60.5 52,60.5 52,60 + + + + + diff --git a/spec/fixtures/kml_file_with_multi_geometry_placemarks.kml b/spec/fixtures/kml_file_with_multi_geometry_placemarks.kml new file mode 100644 index 00000000..a5591abf --- /dev/null +++ b/spec/fixtures/kml_file_with_multi_geometry_placemarks.kml @@ -0,0 +1,29 @@ + + + + kml_file_with_multi_geometry_placemarks.kml + + Multi geometry folder + + Poly 1 + This is a description + + 10,20 10.5,20 10.5,20.5 10,20.5 10,20 + 11,20 11.5,20 11.5,20.5 11,20.5 11,20 + 12,20 12.5,20 12.5,20.5 12,20.5 12,20 + 13,20 13.5,20 13.5,20.5 13,20.5 13,20 + + + + Poly 2 + This is a description also + + 30,40 30.5,40 30.5,40.5 30,40.5 30,40 + 31,40 31.5,40 31.5,40.5 31,40.5 31,40 + 32,40 32.5,40 32.5,40.5 32,40.5 32,40 + 33,40 33.5,40 33.5,40.5 33,40.5 33,40 + + + + + diff --git a/spec/fixtures/kml_file_with_nested_placemarks.kml b/spec/fixtures/kml_file_with_nested_placemarks.kml new file mode 100644 index 00000000..811503b8 --- /dev/null +++ b/spec/fixtures/kml_file_with_nested_placemarks.kml @@ -0,0 +1,19 @@ + + + + kml_file_with_nested_placemarks.kml + + Nested placemark folder + + Outer Poly + This is a description + 10,20 11,20 11,21 10,21 10,20 + + Inner Poly + The inner description + 30,40 31,40 31,41 30,41 30,40 + + + + + diff --git a/spec/lib/spatial_features/importers/kml_spec.rb b/spec/lib/spatial_features/importers/kml_spec.rb index 3b2aee8b..ea7dd7eb 100644 --- a/spec/lib/spatial_features/importers/kml_spec.rb +++ b/spec/lib/spatial_features/importers/kml_spec.rb @@ -103,6 +103,79 @@ end end + context 'when a Placemark holds a MultiGeometry' do + let(:data) { kml_file_with_multi_geometry_placemarks.read } + + describe '#features' do + it 'returns one feature per part' do + expect(subject.features.count).to eq(8) + end + + it 'names each part after the Placemark holding it' do + expect(subject.features.map(&:name)) + .to contain_exactly(*['Poly 1'] * 4, *['Poly 2'] * 4) + end + + it 'gives each part the metadata of the Placemark holding it' do + expect(subject.features) + .to all(have_attributes :metadata => include('description' => be_present)) + end + + # Reading a part's Placemark by searching the document, or by asking the part for its + # ancestors, costs the document's size per part. The document is parsed first, since + # parsing searches it for NetworkLinks and overlays. + it 'reads the parts without searching the document again' do + subject.send(:kml_document) + + expect_any_instance_of(Nokogiri::XML::Document).not_to receive(:search) + + subject.features + end + + it 'reads a Placemark\'s metadata once however many parts it holds' do + expect(subject).to receive(:extract_metadata).twice.and_call_original + + subject.features + end + end + end + + context 'when a Placemark holding a MultiGeometry names photos' do + subject { SpatialFeatures::Importers::KML.new(data, :base_dir => Pathname.new('/base')) } + let(:data) { kml_file_with_multi_geometry_photos.read } + + describe '#features' do + it 'returns one feature per part' do + expect(subject.features.count).to eq(3) + end + + it 'gives every part the photos the Placemark names' do + expect(subject.features).to all(have_attributes :importable_image_paths => + [Pathname.new('/base/images/two_a.jpg'), Pathname.new('/base/images/two_b.jpg')]) + end + + it 'leaves the image keys out of every part\'s metadata' do + keys = subject.features.flat_map {|feature| feature.metadata.keys } + + expect(keys & SpatialFeatures::Importers::KML::IMAGE_METADATA_KEYS).to eq([]) + end + end + end + + context 'when a Placemark holds another Placemark' do + let(:data) { kml_file_with_nested_placemarks.read } + + describe '#features' do + it 'returns one feature per geometry rather than reading the inner one twice' do + expect(subject.features.count).to eq(2) + end + + it 'returns each geometry once' do + expect(subject.features.map(&:geog).uniq.size).to eq(2) + end + end + end + context 'when the input is xml but not kml' do let(:data) { "hi" } diff --git a/spec/support/fixtures.rb b/spec/support/fixtures.rb index a127ca69..054d548a 100644 --- a/spec/support/fixtures.rb +++ b/spec/support/fixtures.rb @@ -22,6 +22,10 @@ def kml_file_with_invalid_placemark open_fixture_file("kml_file_with_invalid_placemark.kml") end +def kml_file_with_nested_placemarks + open_fixture_file("kml_file_with_nested_placemarks.kml") +end + def kml_file_with_network_link open_fixture_file("kml_file_with_network_link.kml") end @@ -38,6 +42,14 @@ def kml_file_with_invalid_altitude open_fixture_file("kml_file_with_invalid_altitude.kml") end +def kml_file_with_multi_geometry_photos + open_fixture_file("kml_file_with_multi_geometry_photos.kml") +end + +def kml_file_with_multi_geometry_placemarks + open_fixture_file("kml_file_with_multi_geometry_placemarks.kml") +end + def kml_file_without_features open_fixture_file("kml_file_without_features.kml") end diff --git a/tasks/fixtures.rake b/tasks/fixtures.rake index 73d6857e..337e0ee2 100644 --- a/tasks/fixtures.rake +++ b/tasks/fixtures.rake @@ -171,6 +171,23 @@ module FixtureGenerator write(::File.join(dir, 'kml_file_with_ground_overlay_and_features.kml'), document('kml_file_with_ground_overlay_and_features.kml', overlay_folder + poly_folder)) + # Two Placemarks, each holding several polygons in one MultiGeometry, the shape design + # software exports when it models an object out of many faces. Each polygon imports as + # its own feature, taking the name and metadata of the Placemark holding it. + write(::File.join(dir, 'kml_file_with_multi_geometry_placemarks.kml'), + document('kml_file_with_multi_geometry_placemarks.kml', multi_geometry_folder)) + + # The same shape carrying photos, so that the images a Placemark names reach every one + # of its parts rather than only the first. + write(::File.join(dir, 'kml_file_with_multi_geometry_photos.kml'), + document('kml_file_with_multi_geometry_photos.kml', multi_geometry_photo_folder)) + + # A Placemark holding another, which KML 2.2 does not allow. The inner geometry belongs + # to the inner Placemark, so each polygon imports once rather than the outer Placemark + # also claiming the inner one. + write(::File.join(dir, 'kml_file_with_nested_placemarks.kml'), + document('kml_file_with_nested_placemarks.kml', nested_placemark_folder)) + # Geometry with no Placemark ancestor, so it imports with no name and no metadata. write_kmz(::File.join(dir, 'kmz_file_features_without_placemarks.kmz'), document('Geometry Without Placemarks', @@ -278,6 +295,37 @@ module FixtureGenerator placemark('Poly 2', 'This is a description also', polygon(square(30, 40)))) end + # The two placemarks of `poly_folder`, each holding `parts` polygons rather than one. + def multi_geometry_folder(parts: 4) + placemarks = [['Poly 1', 'This is a description', 10, 20], + ['Poly 2', 'This is a description also', 30, 40]] + + folder('Multi geometry folder', placemarks.map { |name, description, lon, lat| + placemark(name, description, multi_geometry(parts) {|i| polygon(square(lon + i, lat, :size => 0.5)) }) + }.join) + end + + # A single placemark whose ExtendedData names two photos, holding `parts` polygons, so + # every part of it carries the same images. + def multi_geometry_photo_folder(parts: 3) + folder('Multi geometry photo folder', + placemark('Photo Poly', 'This is a description', + photos('two_a.jpg', 'two_b.jpg') + + multi_geometry(parts) {|i| polygon(square(50 + i, 60, :size => 0.5)) })) + end + + def multi_geometry(parts, &block) + " \n" + Array.new(parts, &block).join + " \n" + end + + # One Placemark holding a polygon and, beside it, a second Placemark of its own. + def nested_placemark_folder + inner = placemark('Inner Poly', 'The inner description', polygon(square(30, 40))) + + folder('Nested placemark folder', + placemark('Outer Poly', 'This is a description', polygon(square(10, 20)) + inner)) + end + def overlay_folder folder('Overlay folder', " \n Basemap Overlay\n" \