Skip to content

Convert OpenXmlAttribute to readonly #1282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 30, 2022
Merged

Convert OpenXmlAttribute to readonly #1282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

### Breaking change
- Core infrastructure is now contained in a new package DocumentFormat.OpenXml.Framework. Typed classes are still in DocumentFormat.OpenXml. This means that you may reference DocumentFormat.OpenXml and still compile the same types, but if you want a smaller package, you may rely on just the framework package.
- Removed mutable properties on OpenXmlAttribute and marked as `readonly`

## [2.20.0]

Expand Down
61 changes: 18 additions & 43 deletions src/DocumentFormat.OpenXml.Framework/OpenXmlAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,13 @@ namespace DocumentFormat.OpenXml
/// <summary>
/// Represents an Open XML attribute.
/// </summary>
public struct OpenXmlAttribute : IEquatable<OpenXmlAttribute>
public readonly struct OpenXmlAttribute : IEquatable<OpenXmlAttribute>
{
private const string ObsoleteMessage = "OpenXmlAttribute is a struct so setters may not behave as you might expect. Mutable setters will be removed in a future version.";

private string? _value;
private string _prefix;

internal OpenXmlAttribute(in OpenXmlQualifiedName qname, string prefix, string? value)
{
QName = qname;
_value = value;
_prefix = prefix;
Value = value;
Prefix = prefix;
}

/// <summary>
Expand All @@ -41,8 +36,8 @@ public OpenXmlAttribute(string qualifiedName, string namespaceUri, string? value
var parsed = PrefixName.Parse(qualifiedName);

QName = new(namespaceUri, parsed.Name);
_prefix = parsed.Prefix;
_value = value;
Prefix = parsed.Prefix;
Value = value;
}

/// <summary>
Expand All @@ -60,49 +55,29 @@ public OpenXmlAttribute(string prefix, string localName, string namespaceUri, st
}

QName = new(namespaceUri, localName);
_prefix = prefix;
_value = value;
Prefix = prefix;
Value = value;
}

/// <summary>
/// Gets or sets the namespace URI of the current attribute.
/// Gets the namespace URI of the current attribute.
/// </summary>
public string NamespaceUri
{
get => QName.Namespace.Uri;
[Obsolete(ObsoleteMessage)]
set => QName = new(value, LocalName);
}
public string NamespaceUri => QName.Namespace.Uri;

/// <summary>
/// Gets or sets the local name of the attribute.
/// Gets the local name of the attribute.
/// </summary>
public string LocalName
{
get => QName.Name;
[Obsolete(ObsoleteMessage)]
set => QName = new(QName.Namespace, value);
}
public string LocalName => QName.Name;

/// <summary>
/// Gets or sets the namespace prefix of the current attribute.
/// Gets the namespace prefix of the current attribute.
/// </summary>
public string Prefix
{
get => _prefix;
[Obsolete(ObsoleteMessage)]
set => _prefix = value;
}
public string Prefix { get; }

/// <summary>
/// Gets or sets the text value of the attribute.
/// Gets the text value of the attribute.
/// </summary>
public string? Value
{
get => _value;
[Obsolete(ObsoleteMessage)]
set => _value = value;
}
public string? Value { get; }

/// <summary>
/// Gets the qualified name of the attribute.
Expand All @@ -114,7 +89,7 @@ public string? Value
/// </summary>
public XName XName => XName.Get(LocalName, QName.Namespace.Uri);

internal OpenXmlQualifiedName QName { get; private set; }
internal OpenXmlQualifiedName QName { get; }

/// <summary>
/// Determines if this instance of an OpenXmlAttribute structure is equal to the specified instance of an OpenXmlAttribute structure.
Expand All @@ -124,7 +99,7 @@ public string? Value
public bool Equals(OpenXmlAttribute other)
=> string.Equals(Value, other.Value, StringComparison.Ordinal)
&& QName.Equals(other.QName)
&& string.Equals(_prefix, other._prefix, StringComparison.Ordinal);
&& string.Equals(Prefix, other.Prefix, StringComparison.Ordinal);

