Skip to content

Commit

Permalink
[Image Resizer]Try to recover metadata even when the metadata data st…
Browse files Browse the repository at this point in the history
…ructure is invalid (#14914)

* metadata.Clone() fails also for situations where we still can recover metadata

metadata.Clone() is also an expensive operation (deep copy) and it is not necessary anymore as we build up the metadata object from scratch anyway

* If an exception is throw here something is seriously wrong with the metadata structure

We take all metadata we have read so far an write it to the resized image

* add log statement

* Adjust test written for #2447 as we are able to copy the metadata now

* Improve documentation
  • Loading branch information
CleanCodeDeveloper committed Dec 10, 2021
1 parent 1e00331 commit 9152ea8
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
4 changes: 2 additions & 2 deletions src/modules/imageresizer/tests/Models/ResizeOperationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ public void ExecuteCopiesFrameMetadata()
}

[TestMethod]
public void ExecuteCopiesFrameMetadataExceptWhenMetadataCannotBeCloned()
public void ExecuteCopiesFrameMetadataEvenWhenMetadataCannotBeCloned()
{
var operation = new ResizeOperation("TestMetadataIssue2447.jpg", _directory, Settings());

operation.Execute();

AssertEx.Image(
_directory.File(),
image => Assert.IsNull(((BitmapMetadata)image.Frames[0].Metadata).CameraModel));
image => Assert.IsNotNull(((BitmapMetadata)image.Frames[0].Metadata).CameraModel));
}

[TestMethod]
Expand Down
35 changes: 31 additions & 4 deletions src/modules/imageresizer/ui/Extensions/BitmapMetadataExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,31 @@ public static void SetQuerySafe(this BitmapMetadata metadata, string query, obje
}

/// <summary>
/// Gets all metadata
/// Iterates recursively through all metadata
/// Gets all metadata.
/// Iterates recursively through metadata and adds valid items to a list while skipping invalid data items.
/// </summary>
/// <remarks>
/// Invalid data items are items which throw an exception when reading the data with metadata.GetQuery(...).
/// Sometimes Metadata collections are improper closed and cause an exception on IEnumerator.MoveNext(). In this case, we return all data items which were successfully collected so far.
/// </remarks>
/// <returns>
/// metadata path and metadata value of all successfully read data items.
/// </returns>
public static List<(string metadataPath, object value)> GetListOfMetadata(this BitmapMetadata metadata)
{
var listOfAllMetadata = new List<(string metadataPath, object value)>();

GetMetadataRecursively(metadata, string.Empty);
try
{
GetMetadataRecursively(metadata, string.Empty);
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
{
Debug.WriteLine($"Exception while trying to iterate recursively over metadata. We were able to read {listOfAllMetadata.Count} metadata entries.");
Debug.WriteLine(ex);
}

return listOfAllMetadata;

Expand Down Expand Up @@ -202,7 +219,17 @@ public static List<(string metadataPath, object value)> GetListOfMetadataForDebu
{
var listOfAllMetadata = new List<(string metadataPath, object value)>();

GetMetadataRecursively(metadata, string.Empty);
try
{
GetMetadataRecursively(metadata, string.Empty);
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
{
Debug.WriteLine($"Exception while trying to iterate recursively over metadata. We were able to read {listOfAllMetadata.Count} metadata entries.");
Debug.WriteLine(ex);
}

return listOfAllMetadata;

Expand Down
7 changes: 2 additions & 5 deletions src/modules/imageresizer/ui/Models/ResizeOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,14 @@ public void Execute()
{
try
{
// Detect whether metadata can copied successfully
var modifiableMetadata = metadata.Clone();

#if DEBUG
Debug.WriteLine($"### Processing metadata of file {_file}");
modifiableMetadata.PrintsAllMetadataToDebugOutput();
metadata.PrintsAllMetadataToDebugOutput();
#endif

// read all metadata and build up metadata object from the scratch. Discard invalid (unreadable/unwritable) metadata.
var newMetadata = new BitmapMetadata(metadata.Format);
var listOfMetadata = modifiableMetadata.GetListOfMetadata();
var listOfMetadata = metadata.GetListOfMetadata();
foreach (var (metadataPath, value) in listOfMetadata)
{
if (value is BitmapMetadata bitmapMetadata)
Expand Down

0 comments on commit 9152ea8

Please sign in to comment.