Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 54 additions & 0 deletions src/EPPlus/Drawing/ExcelDrawing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Date Author Change
using OfficeOpenXml.Utils.FileUtils;
using OfficeOpenXml.Utils.XML;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -336,6 +337,18 @@ public virtual string Name
try
{
if (_nvPrPath == "") throw new NotImplementedException();
string oldName = GetXmlNodeString(_nvPrPath + "/@name");
// Only perform dictionary synchronization if the name has actually changed
if (!string.IsNullOrEmpty(oldName) && !oldName.Equals(value, StringComparison.Ordinal))
{
// Validate name uniqueness in both worksheet and parent-group drawing collections.
// All checks must pass before any dictionaries are updated, preventing partial desynchronization.
ValidateNameUniquenessInCollections(value);

// All uniqueness checks successfully passed! Safe to update name-lookup dictionaries.
UpdateNameInDictionaries(oldName, value);
}

SetXmlNodeString(_nvPrPath + "/@name", value);
if (this is ExcelSlicer<ExcelTableSlicerCache> ts)
{
Expand All @@ -348,6 +361,10 @@ public virtual string Name
pts.SlicerName = value;
}
}
catch (ArgumentException)
{
throw;
}
catch
{
throw new NotImplementedException();
Expand Down Expand Up @@ -2467,6 +2484,43 @@ private XmlNode CopyShape(ExcelWorksheet worksheet, bool isGroupShape = false, X
return drawNode;
}

private void ValidateNameUniqueness(Dictionary<string, int> drawingNames, IEnumerable<ExcelDrawing> collection, string name)
{
if (drawingNames?.ContainsKey(name) == true)
{
int index = drawingNames[name];
var drawing = collection?.ElementAtOrDefault(index);

if (drawing != null && drawing != this)
{
string collectionDescription = collection is ExcelDrawingsGroup ? "the group drawings collection" : "drawings collection";
throw new ArgumentException($"Name '{name}' already exists in {collectionDescription}.");
}
}
}

private void ValidateNameUniquenessInCollections(string name)
{
ValidateNameUniqueness(_drawings?._drawingNames, _drawings?._drawingsList, name);
ValidateNameUniqueness(_parent?.Drawings?._drawingNames, _parent?.Drawings, name);
}

private void UpdateNameInDictionary(Dictionary<string, int> drawingNames, string oldName, string newName)
{
if (drawingNames?.ContainsKey(oldName) == true)
{
int index = drawingNames[oldName];
drawingNames.Remove(oldName);
drawingNames[newName] = index;
}
}

private void UpdateNameInDictionaries(string oldName, string newName)
{
UpdateNameInDictionary(_drawings?._drawingNames, oldName, newName);
UpdateNameInDictionary(_parent?.Drawings?._drawingNames, oldName, newName);
}

internal ExcelAddressBase GetAddress()
{
GetFromBounds(out int fromRow, out _, out int fromCol, out _);
Expand Down
18 changes: 16 additions & 2 deletions src/EPPlus/Drawing/ExcelGroupShape.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal ExcelDrawingsGroup(ExcelGroupShape parent, XmlNamespaceManager nsm, Xml
_parent = parent;
_nsm = nsm;
_topNode = topNode;
_drawingNames = new Dictionary<string, int>();
_drawingNames = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
_drawingsCollectionType = drawingsCollectionType;
AddDrawings();
}
Expand Down Expand Up @@ -298,7 +298,21 @@ IEnumerator IEnumerable.GetEnumerator()
public void Remove(ExcelDrawing drawing)
{
CheckNotDisposed();
_groupDrawings.Remove(drawing);
// Find the 0-based list index of the drawing within this group
int index = _groupDrawings.IndexOf(drawing);
if (index < 0) return;

// Remove the drawing from the list and its entry from the lookup dictionary
_groupDrawings.RemoveAt(index);
_drawingNames.Remove(drawing.Name);

// Re-index all remaining drawings after the removed index in the group's name-to-index lookup.
// When an element is removed, all subsequent elements shift down, so we update their mapped indices accordingly.
for (int i = index; i < _groupDrawings.Count; i++)
{
_drawingNames[_groupDrawings[i].Name] = i;
}

AdjustXmlAndMoveFromGroup(drawing);
var ix = _parent._drawings._drawingsList.IndexOf(_parent);
_parent._drawings._drawingsList.Insert(ix, drawing);
Expand Down
57 changes: 57 additions & 0 deletions src/EPPlusTest/Drawing/DrawingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,5 +1218,62 @@ public void AddTextBox()
SaveAndCleanup(package);
}
}
[TestMethod]
public void DrawingNameChangeAndRemovalTest()
{
using (var package = new ExcelPackage())
{
var ws = package.Workbook.Worksheets.Add("RenameTest");

// Add a drawing (shape)
var shape = ws.Drawings.AddShape("InitialName", eShapeStyle.Rect);
Assert.AreEqual("InitialName", shape.Name);
Assert.IsTrue(ws.Drawings._drawingNames.ContainsKey("InitialName"));

// Rename drawing
shape.Name = "RenamedName";
Assert.AreEqual("RenamedName", shape.Name);

// Verify the internal dictionary was updated
Assert.IsFalse(ws.Drawings._drawingNames.ContainsKey("InitialName"), "Old name key should be removed from dictionary");
Assert.IsTrue(ws.Drawings._drawingNames.ContainsKey("RenamedName"), "New name key should be added to dictionary");

// Test removing by shape object reference
ws.Drawings.Remove(shape);
Assert.AreEqual(0, ws.Drawings.Count, "Drawing should be removed successfully");
Assert.IsFalse(ws.Drawings._drawingNames.ContainsKey("RenamedName"), "Dictionary should no longer contain renamed name");
}
}

[TestMethod]
public void GroupedDrawingNameChangeAndRemovalTest()
{
using (var package = new ExcelPackage())
{
var ws = package.Workbook.Worksheets.Add("GroupRenameTest");

var shape1 = ws.Drawings.AddShape("Shape1", eShapeStyle.Rect);
var shape2 = ws.Drawings.AddShape("Shape2", eShapeStyle.Rect);

// Group shapes using public EPPlus API
var group = shape1.Group(shape2);

Assert.AreEqual("Shape1", shape1.Name);
Assert.IsTrue(group.Drawings._drawingNames.ContainsKey("Shape1"));

// Rename grouped shape
shape1.Name = "GroupedShape1Renamed";
Assert.AreEqual("GroupedShape1Renamed", shape1.Name);

// Verify the internal group dictionary was updated
Assert.IsFalse(group.Drawings._drawingNames.ContainsKey("Shape1"), "Old name key should be removed from group dictionary");
Assert.IsTrue(group.Drawings._drawingNames.ContainsKey("GroupedShape1Renamed"), "New name key should be added to group dictionary");

// Test removing grouped shape
group.Drawings.Remove(shape1);
Assert.AreEqual(1, group.Drawings.Count, "One shape should be left in group");
Assert.IsFalse(group.Drawings._drawingNames.ContainsKey("GroupedShape1Renamed"), "Group dictionary should no longer contain removed name");
}
}
}
}
Loading