/// <summary>
/// Determines if two instances of OpenXmlAttribute structures are equal.
Expand Down Expand Up @@ -162,7 +137,7 @@ public override int GetHashCode()

hashcode.Add(Value, StringComparer.Ordinal);
hashcode.Add(QName);
hashcode.Add(_prefix, StringComparer.Ordinal);
hashcode.Add(Prefix, StringComparer.Ordinal);

return hashcode.ToHashCode();
}
Expand Down
12 changes: 3 additions & 9 deletions test/DocumentFormat.OpenXml.Tests/OpenXmlDomTest/OpenXmlCompositeElementTestClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,15 +587,9 @@ public void OpenXmlAttributeValueTypeTest()

Log.Comment("Constructing OpenXmlAttribute w:rsidR and assigning it to another variable...");
var wrsidR = new OpenXmlAttribute("w", "rsidR", "http://schemas.openxmlformats.org/wordprocessingml/2006/main", "00B327F7");
var wrsidP = wrsidR;
Log.VerifyTrue(wrsidP == wrsidR, "The assigned OpenXmlAttribute variable is NOT equal to original one.");

#pragma warning disable CS0618 // Type or member is obsolete
wrsidP.LocalName = "rsidP";
wrsidP.Value = "00EC35BB";
#pragma warning restore CS0618 // Type or member is obsolete
Log.Comment("Assigned new LocalName: {0} with ByValue: {1} and comparing it with original w:rsidR", wrsidP.LocalName, wrsidP.Value);
Log.VerifyFalse(wrsidP == wrsidR, "The assigned OpenXmlAttribute variable IS equal to original one.");
var wrsidP = new OpenXmlAttribute("w", "rsidP", "http://schemas.openxmlformats.org/wordprocessingml/2006/main", "00EC35BB");

Assert.NotEqual(wrsidP, wrsidR);
}

[Fact]
Expand Down
26 changes: 12 additions & 14 deletions test/DocumentFormat.OpenXml.Tests/ofapiTest/OpenXmlElementTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,6 @@ public void DefaultOpenXmlAttributeTest()
[Fact]
public void OpenXmlAttributeTest()
{
#pragma warning disable CS0618 // Type or member is obsolete
var target = new OpenXmlAttribute("test", "http://test", "test", "value");
var other = new OpenXmlAttribute("test", "http://test", "test", "value");

Expand All @@ -444,8 +443,13 @@ public void OpenXmlAttributeTest()
Assert.True(target.Equals((object)other));
Assert.True(Equals(target, other));
Assert.Equal(target.GetHashCode(), other.GetHashCode());
}

other.Value = "other";
[Fact]
public void OpenXmlAttributeTestDifferentValues()
{
var target = new OpenXmlAttribute("test", "http://test", "test", "value");
var other = new OpenXmlAttribute("test", "http://test", "test", "other");

Assert.NotEqual(target, other);
Assert.False(target == other);
Expand All @@ -454,18 +458,13 @@ public void OpenXmlAttributeTest()
Assert.False(target.Equals((object)other));
Assert.False(Equals(target, other));
Assert.NotEqual(target.GetHashCode(), other.GetHashCode());
}

other.Value = "value";

Assert.Equal(target, other);
Assert.True(target == other);
Assert.False(target != other);
Assert.True(target.Equals(other));
Assert.True(target.Equals((object)other));
Assert.True(Equals(target, other));
Assert.Equal(target.GetHashCode(), other.GetHashCode());

other.Prefix = "t";
[Fact]
public void OpenXmlAttributeTestDifferentPrefix()
{
var target = new OpenXmlAttribute("test", "http://test", "test", "value");
var other = new OpenXmlAttribute("t", "http://test", "test", "value");

Assert.NotEqual(target, other);
Assert.False(target == other);
Expand All @@ -474,7 +473,6 @@ public void OpenXmlAttributeTest()
Assert.False(target.Equals((object)other));
Assert.False(Equals(target, other));
Assert.NotEqual(target.GetHashCode(), other.GetHashCode());
#pragma warning restore CS0618 // Type or member is obsolete
}

/// <summary>
Expand Down