From 0e1a8c7f61b80a93e1fa4be015d80edda533e187 Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Sun, 21 Apr 2024 09:52:55 -0700 Subject: [PATCH] Fix update aae 1526 (#1530) * Initial implementation of #1526 * Fixed tests * Fixed history for AAE files * Updated report writer * Updated verbose verbiage --- osxphotos/cli/report_writer.py | 31 +++++++- osxphotos/exportoptions.py | 4 + osxphotos/photoexporter.py | 130 +++++++++++++++++++++++++-------- tests/test_cli.py | 46 +++++++++++- 4 files changed, 175 insertions(+), 36 deletions(-) diff --git a/osxphotos/cli/report_writer.py b/osxphotos/cli/report_writer.py index aa2c0207..66039f28 100644 --- a/osxphotos/cli/report_writer.py +++ b/osxphotos/cli/report_writer.py @@ -109,6 +109,8 @@ def __init__( "user_written", "user_skipped", "user_error", + "aae_written", + "aae_skipped", ] mode = "a" if append else "w" @@ -208,9 +210,9 @@ def write(self, export_results: ExportResults): cursor = self._conn.cursor() cursor.execute( "INSERT INTO report " - "(datetime, filename, exported, new, updated, skipped, exif_updated, touched, converted_to_jpeg, sidecar_xmp, sidecar_json, sidecar_exiftool, missing, error, exiftool_warning, exiftool_error, extended_attributes_written, extended_attributes_skipped, cleanup_deleted_file, cleanup_deleted_directory, exported_album, report_id, sidecar_user, sidecar_user_error, user_written, user_skipped, user_error) " # noqa + "(datetime, filename, exported, new, updated, skipped, exif_updated, touched, converted_to_jpeg, sidecar_xmp, sidecar_json, sidecar_exiftool, missing, error, exiftool_warning, exiftool_error, extended_attributes_written, extended_attributes_skipped, cleanup_deleted_file, cleanup_deleted_directory, exported_album, report_id, sidecar_user, sidecar_user_error, user_written, user_skipped, user_error, aae_written, aae_skipped) " # noqa "VALUES " - "(:datetime, :filename, :exported, :new, :updated, :skipped, :exif_updated, :touched, :converted_to_jpeg, :sidecar_xmp, :sidecar_json, :sidecar_exiftool, :missing, :error, :exiftool_warning, :exiftool_error, :extended_attributes_written, :extended_attributes_skipped, :cleanup_deleted_file, :cleanup_deleted_directory, :exported_album, :report_id, :sidecar_user, :sidecar_user_error, :user_written, :user_skipped, :user_error);", # noqa + "(:datetime, :filename, :exported, :new, :updated, :skipped, :exif_updated, :touched, :converted_to_jpeg, :sidecar_xmp, :sidecar_json, :sidecar_exiftool, :missing, :error, :exiftool_warning, :exiftool_error, :extended_attributes_written, :extended_attributes_skipped, :cleanup_deleted_file, :cleanup_deleted_directory, :exported_album, :report_id, :sidecar_user, :sidecar_user_error, :user_written, :user_skipped, :user_error, :aae_written, :aae_skipped);", # noqa data, ) self._conn.commit() @@ -306,6 +308,19 @@ def _create_tables(self): ) self._conn.commit() + # migrate report table for aae_written and aae_skipped + if "aae_written" not in sqlite_columns(self._conn, "report"): + self._conn.cursor().execute( + "ALTER TABLE report ADD COLUMN aae_written TEXT;" + ) + self._conn.commit() + + if "aae_skipped" not in sqlite_columns(self._conn, "report"): + self._conn.cursor().execute( + "ALTER TABLE report ADD COLUMN aae_skipped TEXT;" + ) + self._conn.commit() + # create report_summary view c.execute( """ @@ -396,6 +411,8 @@ def prepare_export_results_for_writing( "user_written": false, "user_skipped": false, "user_error": "", + "aae_written": false, + "aae_skipped": false, } for result in export_results.exported: @@ -492,6 +509,12 @@ def prepare_export_results_for_writing( for result in export_results.user_error: all_results[str(result[0])]["user_error"] = result[1] + for result in export_results.aae_written: + all_results[str(result)]["aae_written"] = true + + for result in export_results.aae_skipped: + all_results[str(result)]["aae_skipped"] = true + return all_results @@ -644,7 +667,7 @@ def _create_tables(self): c.execute( """ CREATE TABLE IF NOT EXISTS report ( - report_id TEXT, + report_id TEXT, uuid TEXT, filename TEXT, fingerprint TEXT, @@ -1002,7 +1025,7 @@ def _create_tables(self): c.execute( """ CREATE TABLE IF NOT EXISTS report ( - report_id TEXT, + report_id TEXT, uuid TEXT, original_filename TEXT, filename TEXT, diff --git a/osxphotos/exportoptions.py b/osxphotos/exportoptions.py index edd12df6..94cb94da 100644 --- a/osxphotos/exportoptions.py +++ b/osxphotos/exportoptions.py @@ -150,6 +150,7 @@ class ExportResults: missing_album: list of tuples of (file, album) for any files that were missing from an album new: list of files that were new aae_written: list of files where .AAE file was written + aae_skipped: list of files where .AAE file was written sidecar_exiftool_skipped: list of files where exiftool sidecar was skipped sidecar_exiftool_written: list of files where exiftool sidecar was written sidecar_json_skipped: list of files where json sidecar was skipped @@ -195,6 +196,7 @@ class ExportResults: "missing_album", "new", "aae_written", + "aae_skipped", "sidecar_exiftool_skipped", "sidecar_exiftool_written", "sidecar_json_skipped", @@ -232,6 +234,7 @@ def __init__( missing_album: list[tuple[str, str]] | None = None, new: list[str] | None = None, aae_written: list[str] | None = None, + aae_skipped: list[str] | None = None, sidecar_exiftool_skipped: list[str] | None = None, sidecar_exiftool_written: list[str] | None = None, sidecar_json_skipped: list[str] | None = None, @@ -282,6 +285,7 @@ def all_files(self) -> list[str]: + self.touched + self.converted_to_jpeg + self.aae_written + + self.aae_skipped + self.sidecar_json_written + self.sidecar_json_skipped + self.sidecar_exiftool_written diff --git a/osxphotos/photoexporter.py b/osxphotos/photoexporter.py index 96afefc3..4b7a9750 100644 --- a/osxphotos/photoexporter.py +++ b/osxphotos/photoexporter.py @@ -92,6 +92,7 @@ def __init__( edited_live: t.Optional[str] = None, preview: t.Optional[str] = None, raw: t.Optional[str] = None, + aae: t.Optional[str] = None, error: t.Optional[t.List[str]] = None, ): self.original = original @@ -100,6 +101,7 @@ def __init__( self.edited_live = edited_live self.preview = preview self.raw = raw + self.aae = aae self.error = error or [] # TODO: bursts? @@ -354,8 +356,23 @@ def export( f"Skipping missing preview photo for {self._filename(self.photo.original_filename)} ({self._uuid(self.photo.uuid)})" ) - if options.export_aae: - all_results += self._write_aae_file(dest=dest, options=options) + if export_original and self.photo.hasadjustments and options.export_aae: + # export associated AAE adjustments file if requested but only for original images + # AAE applies changes to the original so is not meaningful for the edited image + aae_name = normalize_fs_path(dest.with_suffix(".AAE")) + if staged_files.aae: + aae_path = pathlib.Path(staged_files.aae) + all_results += self._export_aae( + aae_path, + aae_name, + options=options, + ) + else: + verbose( + f"Skipping adjustments for {self._filename(self.photo.original_filename)}: no AAE adjustments file" + ) + all_results += ExportResults(missing=[aae_name]) + sidecar_writer = SidecarWriter(self.photo) all_results += sidecar_writer.write_sidecar_files(dest=dest, options=options) @@ -364,7 +381,8 @@ def export( # the history record is written in _export_photo # but this isn't called for missing photos for filename in all_results.missing: - options.export_db.set_history(filename, self.photo.uuid, "missing", None) + action = "AAE: missing" if str(filename).endswith(".AAE") else "missing" + options.export_db.set_history(filename, self.photo.uuid, action, None) return all_results @@ -590,6 +608,8 @@ def _stage_photos_for_export(self, options: ExportOptions) -> StagedFiles: staged.original = self.photo.path if options.live_photo and self.photo.live_photo: staged.original_live = self.photo.path_live_photo + if options.export_aae: + staged.aae = self.photo.adjustments_path if options.edited: # edited file @@ -1181,12 +1201,13 @@ def _export_photo_uuid_applescript( exported_paths.append(str(dest_new)) return exported_paths - def _write_aae_file( + def _export_aae( self, + src: pathlib.Path, dest: pathlib.Path, options: ExportOptions, ) -> ExportResults: - """Write AAE file for the photo.""" + """Export AAE file for the photo.""" # AAE files describe adjustments to originals, so they don't make sense # for edited files @@ -1195,42 +1216,89 @@ def _write_aae_file( verbose = options.verbose or self._verbose - aae_src = self.photo.adjustments_path - if aae_src is None: - verbose( - f"Skipping adjustments for {self._filename(self.photo.original_filename)}: no AAE adjustments file" - ) - return ExportResults() - aae_dest = normalize_fs_path(dest.with_suffix(".AAE")) + action = None + + if options.update or options.force_update: # updating + if dest.exists(): + if update_reason := self._should_update_photo(src, dest, options): + action = "update: " + update_reason.name + else: + # update_skipped_files.append(dest_str) + action = "skip" + else: + action = "new" + else: + action = "export" + + if action == "skip": + if dest.exists(): + options.export_db.set_history( + filename=str(dest), uuid=self.photo.uuid, action=f"AAE: {action}", diff=None + ) + return ExportResults(aae_skipped=[str(dest)], skipped=[str(dest)]) + else: + action = "export" + + errors = [] + if dest.exists() and any( + [options.overwrite, options.update, options.force_update] + ): + try: + options.fileutil.unlink(dest) + except Exception as e: + errors.append(f"Error removing file {dest}: {e} (({lineno(__file__)})") if options.export_as_hardlink: try: - if aae_dest.exists() and any( - [options.overwrite, options.update, options.force_update] - ): - try: - options.fileutil.unlink(aae_dest) - except Exception as e: - raise ExportError( - f"Error removing file {aae_dest}: {e} (({lineno(__file__)})" - ) from e - options.fileutil.hardlink(aae_src, aae_dest) + options.fileutil.hardlink(src, dest) except Exception as e: - raise ExportError( - f"Error hardlinking {aae_src} to {aae_dest}: {e} ({lineno(__file__)})" - ) from e + errors.append( + f"Error hardlinking {src} to {dest}: {e} ({lineno(__file__)})" + ) else: try: - options.fileutil.copy(aae_src, aae_dest) + options.fileutil.copy(src, dest) except Exception as e: - raise ExportError( - f"Error copying file {aae_src} to {aae_dest}: {e} ({lineno(__file__)})" - ) from e + errors.append( + f"Error copying file {src} to {dest}: {e} ({lineno(__file__)})" + ) + + # set data in the database + fileutil = options.fileutil + with options.export_db.create_or_get_file_record( + str(dest), self.photo.uuid + ) as rec: + # don't set src_sig as that is set above before any modifications by convert_to_jpeg or exiftool + rec.src_sig = fileutil.file_sig(src) + if not options.ignore_signature: + rec.dest_sig = fileutil.file_sig(dest) + rec.export_options = options.bit_flags + if errors: + rec.error = { + "error": errors, + "exiftool_error": None, + "exiftool_warning": None, + } + + options.export_db.set_history( + filename=str(dest), uuid=self.photo.uuid, action=f"AAE: {action}", diff=None + ) verbose( - f"Exported adjustments of {self._filename(self.photo.original_filename)} to {self._filepath(aae_dest)}" + f"Exported adjustments for {self._filename(self.photo.original_filename)} to {self._filepath(dest)}" + ) + + written = [str(dest)] + exported = written if action in {"export", "new"} else [] + new = written if action == "new" else [] + updated = written if "update" in action else [] + return ExportResults( + aae_written=written, + exported=exported, + new=new, + updated=updated, + error=errors, ) - return ExportResults(aae_written=[aae_dest]) def write_exiftool_metadata_to_file( self, diff --git a/tests/test_cli.py b/tests/test_cli.py index a8732778..a47f3116 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -3854,6 +3854,50 @@ def test_export_aae_as_hardlink(): assert sorted(files) == sorted(CLI_EXPORT_AAE_FILENAMES) +def test_export_aae_update(): + """Test export with --export-aae --update""" + + runner = CliRunner() + cwd = os.getcwd() + # pylint: disable=not-context-manager + with runner.isolated_filesystem(): + result = runner.invoke( + cli_main, + [ + "export", + "--db", + os.path.join(cwd, CLI_PHOTOS_DB), + ".", + "--export-aae", + f"--uuid={CLI_EXPORT_AAE_UUID}", + "-V", + ], + ) + assert result.exit_code == 0 + files = glob.glob("*.*") + assert sorted(files) == sorted(CLI_EXPORT_AAE_FILENAMES) + + # now update + result = runner.invoke( + cli_main, + [ + "export", + "--db", + os.path.join(cwd, CLI_PHOTOS_DB), + ".", + "--export-aae", + f"--uuid={CLI_EXPORT_AAE_UUID}", + "--update", + "-V", + ], + ) + assert result.exit_code == 0 + assert "Error" not in result.output + assert re.findall( + r"Skipped up to date file (.*\.AAE)", result.output, re.MULTILINE + ) + + def test_export_sidecar(): """test --sidecar""" @@ -6496,7 +6540,7 @@ def test_export_ignore_signature(): def test_export_ignore_signature_sidecar(): """test export with --ignore-signature and --sidecar""" """ - Test the following use cases: + Test the following use cases: If the metadata (in Photos) that went into the sidecar did not change, the sidecar will not be updated If the metadata (in Photos) that went into the sidecar did change, a new sidecar is written but a new image file is not If a sidecar does not exist for the photo, a sidecar will be written whether or not the photo file was written