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

@@ -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,

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

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,

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