diff --git a/osxphotos/photoexporter.py b/osxphotos/photoexporter.py index 6df21a8c..0f47b37d 100644 --- a/osxphotos/photoexporter.py +++ b/osxphotos/photoexporter.py @@ -1007,6 +1007,9 @@ class PhotoExporter: elif options.exiftool: sig_exif = export_db.get_stat_exif_for_file(dest_str) cmp_orig = fileutil.cmp_file_sig(dest_str, sig_exif) + if cmp_orig: + # if signatures match also need to compare exifdata to see if metadata changed + cmp_orig = not self._should_run_exiftool(dest_str, options) sig_exif = ( sig_exif[0], sig_exif[1], @@ -1303,27 +1306,7 @@ class PhotoExporter: # determine if we need to write the exif metadata # if we are not updating, we always write # else, need to check the database to determine if we need to write - run_exiftool = not options.update - if options.update: - files_are_different = False - old_data = export_db.get_exifdata_for_file(dest) - if old_data is not None: - old_data = json.loads(old_data)[0] - current_data = json.loads(self._exiftool_json_sidecar(options=options))[ - 0 - ] - if old_data != current_data: - files_are_different = True - - if old_data is None or files_are_different: - # didn't have old data, assume we need to write it - # or files were different - run_exiftool = True - else: - verbose( - f"Skipped up to date exiftool metadata for {pathlib.Path(dest).name}" - ) - + run_exiftool = self._should_run_exiftool(dest, options) if run_exiftool: verbose(f"Writing metadata with exiftool for {pathlib.Path(dest).name}") if not options.dry_run: @@ -1342,8 +1325,32 @@ class PhotoExporter: ) exiftool_results.exif_updated.append(dest) exiftool_results.to_touch.append(dest) + else: + verbose( + f"Skipped up to date exiftool metadata for {pathlib.Path(dest).name}" + ) return exiftool_results + def _should_run_exiftool(self, dest, options: ExportOptions) -> bool: + """Return True if exiftool should be run to update metadata""" + run_exiftool = not options.update + if options.update: + files_are_different = False + old_data = options.export_db.get_exifdata_for_file(dest) + if old_data is not None: + old_data = json.loads(old_data)[0] + current_data = json.loads(self._exiftool_json_sidecar(options=options))[ + 0 + ] + if old_data != current_data: + files_are_different = True + + if old_data is None or files_are_different: + # didn't have old data, assume we need to write it + # or files were different + run_exiftool = True + return run_exiftool + def _write_exif_data(self, filepath: str, options: ExportOptions): """write exif data to image file at filepath diff --git a/tests/test_cli.py b/tests/test_cli.py index 92aa0004..99f4be7c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1971,6 +1971,89 @@ def test_export_exiftool(): assert exif[key] == CLI_EXIFTOOL[uuid][key] +@pytest.mark.skipif(exiftool is None, reason="exiftool not installed") +def test_export_exiftool_template_change(): + """Test --exiftool when template changes with --update, #630""" + import glob + import os + import os.path + + from osxphotos.cli import export + from osxphotos.exiftool import ExifTool + + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + for uuid in CLI_EXIFTOOL: + # export with --exiftool + result = runner.invoke( + export, + [ + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "-V", + "--exiftool", + "--uuid", + f"{uuid}", + ], + ) + assert result.exit_code == 0 + + # export with --update, should be no change + result = runner.invoke( + export, + [ + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "-V", + "--exiftool", + "--update", + "--uuid", + f"{uuid}", + ], + ) + assert result.exit_code == 0 + assert "exported: 0" in result.output + + # export with --update and template change, should export + result = runner.invoke( + export, + [ + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "-V", + "--exiftool", + "--keyword-template", + "FOO", + "--update", + "--uuid", + f"{uuid}", + ], + ) + assert result.exit_code == 0 + assert "updated EXIF data: 1" in result.output + + + # export with --update, nothing should export + result = runner.invoke( + export, + [ + os.path.join(cwd, PHOTOS_DB_15_7), + ".", + "-V", + "--exiftool", + "--keyword-template", + "FOO", + "--update", + "--uuid", + f"{uuid}", + ], + ) + assert result.exit_code == 0 + assert "exported: 0" in result.output + assert "updated EXIF data: 0" in result.output + @pytest.mark.skipif(exiftool is None, reason="exiftool not installed") def test_export_exiftool_path(): """test --exiftool with --exiftool-path""" @@ -4721,7 +4804,7 @@ def test_export_update_basic(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: 0, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, updated EXIF data: 0, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: 0, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, updated EXIF data: 0, missing: 3, error: 0" in result.output ) @@ -4838,7 +4921,7 @@ def test_export_update_exiftool(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 2, error: 1" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 3, error: 1" in result.output ) @@ -4885,7 +4968,7 @@ def test_export_update_hardlink(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: 0, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: 0, missing: 3, error: 0" in result.output ) assert not os.path.samefile(CLI_EXPORT_UUID_FILENAME, photo.path) @@ -4924,7 +5007,7 @@ def test_export_update_hardlink_exiftool(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 2, error: 1" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, skipped: 0, updated EXIF data: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 3, error: 1" in result.output ) assert not os.path.samefile(CLI_EXPORT_UUID_FILENAME, photo.path) @@ -4962,7 +5045,7 @@ def test_export_update_edits(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 1, updated: 1, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7-2}, updated EXIF data: 0, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 1, updated: 1, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7-2}, updated EXIF data: 0, missing: 3, error: 0" in result.output ) @@ -5060,7 +5143,7 @@ def test_export_update_no_db(): # edited files will be re-exported because there won't be an edited signature # in the database assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_EDITED_15_7}, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7}, updated EXIF data: 0, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 0, updated: {PHOTOS_EDITED_15_7}, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7}, updated EXIF data: 0, missing: 3, error: 0" in result.output ) assert os.path.isfile(OSXPHOTOS_EXPORT_DB) @@ -5100,7 +5183,7 @@ def test_export_then_hardlink(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 3, error: 0" in result.output ) assert os.path.samefile(CLI_EXPORT_UUID_FILENAME, photo.path) @@ -5125,7 +5208,7 @@ def test_export_dry_run(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7}, missing: 3, error: 0" in result.output ) for filepath in CLI_EXPORT_FILENAMES_DRY_RUN: @@ -5171,7 +5254,7 @@ def test_export_update_edits_dry_run(): ) assert result.exit_code == 0 assert ( - f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 1, updated: 1, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7-2}, updated EXIF data: 0, missing: 2, error: 0" + f"Processed: {PHOTOS_NOT_IN_TRASH_LEN_15_7} photos, exported: 1, updated: 1, skipped: {PHOTOS_NOT_IN_TRASH_LEN_15_7+PHOTOS_EDITED_15_7-2}, updated EXIF data: 0, missing: 3, error: 0" in result.output )