From c19003e5b95f993887e1a0d8b4b0b1d52ecd32ec Mon Sep 17 00:00:00 2001 From: Ming Ming Date: Sat, 19 Nov 2022 21:16:56 +0800 Subject: [PATCH] Remove erratic metadata from heic files w/o exif --- app/lib/entity/file.dart | 43 ++++++++++++- app/lib/entity/webdav_response_parser.dart | 4 ++ app/lib/use_case/compat/v55.dart | 26 ++++++-- app/test/entity/file_test.dart | 71 ++++++++++++++++++++-- 4 files changed, 133 insertions(+), 11 deletions(-) diff --git a/app/lib/entity/file.dart b/app/lib/entity/file.dart index 6180fdcb..9a8d50f6 100644 --- a/app/lib/entity/file.dart +++ b/app/lib/entity/file.dart @@ -124,6 +124,7 @@ class Metadata with EquatableMixin { JsonObj json, { required MetadataUpgraderV1? upgraderV1, required MetadataUpgraderV2? upgraderV2, + required MetadataUpgraderV3? upgraderV3, }) { final jsonVersion = json["version"]; JsonObj? result = json; @@ -141,6 +142,13 @@ class Metadata with EquatableMixin { return null; } } + if (jsonVersion < 4) { + result = upgraderV3?.call(result); + if (result == null) { + _log.info("[fromJson] Version $jsonVersion not compatible"); + return null; + } + } return Metadata( lastUpdated: result["lastUpdated"] == null ? null @@ -219,7 +227,7 @@ class Metadata with EquatableMixin { final Exif? exif; /// versioning of this class, use to upgrade old persisted metadata - static const version = 3; + static const version = 4; static final _log = Logger("entity.file.Metadata"); } @@ -287,6 +295,35 @@ class MetadataUpgraderV2 implements MetadataUpgrader { static final _log = Logger("entity.file.MetadataUpgraderV2"); } +/// Upgrade v3 Metadata to v4 +class MetadataUpgraderV3 implements MetadataUpgrader { + const MetadataUpgraderV3({ + required this.fileContentType, + this.logFilePath, + }); + + @override + JsonObj? call(JsonObj json) { + if (fileContentType == "image/heic") { + // Version 3 metadata for heic may incorrectly have exif as null due to a + // bug in exifdart + if (json["exif"] == null) { + _log.fine("[call] Remove v3 metadata for file: $logFilePath"); + // return null to let the app parse the file again + return null; + } + } + return json; + } + + final String? fileContentType; + + /// File path for logging only + final String? logFilePath; + + static final _log = Logger("entity.file.MetadataUpgraderV3"); +} + class File with EquatableMixin implements FileDescriptor { File({ required String path, @@ -357,6 +394,10 @@ class File with EquatableMixin implements FileDescriptor { fileContentType: json["contentType"], logFilePath: json["path"], ), + upgraderV3: MetadataUpgraderV3( + fileContentType: json["contentType"], + logFilePath: json["path"], + ), ), isArchived: json["isArchived"], overrideDateTime: json["overrideDateTime"] == null diff --git a/app/lib/entity/webdav_response_parser.dart b/app/lib/entity/webdav_response_parser.dart index f5f6cd64..e63a274a 100644 --- a/app/lib/entity/webdav_response_parser.dart +++ b/app/lib/entity/webdav_response_parser.dart @@ -379,6 +379,10 @@ class _FilePropParser { fileContentType: _contentType, logFilePath: logFilePath, ), + upgraderV3: MetadataUpgraderV3( + fileContentType: _contentType, + logFilePath: logFilePath, + ), ); } } diff --git a/app/lib/use_case/compat/v55.dart b/app/lib/use_case/compat/v55.dart index 96f05f65..9451dd7a 100644 --- a/app/lib/use_case/compat/v55.dart +++ b/app/lib/use_case/compat/v55.dart @@ -18,7 +18,8 @@ class CompatV55 { final count = await countQ.map((r) => r.read(countExp)).getSingle(); onProgress?.call(0, count); - final needUpdates = >[]; + final dateTimeUpdates = >[]; + final imageRemoves = []; for (var i = 0; i < count; i += 1000) { final q = db.select(db.files).join([ sql.innerJoin( @@ -51,19 +52,28 @@ class CompatV55 { ); if (f.accountFile.bestDateTime != bestDateTime) { // need update - needUpdates.add(Tuple2(f.accountFile.rowId, bestDateTime)); + dateTimeUpdates.add(Tuple2(f.accountFile.rowId, bestDateTime)); + } + + if (f.file.contentType == "image/heic" && + f.image != null && + f.image!.exifRaw == null) { + imageRemoves.add(f.accountFile.rowId); } } onProgress?.call(i, count); } - _log.info("[migrateDb] ${needUpdates.length} rows require updating"); + _log.info( + "[migrateDb] ${dateTimeUpdates.length} rows require updating, ${imageRemoves.length} rows require removing"); if (kDebugMode) { _log.fine( - "[migrateDb] ${needUpdates.map((e) => e.item1).toReadableString()}"); + "[migrateDb] dateTimeUpdates: ${dateTimeUpdates.map((e) => e.item1).toReadableString()}"); + _log.fine( + "[migrateDb] imageRemoves: ${imageRemoves.map((e) => e).toReadableString()}"); } await db.batch((batch) { - for (final pair in needUpdates) { + for (final pair in dateTimeUpdates) { batch.update( db.accountFiles, sql.AccountFilesCompanion( @@ -73,6 +83,12 @@ class CompatV55 { table.rowId.equals(pair.item1), ); } + for (final r in imageRemoves) { + batch.deleteWhere( + db.images, + (sql.$ImagesTable table) => table.accountFile.equals(r), + ); + } }); }); } diff --git a/app/test/entity/file_test.dart b/app/test/entity/file_test.dart index 3de0f89a..b1675c02 100644 --- a/app/test/entity/file_test.dart +++ b/app/test/entity/file_test.dart @@ -111,7 +111,9 @@ void main() { "version": Metadata.version, "lastUpdated": "2020-01-02T03:04:05.678901Z", }; - expect(Metadata.fromJson(json, upgraderV1: null, upgraderV2: null), + expect( + Metadata.fromJson(json, + upgraderV1: null, upgraderV2: null, upgraderV3: null), Metadata(lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5, 678, 901))); }); @@ -122,7 +124,8 @@ void main() { "fileEtag": "8a3e0799b6f0711c23cc2d93950eceb5", }; expect( - Metadata.fromJson(json, upgraderV1: null, upgraderV2: null), + Metadata.fromJson(json, + upgraderV1: null, upgraderV2: null, upgraderV3: null), Metadata( lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5, 678, 901), fileEtag: "8a3e0799b6f0711c23cc2d93950eceb5", @@ -136,7 +139,8 @@ void main() { "imageWidth": 1024, }; expect( - Metadata.fromJson(json, upgraderV1: null, upgraderV2: null), + Metadata.fromJson(json, + upgraderV1: null, upgraderV2: null, upgraderV3: null), Metadata( lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5, 678, 901), imageWidth: 1024, @@ -150,7 +154,8 @@ void main() { "imageHeight": 768, }; expect( - Metadata.fromJson(json, upgraderV1: null, upgraderV2: null), + Metadata.fromJson(json, + upgraderV1: null, upgraderV2: null, upgraderV3: null), Metadata( lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5, 678, 901), imageHeight: 768, @@ -166,7 +171,8 @@ void main() { }, }; expect( - Metadata.fromJson(json, upgraderV1: null, upgraderV2: null), + Metadata.fromJson(json, + upgraderV1: null, upgraderV2: null, upgraderV3: null), Metadata( lastUpdated: DateTime.utc(2020, 1, 2, 3, 4, 5, 678, 901), exif: Exif({ @@ -328,6 +334,61 @@ void main() { }); }); + group("MetadataUpgraderV3", () { + test("exif null", () { + final json = { + "version": 3, + "exif": null, + "imageWidth": 1024, + "imageHeight": 768, + }; + expect( + const MetadataUpgraderV3(fileContentType: "image/heic")(json), + null, + ); + }); + + test("exif non-null", () { + final json = { + "version": 3, + "exif": { + "Make": "Amazing", + }, + "imageWidth": 1024, + "imageHeight": 768, + }; + expect( + const MetadataUpgraderV3(fileContentType: "image/heic")(json), + { + "version": 3, + "exif": { + "Make": "Amazing", + }, + "imageWidth": 1024, + "imageHeight": 768, + }, + ); + }); + + test("non-heic", () { + final json = { + "version": 3, + "exif": null, + "imageWidth": 1024, + "imageHeight": 768, + }; + expect( + const MetadataUpgraderV3(fileContentType: "image/jpeg")(json), + { + "version": 3, + "exif": null, + "imageWidth": 1024, + "imageHeight": 768, + }, + ); + }); + }); + group("File", () { group("constructor", () { test("path trim slash", () {