From fa3e6ff1808fe1b75cbd4b87c0e6525901894d08 Mon Sep 17 00:00:00 2001 From: Ynse Hoornenborg Date: Sun, 2 Jan 2022 14:18:36 +0100 Subject: [PATCH] More robust equals implementation --- .../Profiles/XMP/XElementEqualityComparer.cs | 21 +++++++ .../Metadata/Profiles/XMP/XmpProfile.cs | 56 ++++++++++++++----- .../Metadata/Profiles/XMP/XmpProfileTests.cs | 53 +++++++++++++----- 3 files changed, 104 insertions(+), 26 deletions(-) create mode 100644 src/ImageSharp/Metadata/Profiles/XMP/XElementEqualityComparer.cs diff --git a/src/ImageSharp/Metadata/Profiles/XMP/XElementEqualityComparer.cs b/src/ImageSharp/Metadata/Profiles/XMP/XElementEqualityComparer.cs new file mode 100644 index 000000000..156872792 --- /dev/null +++ b/src/ImageSharp/Metadata/Profiles/XMP/XElementEqualityComparer.cs @@ -0,0 +1,21 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Xml.Linq; + +namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp +{ + /// + /// Compare objects for Name and Value equality. + /// + public class XElementEqualityComparer : IEqualityComparer + { + /// + public bool Equals([AllowNull] XElement x, [AllowNull] XElement y) => x.Name == y.Name && x.Value == y.Value; + + /// + public int GetHashCode([DisallowNull] XElement obj) => obj.GetHashCode(); + } +} diff --git a/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs b/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs index 7c9cfc55f..7261b4521 100644 --- a/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs +++ b/src/ImageSharp/Metadata/Profiles/XMP/XmpProfile.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Linq; using System.Text; using System.Xml.Linq; @@ -10,6 +11,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp { /// /// Represents an XMP profile, providing access to the raw XML. + /// See for the full specification. /// public sealed class XmpProfile : IDeepCloneable { @@ -29,6 +31,16 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp /// The UTF8 encoded byte array to read the XMP profile from. public XmpProfile(byte[] data) => this.Data = data; + /// + /// Initializes a new instance of the class, based on the speicief . + /// + /// The document to base this instance on. + public XmpProfile(XDocument doc) + { + this.document = doc; + this.UpdateData(); + } + /// /// Initializes a new instance of the class /// by making a copy from another XMP profile. @@ -78,7 +90,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp return true; } - if (ReferenceEquals(left, null)) + if (left is null) { return false; } @@ -95,10 +107,7 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp /// True if the parameter is not equal to the parameter; /// otherwise, false. /// - public static bool operator !=(XmpProfile left, XmpProfile right) - { - return !(left == right); - } + public static bool operator !=(XmpProfile left, XmpProfile right) => !(left == right); /// public XmpProfile DeepClone() => new(this); @@ -113,7 +122,13 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp return; } - using var stream = new MemoryStream(this.Data.Length); + int initialLength = 256; + if (this.Data != null) + { + initialLength = this.Data.Length; + } + + using var stream = new MemoryStream(initialLength); using var writer = new StreamWriter(stream, Encoding.UTF8); this.document.Save(writer); this.Data = stream.ToArray(); @@ -125,28 +140,37 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp /// public override bool Equals(object obj) { - XmpProfile other = obj as XmpProfile; - if (ReferenceEquals(other, null)) + if (obj is not XmpProfile other) { return false; } - if (ReferenceEquals(this.Data, null)) + XElement thisRoot = this.Document.Root; + XElement otherRoot = other.Document.Root; + + return this.CompareElements(thisRoot, otherRoot); + } + + private bool CompareElements(XElement left, XElement right) + { + var comparer = new XElementEqualityComparer(); + bool result = comparer.Equals(left, right); + if (result) { - return false; + result |= !left.Elements().Except(right.Elements(), comparer).Any(); } - return this.Data.Equals(other.Data); + return result; } private void InitializeDocument() { - if (this.document != null) + if (!(this.document is null)) { return; } - if (this.Data == null) + if (this.Data is null) { return; } @@ -164,6 +188,12 @@ namespace SixLabors.ImageSharp.Metadata.Profiles.Xmp using var stream = new MemoryStream(this.Data, 0, count); using var reader = new StreamReader(stream, Encoding.UTF8); this.document = XDocument.Load(reader); + + // In case we removed any trailing bytes, update the Data property accordingly. + if (count != this.Data.Length) + { + this.UpdateData(); + } } } } diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs index 371c110b7..5aa68f59d 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.IO; using System.Text; using System.Xml.Linq; @@ -108,22 +109,36 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp } [Fact] - public void XmpProfile_EqualalityIsByValue() + public void XmpProfile_EqualityIsByValue() { // arrange - byte[] content = new byte[0]; - XmpProfile original = new XmpProfile(content); - XmpProfile other = new XmpProfile(content); + XmpProfile original = CreateMinimalXmlProfile(); + var other = new XmpProfile(original.Data); // act - var equals = original.Equals(other); - var equality = original == other; - var inequality = original != other; + bool equals = original.Equals(other); + bool equality = original == other; + bool inequality = original != other; // assert Assert.True(equals); Assert.True(equality); Assert.False(inequality); + Assert.Equal(original, other); + } + + [Fact] + public void XmpProfile_DocumentConstructor() + { + // arrange + XmpProfile original = CreateMinimalXmlProfile(); + + // act + var actual = new XmpProfile(original.Document); + + // assert + XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -147,7 +162,8 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp { // arrange var image = new Image(1, 1); - image.Metadata.XmpProfile = CreateMinimalXmlProfile(); + XmpProfile original = CreateMinimalXmlProfile(); + image.Metadata.XmpProfile = original; var encoder = new GifEncoder(); // act @@ -156,6 +172,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -163,7 +180,8 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp { // arrange var image = new Image(1, 1); - image.Metadata.XmpProfile = CreateMinimalXmlProfile(); + XmpProfile original = CreateMinimalXmlProfile(); + image.Metadata.XmpProfile = original; var encoder = new JpegEncoder(); // act @@ -172,6 +190,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -180,6 +199,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // arrange var provider = TestImageProvider.File(TestImages.Jpeg.Baseline.ExtendedXmp); using Image image = await provider.GetImageAsync(JpegDecoder); + XmpProfile original = image.Metadata.XmpProfile; var encoder = new JpegEncoder(); // act @@ -188,6 +208,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -195,7 +216,8 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp { // arrange var image = new Image(1, 1); - image.Metadata.XmpProfile = CreateMinimalXmlProfile(); + XmpProfile original = CreateMinimalXmlProfile(); + image.Metadata.XmpProfile = original; var encoder = new PngEncoder(); // act @@ -204,6 +226,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -211,7 +234,8 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp { // arrange var image = new Image(1, 1); - image.Frames.RootFrame.Metadata.XmpProfile = CreateMinimalXmlProfile(); + XmpProfile original = CreateMinimalXmlProfile(); + image.Frames.RootFrame.Metadata.XmpProfile = original; var encoder = new TiffEncoder(); // act @@ -220,6 +244,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } [Fact] @@ -227,7 +252,8 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp { // arrange var image = new Image(1, 1); - image.Metadata.XmpProfile = CreateMinimalXmlProfile(); + XmpProfile original = CreateMinimalXmlProfile(); + image.Metadata.XmpProfile = original; var encoder = new WebpEncoder(); // act @@ -236,6 +262,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp // assert XmpProfile actual = reloadedImage.Metadata.XmpProfile ?? reloadedImage.Frames.RootFrame.Metadata.XmpProfile; XmpProfileContainsExpectedValues(actual); + Assert.Equal(original, actual); } private static void XmpProfileContainsExpectedValues(XmpProfile xmp) @@ -249,7 +276,7 @@ namespace SixLabors.ImageSharp.Tests.Metadata.Profiles.Xmp private static XmpProfile CreateMinimalXmlProfile() { - string content = ""; + string content = $""; byte[] data = Encoding.UTF8.GetBytes(content); var profile = new XmpProfile(data); return profile;