From 52b16a32be81694ff32e7da820c3fab934c76c93 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 27 Mar 2018 11:39:52 -0700 Subject: [PATCH] Make ExifValue immutable --- .../MetaData/Profiles/Exif/ExifProfile.cs | 9 +- .../MetaData/Profiles/Exif/ExifValue.cs | 163 ++++++++---------- .../MetaData/Profiles/Exif/ExifWriter.cs | 32 ++-- .../Profiles/Exif/ExifProfileTests.cs | 10 +- 4 files changed, 94 insertions(+), 120 deletions(-) diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifProfile.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifProfile.cs index 7cd2d002d..82542d2ee 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifProfile.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifProfile.cs @@ -106,7 +106,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// /// Gets the values of this EXIF profile. /// - public IEnumerable Values + public IList Values { get { @@ -193,16 +193,17 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// The value. public void SetValue(ExifTag tag, object value) { - foreach (ExifValue exifValue in this.Values) + for (int i = 0; i < this.Values.Count; i++) { - if (exifValue.Tag == tag) + if (this.Values[i].Tag == tag) { - exifValue.Value = value; + this.Values[i] = this.Values[i].WithValue(value); return; } } var newExifValue = ExifValue.Create(tag, value); + this.values.Add(newExifValue); } diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifValue.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifValue.cs index e36d0a25a..055c22995 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifValue.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifValue.cs @@ -13,11 +13,6 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// public sealed class ExifValue : IEquatable { - /// - /// The exif value. - /// - private object exifValue; - /// /// Initializes a new instance of the class /// by making a copy from another exif value. @@ -31,33 +26,16 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif this.DataType = other.DataType; this.IsArray = other.IsArray; this.Tag = other.Tag; + if (!other.IsArray) { - this.exifValue = other.exifValue; + this.Value = other.Value; } else { - var array = (Array)other.exifValue; - this.exifValue = array.Clone(); - } - } - - /// - /// Initializes a new instance of the class. - /// - /// The tag. - /// The data type. - /// Whether the value is an array. - internal ExifValue(ExifTag tag, ExifDataType dataType, bool isArray) - { - this.Tag = tag; - this.DataType = dataType; - this.IsArray = isArray; - - if (dataType == ExifDataType.Ascii) - { - this.IsArray = false; + var array = (Array)other.Value; + this.Value = array.Clone(); } } @@ -69,9 +47,13 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// The value. /// Whether the value is an array. internal ExifValue(ExifTag tag, ExifDataType dataType, object value, bool isArray) - : this(tag, dataType, isArray) { - this.exifValue = value; + this.Tag = tag; + this.DataType = dataType; + this.IsArray = isArray && dataType != ExifDataType.Ascii; + this.Value = value; + + // this.CheckValue(value); } /// @@ -90,17 +72,9 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif public ExifTag Tag { get; } /// - /// Gets or sets the value. + /// Gets the value. /// - public object Value - { - get => this.exifValue; - set - { - this.CheckValue(value); - this.exifValue = value; - } - } + public object Value { get; } /// /// Gets a value indicating whether the EXIF value has a value. @@ -109,14 +83,14 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif { get { - if (this.exifValue == null) + if (this.Value == null) { return false; } if (this.DataType == ExifDataType.Ascii) { - return ((string)this.exifValue).Length > 0; + return ((string)this.Value).Length > 0; } return true; @@ -130,7 +104,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif { get { - if (this.exifValue == null) + if (this.Value == null) { return 4; } @@ -150,18 +124,30 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif { if (this.DataType == ExifDataType.Ascii) { - return Encoding.UTF8.GetBytes((string)this.exifValue).Length; + return Encoding.UTF8.GetBytes((string)this.Value).Length; } if (this.IsArray) { - return ((Array)this.exifValue).Length; + return ((Array)this.Value).Length; } return 1; } } + /// + /// Clones the current value, overwriting the value. + /// + /// The value to overwrite. + /// + public ExifValue WithValue(object value) + { + this.CheckValue(value); + + return new ExifValue(this.Tag, this.DataType, value, this.IsArray); + } + /// /// Compares two objects for equality. /// @@ -223,7 +209,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return this.Tag == other.Tag && this.DataType == other.DataType && - object.Equals(this.exifValue, other.exifValue); + object.Equals(this.Value, other.Value); } /// @@ -235,23 +221,23 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// public override string ToString() { - if (this.exifValue == null) + if (this.Value == null) { return null; } if (this.DataType == ExifDataType.Ascii) { - return (string)this.exifValue; + return (string)this.Value; } if (!this.IsArray) { - return this.ToString(this.exifValue); + return this.ToString(this.Value); } var sb = new StringBuilder(); - foreach (object value in (Array)this.exifValue) + foreach (object value in (Array)this.Value) { sb.Append(this.ToString(value)); sb.Append(' '); @@ -275,13 +261,6 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif { Guard.IsFalse(tag == ExifTag.Unknown, nameof(tag), "Invalid Tag"); - ExifValue exifValue; - Type type = value?.GetType(); - if (type != null && type.IsArray) - { - type = type.GetElementType(); - } - switch (tag) { case ExifTag.ImageDescription: @@ -337,8 +316,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.GPSDestBearingRef: case ExifTag.GPSDestDistanceRef: case ExifTag.GPSDateStamp: - exifValue = new ExifValue(tag, ExifDataType.Ascii, true); - break; + return new ExifValue(tag, ExifDataType.Ascii, value, true); case ExifTag.ClipPath: case ExifTag.VersionYear: @@ -351,13 +329,12 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.XPKeywords: case ExifTag.XPSubject: case ExifTag.GPSVersionID: - exifValue = new ExifValue(tag, ExifDataType.Byte, true); - break; + return new ExifValue(tag, ExifDataType.Byte, value, true); + case ExifTag.FaxProfile: case ExifTag.ModeNumber: case ExifTag.GPSAltitudeRef: - exifValue = new ExifValue(tag, ExifDataType.Byte, false); - break; + return new ExifValue(tag, ExifDataType.Byte, value, false); case ExifTag.FreeOffsets: case ExifTag.FreeByteCounts: @@ -371,8 +348,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.StripRowCounts: case ExifTag.IntergraphRegisters: case ExifTag.TimeZoneOffset: - exifValue = new ExifValue(tag, ExifDataType.Long, true); - break; + return new ExifValue(tag, ExifDataType.Long, value, true); case ExifTag.SubfileType: case ExifTag.SubIFDOffset: case ExifTag.GPSIFDOffset: @@ -394,8 +370,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.FaxRecvParams: case ExifTag.FaxRecvTime: case ExifTag.ImageNumber: - exifValue = new ExifValue(tag, ExifDataType.Long, false); - break; + return new ExifValue(tag, ExifDataType.Long, value, false); case ExifTag.WhitePoint: case ExifTag.PrimaryChromaticities: @@ -410,8 +385,8 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.GPSTimestamp: case ExifTag.GPSDestLatitude: case ExifTag.GPSDestLongitude: - exifValue = new ExifValue(tag, ExifDataType.Rational, true); - break; + return new ExifValue(tag, ExifDataType.Rational, value, true); + case ExifTag.XPosition: case ExifTag.YPosition: case ExifTag.XResolution: @@ -445,8 +420,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.GPSImgDirection: case ExifTag.GPSDestBearing: case ExifTag.GPSDestDistance: - exifValue = new ExifValue(tag, ExifDataType.Rational, false); - break; + return new ExifValue(tag, ExifDataType.Rational, value, false); case ExifTag.BitsPerSample: case ExifTag.MinSampleValue: @@ -469,8 +443,8 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.ISOSpeedRatings: case ExifTag.SubjectArea: case ExifTag.SubjectLocation: - exifValue = new ExifValue(tag, ExifDataType.Short, true); - break; + return new ExifValue(tag, ExifDataType.Short, value, true); + case ExifTag.OldSubfileType: case ExifTag.Compression: case ExifTag.PhotometricInterpretation: @@ -517,20 +491,18 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.Sharpness: case ExifTag.SubjectDistanceRange: case ExifTag.GPSDifferential: - exifValue = new ExifValue(tag, ExifDataType.Short, false); - break; + return new ExifValue(tag, ExifDataType.Short, value, false); case ExifTag.Decode: - exifValue = new ExifValue(tag, ExifDataType.SignedRational, true); - break; + return new ExifValue(tag, ExifDataType.SignedRational, value, true); + case ExifTag.ShutterSpeedValue: case ExifTag.BrightnessValue: case ExifTag.ExposureBiasValue: case ExifTag.AmbientTemperature: case ExifTag.WaterDepth: case ExifTag.CameraElevationAngle: - exifValue = new ExifValue(tag, ExifDataType.SignedRational, false); - break; + return new ExifValue(tag, ExifDataType.SignedRational, value, false); case ExifTag.JPEGTables: case ExifTag.OECF: @@ -547,18 +519,17 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.ImageSourceData: case ExifTag.GPSProcessingMethod: case ExifTag.GPSAreaInformation: - exifValue = new ExifValue(tag, ExifDataType.Undefined, true); - break; + return new ExifValue(tag, ExifDataType.Undefined, value, true); + case ExifTag.FileSource: case ExifTag.SceneType: - exifValue = new ExifValue(tag, ExifDataType.Undefined, false); - break; + return new ExifValue(tag, ExifDataType.Undefined, value, false); case ExifTag.StripOffsets: case ExifTag.TileByteCounts: case ExifTag.ImageLayer: - exifValue = CreateNumber(tag, type, true); - break; + return CreateNumber(tag, value, true); + case ExifTag.ImageWidth: case ExifTag.ImageLength: case ExifTag.TileWidth: @@ -567,15 +538,11 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif case ExifTag.ConsecutiveBadFaxLines: case ExifTag.PixelXDimension: case ExifTag.PixelYDimension: - exifValue = CreateNumber(tag, type, false); - break; + return CreateNumber(tag, value, false); default: throw new NotSupportedException(); } - - exifValue.Value = value; - return exifValue; } /// @@ -617,29 +584,35 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// Returns an EXIF value with a numeric type for the given tag. /// /// The tag. - /// The numeric type. + /// The value. /// Whether the value is an array. /// /// The . /// - private static ExifValue CreateNumber(ExifTag tag, Type type, bool isArray) + private static ExifValue CreateNumber(ExifTag tag, object value, bool isArray) { + Type type = value?.GetType(); + if (type != null && type.IsArray) + { + type = type.GetElementType(); + } + if (type == null || type == typeof(ushort)) { - return new ExifValue(tag, ExifDataType.Short, isArray); + return new ExifValue(tag, ExifDataType.Short, value, isArray); } if (type == typeof(short)) { - return new ExifValue(tag, ExifDataType.SignedShort, isArray); + return new ExifValue(tag, ExifDataType.SignedShort, value, isArray); } if (type == typeof(uint)) { - return new ExifValue(tag, ExifDataType.Long, isArray); + return new ExifValue(tag, ExifDataType.Long, value, isArray); } - return new ExifValue(tag, ExifDataType.SignedLong, isArray); + return new ExifValue(tag, ExifDataType.SignedLong, value, isArray); } /// @@ -770,7 +743,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif private int GetHashCode(ExifValue exif) { int hashCode = exif.Tag.GetHashCode() ^ exif.DataType.GetHashCode(); - return hashCode ^ exif.exifValue?.GetHashCode() ?? hashCode; + return hashCode ^ exif.Value?.GetHashCode() ?? hashCode; } } } \ No newline at end of file diff --git a/src/ImageSharp/MetaData/Profiles/Exif/ExifWriter.cs b/src/ImageSharp/MetaData/Profiles/Exif/ExifWriter.cs index a4cfc7e13..ef51392dd 100644 --- a/src/ImageSharp/MetaData/Profiles/Exif/ExifWriter.cs +++ b/src/ImageSharp/MetaData/Profiles/Exif/ExifWriter.cs @@ -294,18 +294,18 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif /// Which parts will be written. /// private ExifParts allowedParts; - private Collection values; - private Collection dataOffsets; - private Collection ifdIndexes; - private Collection exifIndexes; - private Collection gpsIndexes; + private IList values; + private IList dataOffsets; + private IList ifdIndexes; + private IList exifIndexes; + private IList gpsIndexes; /// /// Initializes a new instance of the class. /// /// The values. /// The allowed parts. - public ExifWriter(Collection values, ExifParts allowedParts) + public ExifWriter(IList values, ExifParts allowedParts) { this.values = values; this.allowedParts = allowedParts; @@ -377,12 +377,12 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif if (exifLength > 0) { - this.values[exifIndex].Value = ifdOffset + ifdLength; + this.values[exifIndex] = this.values[exifIndex].WithValue(ifdOffset + ifdLength); } if (gpsLength > 0) { - this.values[gpsIndex].Value = ifdOffset + ifdLength + exifLength; + this.values[gpsIndex] = this.values[gpsIndex].WithValue(ifdOffset + ifdLength + exifLength); } i = Write(BitConverter.GetBytes(ifdOffset), result, i); @@ -414,7 +414,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return offset + source.Length; } - private int GetIndex(Collection indexes, ExifTag tag) + private int GetIndex(IList indexes, ExifTag tag) { foreach (int index in indexes) { @@ -437,7 +437,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return new Collection(); } - Collection result = new Collection(); + var result = new Collection(); for (int i = 0; i < this.values.Count; i++) { ExifValue value = this.values[i]; @@ -457,7 +457,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return result; } - private uint GetLength(IEnumerable indexes) + private uint GetLength(IList indexes) { uint length = 0; @@ -494,7 +494,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return newOffset; } - private int WriteData(Collection indexes, byte[] destination, int offset) + private int WriteData(IList indexes, byte[] destination, int offset) { if (this.dataOffsets.Count == 0) { @@ -517,9 +517,9 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return newOffset; } - private int WriteHeaders(Collection indexes, byte[] destination, int offset) + private int WriteHeaders(IList indexes, byte[] destination, int offset) { - this.dataOffsets = new Collection(); + this.dataOffsets = new List(); int newOffset = Write(BitConverter.GetBytes((ushort)indexes.Count), destination, offset); @@ -550,7 +550,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return newOffset; } - private int WriteRational(Rational value, byte[] destination, int offset) + private int WriteRational(in Rational value, byte[] destination, int offset) { Write(BitConverter.GetBytes(value.Numerator), destination, offset); Write(BitConverter.GetBytes(value.Denominator), destination, offset + 4); @@ -558,7 +558,7 @@ namespace SixLabors.ImageSharp.MetaData.Profiles.Exif return offset + 8; } - private int WriteSignedRational(SignedRational value, byte[] destination, int offset) + private int WriteSignedRational(in SignedRational value, byte[] destination, int offset) { Write(BitConverter.GetBytes(value.Numerator), destination, offset); Write(BitConverter.GetBytes(value.Denominator), destination, offset + 4); diff --git a/tests/ImageSharp.Tests/MetaData/Profiles/Exif/ExifProfileTests.cs b/tests/ImageSharp.Tests/MetaData/Profiles/Exif/ExifProfileTests.cs index 65a469b6f..7b5924b77 100644 --- a/tests/ImageSharp.Tests/MetaData/Profiles/Exif/ExifProfileTests.cs +++ b/tests/ImageSharp.Tests/MetaData/Profiles/Exif/ExifProfileTests.cs @@ -141,7 +141,7 @@ namespace SixLabors.ImageSharp.Tests ExifValue value = image.MetaData.ExifProfile.GetValue(ExifTag.Software); TestValue(value, "ImageSharp"); - Assert.Throws(() => { value.Value = 15; }); + Assert.Throws(() => { value.WithValue(15); }); image.MetaData.ExifProfile.SetValue(ExifTag.ShutterSpeedValue, new SignedRational(75.55)); @@ -149,7 +149,7 @@ namespace SixLabors.ImageSharp.Tests TestValue(value, new SignedRational(7555, 100)); - Assert.Throws(() => { value.Value = 75; }); + Assert.Throws(() => { value.WithValue(75); }); image.MetaData.ExifProfile.SetValue(ExifTag.XResolution, new Rational(150.0)); @@ -159,7 +159,7 @@ namespace SixLabors.ImageSharp.Tests value = image.MetaData.ExifProfile.GetValue(ExifTag.XResolution); TestValue(value, new Rational(150, 1)); - Assert.Throws(() => { value.Value = "ImageSharp"; }); + Assert.Throws(() => { value.WithValue("ImageSharp"); }); image.MetaData.ExifProfile.SetValue(ExifTag.ReferenceBlackWhite, null); @@ -209,11 +209,11 @@ namespace SixLabors.ImageSharp.Tests [Fact] public void Syncs() { - ExifProfile exifProfile = new ExifProfile(); + var exifProfile = new ExifProfile(); exifProfile.SetValue(ExifTag.XResolution, new Rational(200)); exifProfile.SetValue(ExifTag.YResolution, new Rational(300)); - ImageMetaData metaData = new ImageMetaData(); + var metaData = new ImageMetaData(); metaData.ExifProfile = exifProfile; metaData.HorizontalResolution = 200; metaData.VerticalResolution = 300;