diff --git a/lib/spatial_features.rb b/lib/spatial_features.rb index 53d827d3..c6de0594 100644 --- a/lib/spatial_features.rb +++ b/lib/spatial_features.rb @@ -21,6 +21,7 @@ require 'spatial_features/has_spatial_features/feature_import' require 'spatial_features/importers/base' +require 'spatial_features/importers/exif_photo' require 'spatial_features/importers/file' require 'spatial_features/importers/geo_json' require 'spatial_features/importers/esri_geo_json' diff --git a/lib/spatial_features/importers/exif_photo.rb b/lib/spatial_features/importers/exif_photo.rb new file mode 100644 index 00000000..b9ad27cd --- /dev/null +++ b/lib/spatial_features/importers/exif_photo.rb @@ -0,0 +1,86 @@ +require 'exifr/jpeg' +require 'ostruct' +require 'fileutils' + +module SpatialFeatures + module Importers + class ExifPhoto < Base + JPEG_PATTERN = /\.jpe?g\z/i.freeze + NO_PHOTOS = "This archive doesn't contain any JPEG photos.".freeze + UNREADABLE_PHOTO = "This photo couldn't be read. It may be damaged, or saved in a JPEG format we don't support.".freeze + + def self.create_all(data, **options) + tmpdir = options.fetch(:tmpdir) + photos_dir = ::File.join(tmpdir, 'exif_photos') + FileUtils.mkdir_p(photos_dir) + + files = Download.open_each(data, unzip: JPEG_PATTERN, tmpdir: tmpdir) + files.map.with_index do |file, index| + filename = ::File.basename(file.path) + + # Separate directories prevent two photos with the same filename colliding, + # while keeping the original filename for feature names and warnings. + staged_path = ::File.join(photos_dir, index.to_s, filename) + FileUtils.mkdir_p(::File.dirname(staged_path)) + + begin + file.rewind + ::File.open(staged_path, 'wb') do |staged_file| + IO.copy_stream(file, staged_file) + end + ensure + file.close + end + + new(staged_path, **options) + end + rescue Unzip::PathNotFound + raise ImportError, NO_PHOTOS + ensure + Array(files).each {|file| file.close unless file.closed? } + end + + def initialize(data, **options) + options[:source_identifier] ||= ::File.basename(data.to_s) + super(data, **options) + end + + def cache_key + @cache_key ||= Digest::MD5.file(@data).hexdigest + end + + private + + def each_record + photo = EXIFR::JPEG.new(@data) + gps = photo.gps + unless usable_gps?(gps) + @warnings << 'No usable GPS coordinates were found in this photo.' + return + end + + yield OpenStruct.new( + name: ::File.basename(@data), + geog: "POINT(#{gps.longitude} #{gps.latitude})", + metadata: metadata_from(photo, gps), + importable_image_paths: [@data] + ) + rescue EXIFR::MalformedImage + raise ImportError, UNREADABLE_PHOTO + end + + def usable_gps?(gps) + gps && gps.latitude.is_a?(Numeric) && gps.longitude.is_a?(Numeric) && + (-90..90).cover?(gps.latitude) && (-180..180).cover?(gps.longitude) + end + + def metadata_from(photo, gps) + { + 'capture_time' => photo.date_time_original&.strftime('%Y-%m-%d %H:%M:%S'), + 'altitude' => gps.altitude&.to_s, + 'camera_model' => photo.model.presence + }.compact + end + end + end +end diff --git a/spatial_features.gemspec b/spatial_features.gemspec index e234fa26..a50237d1 100644 --- a/spatial_features.gemspec +++ b/spatial_features.gemspec @@ -24,6 +24,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency "rubyzip", "~> 3.0" s.add_runtime_dependency "nokogiri" s.add_runtime_dependency "ostruct" + s.add_runtime_dependency "exifr" s.add_development_dependency "rails", '>= 7', '< 9' s.add_development_dependency "pg", '~> 1' diff --git a/spec/fixtures/bc25_bt_0030.JPG b/spec/fixtures/bc25_bt_0030.JPG new file mode 100644 index 00000000..f0539861 Binary files /dev/null and b/spec/fixtures/bc25_bt_0030.JPG differ diff --git a/spec/fixtures/sample_photos.zip b/spec/fixtures/sample_photos.zip new file mode 100644 index 00000000..adf82ae4 Binary files /dev/null and b/spec/fixtures/sample_photos.zip differ diff --git a/spec/lib/spatial_features/has_spatial_features/feature_import_spec.rb b/spec/lib/spatial_features/has_spatial_features/feature_import_spec.rb index 867f0071..696c1201 100644 --- a/spec/lib/spatial_features/has_spatial_features/feature_import_spec.rb +++ b/spec/lib/spatial_features/has_spatial_features/feature_import_spec.rb @@ -553,6 +553,10 @@ class ImageHandlerMock def self.call(feature, images); end end + class ExifImageHandlerMock + def self.call(feature, images); end + end + subject do new_dummy_class(:parent => FeatureImportMock) do has_spatial_features :import => { :test_kml => :KMLFile }, :image_handlers => [:ImageHandlerMock] @@ -570,6 +574,33 @@ def test_kml expect(ImageHandlerMock).to have_received(:call).with(Feature, [Pathname, Pathname]).once end + it 'keeps staged EXIF photos available through image handling, then cleans them up' do + source_photo = fixture_file_path('bc25_bt_0030.JPG') + tmpdir = Dir.mktmpdir + handled_paths = [] + handled_bytes = [] + photo_subject = new_dummy_class(:parent => FeatureImportMock) do + has_spatial_features :import => { :test_photo => :ExifPhoto }, :image_handlers => [:ExifImageHandlerMock] + + define_method(:test_photo) { source_photo } + end.create + allow(ExifImageHandlerMock).to receive(:call) do |_feature, images| + handled_paths.concat(images) + handled_bytes.concat(images.map {|path| ::File.binread(path) }) + end + + photo_subject.update_features!(:tmpdir => tmpdir) + + expect(photo_subject.features.count).to eq(1) + expect(ExifImageHandlerMock).to have_received(:call).once + expect(handled_paths).to all(start_with(tmpdir)) + expect(handled_bytes).to eq([::File.binread(source_photo)]) + expect(handled_paths).to all(satisfy {|path| !::File.exist?(path) }) + expect(::Dir.exist?(tmpdir)).to be(false) + ensure + FileUtils.remove_entry(tmpdir) if tmpdir && Dir.exist?(tmpdir) + end + let(:keys_to_remove) { SpatialFeatures::Importers::KML::IMAGE_METADATA_KEYS } it 'removes image metadata keys from persisted feature metadata' do subject.update_features! diff --git a/spec/lib/spatial_features/importers/exif_photo_spec.rb b/spec/lib/spatial_features/importers/exif_photo_spec.rb new file mode 100644 index 00000000..06825d62 --- /dev/null +++ b/spec/lib/spatial_features/importers/exif_photo_spec.rb @@ -0,0 +1,240 @@ +require 'spec_helper' + +describe SpatialFeatures::Importers::ExifPhoto do + let(:photo_path) { fixture_file_path('bc25_bt_0030.JPG') } + + subject(:importer) { described_class.new(photo_path) } + + let(:features) { importer.features } + + describe '#features' do + it 'imports one feature from a geotagged photo' do + expect(features.count).to eq(1) + end + + it 'places the feature at the EXIF GPS coordinates' do + expect(features.first.geog).to eq( + 'POINT(-125.12249 50.36145)' + ) + end + + it 'includes EXIF metadata' do + expect(features.first.metadata).to eq( + 'capture_time' => '2025-08-08 16:48:16', + 'altitude' => '109.4', + 'camera_model' => 'NIKON D7500' + ) + end + + it 'makes the photo available for attachment importing' do + expect(features.first.importable_image_paths).to eq([photo_path]) + end + end + + describe '#cache_key' do + it 'is based on the photo contents rather than its path' do + expect(importer.cache_key).to eq(Digest::MD5.file(photo_path).hexdigest) + end + end + + context 'when the photo has no GPS coordinates' do + before do + allow(EXIFR::JPEG).to receive(:new).with(photo_path).and_return(double(gps: nil)) + end + + it 'does not import a feature' do + expect(features).to be_empty + end + + it 'records a warning that identifies the problem' do + features + + expect(importer.warnings).to include(a_string_matching(/GPS coordinates/i)) + end + + it 'identifies the source photo by filename' do + expect(importer.source_identifier).to eq('bc25_bt_0030.JPG') + end + end + + context 'when optional EXIF metadata is absent' do + let(:gps) { double(latitude: 50.36145, longitude: -125.12249, altitude: nil) } + let(:photo) { double(gps: gps, date_time_original: nil, model: nil) } + + before do + allow(EXIFR::JPEG).to receive(:new).with(photo_path).and_return(photo) + end + + it 'imports the point without blank metadata values' do + expect(features.first.metadata).to eq({}) + end + end + + context 'when the JPEG is malformed' do + before do + allow(EXIFR::JPEG).to receive(:new).with(photo_path).and_raise(EXIFR::MalformedJPEG) + end + + it 'raises an import error with a useful message' do + expect { features } + .to raise_error(SpatialFeatures::ImportError, /photo couldn't be read/i) + end + end + + describe '.create_all' do + let(:tmpdir) { Dir.mktmpdir } + + after do + FileUtils.remove_entry(tmpdir) if Dir.exist?(tmpdir) + end + + context 'with an individual JPEG' do + subject(:created_importer) do + described_class.create_all(photo_path, tmpdir: tmpdir).first + end + + it 'creates one importer from a staged copy' do + staged_path = created_importer.features.first.importable_image_paths.first + + expect(created_importer.features.count).to eq(1) + expect(staged_path).to eq(::File.join(tmpdir, 'exif_photos', '0', 'bc25_bt_0030.JPG')) + expect(staged_path).not_to eq(photo_path) + expect(::File.binread(staged_path)).to eq(::File.binread(photo_path)) + end + + it 'keeps the original filename for the feature and source identifier' do + expect(created_importer.source_identifier).to eq('bc25_bt_0030.JPG') + expect(created_importer.features.first.name).to eq('bc25_bt_0030.JPG') + end + end + + context 'when the source path is unlinked while its file is still open' do + it 'stages the JPEG from the open file descriptor and closes it' do + owner = Tempfile.new(['remote-photo', '.JPG']) + owner.binmode + owner.write(::File.binread(photo_path)) + owner.flush + open_file = ::File.open(owner.path, 'rb') + owner.close! + + expect(::File.exist?(open_file.path)).to be(false) + allow(SpatialFeatures::Download).to receive(:open_each).and_return([open_file]) + + created_importer = described_class.create_all('https://example.test/photo.JPG', tmpdir: tmpdir).first + staged_path = created_importer.features.first.importable_image_paths.first + + expect(open_file).to be_closed + expect(created_importer.cache_key).to eq(Digest::MD5.file(photo_path).hexdigest) + expect(created_importer.features.count).to eq(1) + expect(::File.binread(staged_path)).to eq(::File.binread(photo_path)) + ensure + open_file&.close unless open_file&.closed? + owner&.close! + end + end + + context 'when staging the photo fails' do + it 'still closes every source file' do + source_files = Array.new(2) { ::File.open(photo_path, 'rb') } + allow(SpatialFeatures::Download).to receive(:open_each).and_return(source_files) + allow(IO).to receive(:copy_stream).and_raise(IOError, 'copy failed') + + expect do + described_class.create_all(photo_path, tmpdir: tmpdir) + end.to raise_error(IOError, 'copy failed') + expect(source_files).to all(be_closed) + ensure + source_files&.each {|file| file.close unless file.closed? } + end + end + + context 'with an in-memory remote JPEG' do + it 'remains available after the download temporary object is collected' do + bytes = ::File.binread(photo_path) + allow(URI).to receive(:open).and_return(StringIO.new(bytes)) + + created_importer = described_class.create_all('https://example.test/photo.JPG', tmpdir: tmpdir).first + GC.start + staged_path = created_importer.features.first.importable_image_paths.first + + expect(created_importer.cache_key).to eq(Digest::MD5.hexdigest(bytes)) + expect(created_importer.features.count).to eq(1) + expect(::File.file?(staged_path)).to be(true) + expect(::File.binread(staged_path)).to eq(bytes) + end + end + + context 'with a ZIP of JPEGs' do + subject(:importers) do + described_class.create_all(fixture_file_path('sample_photos.zip'), tmpdir: tmpdir) + end + + it 'creates one importer per photo' do + expect(importers.count).to eq(5) + end + + it 'imports one distinct point per photo' do + features = importers.flat_map(&:features) + + expect(features.count).to eq(5) + expect(features.map(&:geog).uniq.count).to eq(5) + end + + it 'preserves each photo filename' do + expect(importers.map(&:source_identifier)).to contain_exactly( + 'bc25_bt_0030.JPG', + 'bc25_bt_0031.JPG', + 'bc25_bt_0032.JPG', + 'bc25_bt_0033.JPG', + 'bc25_bt_0034.JPG' + ) + end + + it 'keeps every extracted photo available to image handlers' do + image_paths = importers.flat_map(&:features).flat_map(&:importable_image_paths) + + expect(image_paths.count).to eq(5) + expect(image_paths.all? {|path| ::File.file?(path) }).to be(true) + end + + it 'stages every photo below the managed temporary directory' do + image_paths = importers.flat_map(&:features).flat_map(&:importable_image_paths) + + expect(image_paths).to all(start_with("#{tmpdir}/exif_photos/")) + end + end + + context 'with duplicate filenames in different ZIP directories' do + let(:archive_path) do + ::File.join(tmpdir, 'duplicate_names.zip').tap do |path| + bytes = ::File.binread(photo_path) + Zip::OutputStream.open(path) do |zip| + zip.put_next_entry('first/repeated.JPG') + zip.write(bytes) + zip.put_next_entry('second/repeated.JPG') + zip.write(bytes) + end + end + end + + it 'keeps both photos without changing their displayed filename' do + importers = described_class.create_all(archive_path, tmpdir: tmpdir) + image_paths = importers.flat_map(&:features).flat_map(&:importable_image_paths) + + expect(importers.map(&:source_identifier)).to eq(['repeated.JPG', 'repeated.JPG']) + expect(importers.flat_map(&:features).map(&:name)).to eq(['repeated.JPG', 'repeated.JPG']) + expect(image_paths.map {|path| ::File.basename(path) }).to eq(['repeated.JPG', 'repeated.JPG']) + expect(image_paths.uniq.count).to eq(2) + expect(image_paths.map {|path| ::File.binread(path) }).to all(eq(::File.binread(photo_path))) + end + end + + context 'with a ZIP containing no JPEGs' do + it 'reports that the archive has no photos' do + expect do + described_class.create_all(fixture_file_path('archive_without_any_known_file.zip'), tmpdir: tmpdir) + end.to raise_error(SpatialFeatures::ImportError, /JPEG photos/i) + end + end + end +end