Fixed part already exists and rId errors when copying pictures between workbooks (#1846)

* fix for i1841

* In-depth tests + extras

* Fixed issues with rId when copying back

* Moved tests to copy drawing file

Author: OssianEPPlus

src/EPPlus/Drawing/ExcelDrawing.cs

@@ -1686,73 +1686,100 @@ private XmlNode CopyChart(ExcelWorksheet worksheet, bool isGroupShape = false, X
             return drawNode;
         }
 
-        private XmlNode CopyPicture(ExcelWorksheet worksheet, bool isGroupShape = false, XmlNode groupDrawNode = null)
+        private XmlNode CopyPicture(ExcelWorksheet targetWorksheet, bool isGroupShape = false, XmlNode groupDrawNode = null)
         {
             XmlNode drawNode = null;
+
+            var targetWorkbook = targetWorksheet.Workbook;
+            var targetPackage = targetWorkbook._package;
+
             if (isGroupShape && groupDrawNode != null)
             {
                 drawNode = groupDrawNode;
-                groupDrawNode.SelectSingleNode("xdr:nvPicPr/xdr:cNvPr", worksheet._drawings.NameSpaceManager).Attributes["id"].Value = (++worksheet.Workbook._nextDrawingId).ToString();
+                groupDrawNode.SelectSingleNode("xdr:nvPicPr/xdr:cNvPr", targetWorksheet._drawings.NameSpaceManager).Attributes["id"].Value = (++targetWorkbook._nextDrawingId).ToString();
             }
             else
             {
                 //Create node in drawing.xml
-                drawNode = worksheet.Drawings.CreateDocumentAndTopNode(CellAnchor, false);
+                drawNode = targetWorksheet.Drawings.CreateDocumentAndTopNode(CellAnchor, false);
                 drawNode.InnerXml = TopNode.InnerXml;
             }
             //If same drawings object, we are done.
-            if (worksheet._drawings != _drawings)
+            if (targetWorksheet._drawings != _drawings)
             {
                 //Get the relation node
                 var relNode = drawNode.SelectSingleNode("xdr:pic/xdr:blipFill/a:blip/@r:embed", NameSpaceManager);
                 if(relNode == null)
                 {
                     relNode = drawNode.SelectSingleNode("xdr:blipFill/a:blip/@r:embed", NameSpaceManager);
                 }
+
                 if (relNode != null && _drawings.Part.RelationshipExists(relNode.Value))
                 {
-                    var rel = _drawings.Part.GetRelationship(relNode.Value);
+                    var srcsRel = _drawings.Part.GetRelationship(relNode.Value);
+                    ZipPackageRelationship newRel = null;
+                    bool imageExists = false;
+
                     //Copy image file to new workbook if target worksheet is in a different workbook.
-                    if (worksheet.Workbook != _drawings.Worksheet.Workbook)
+                    if (targetWorkbook != _drawings.Worksheet.Workbook)
                     {
-                        var uri = UriHelper.ResolvePartUri(rel.SourceUri, rel.TargetUri);
+                        var uri = UriHelper.ResolvePartUri(srcsRel.SourceUri, srcsRel.TargetUri);
                         var imagePart = _drawings.Worksheet.Workbook._package.ZipPackage.GetPart(uri);
+
                         var imageStream = (MemoryStream)imagePart.GetStream(FileMode.Open, FileAccess.Read);
                         var image = new byte[imageStream.Length];
+
                         imageStream.Seek(0, SeekOrigin.Begin);
                         imageStream.Read(image, 0, (int)imageStream.Length);
-                        var imageInfo = worksheet.Workbook._package.PictureStore.GetImageInfo(image);
+
+                        var imageInfo = targetPackage.PictureStore.GetImageInfo(image);
+
                         if (imageInfo == null)
                         {
-                            var copyPart = worksheet.Workbook._package.ZipPackage.CreatePart(uri, imagePart.ContentType);
+                            var info = new FileInfo(uri.OriginalString);
+                            Uri absUri = GetNewUri(targetPackage.ZipPackage, "/xl/media/image{0}" + info.Extension);
+
+                            newRel = targetWorksheet._drawings.Part.CreateRelationshipFromCopy(srcsRel);
+
+                            var relativeUri = UriHelper.GetRelativeUri(newRel.SourceUri, absUri);
+                            newRel.TargetUri = relativeUri;
+
+                            var copyPart = targetPackage.ZipPackage.CreatePart(absUri, imagePart.ContentType);
                             var copyStream = (MemoryStream)copyPart.GetStream(FileMode.Create, FileAccess.Write);
                             copyStream.Write(image, 0, image.Length);
+
+                            relNode.Value = newRel.Id;
                         }
                         else
                         {
-                            rel.TargetUri = imageInfo.Uri;
+                            var relativeUri = UriHelper.GetRelativeUri(srcsRel.SourceUri, imageInfo.Uri);
+                            var exisistingRel = targetWorksheet._drawings.Part.GetRelationshipsByType(srcsRel.RelationshipType).Where(x => x.TargetUri == relativeUri).FirstOrDefault();
+                            relNode.Value = exisistingRel.Id;
                         }
                     }
-                    //Check if relationship exists.
-                    var exisistingRel = worksheet._drawings.Part.GetRelationshipsByType(rel.RelationshipType).Where(x => x.Target == rel.Target).FirstOrDefault();
-                    //Create new relation id if no relation exsist or if it's a different worksheet. Otherwise asign the exsisting relationship Id
-                    if (exisistingRel == null || worksheet != _drawings.Worksheet)
-                    {
-                        var newRel = worksheet._drawings.Part.CreateRelationshipFromCopy(rel);
-                        relNode.Value = newRel.Id;
-                    }
                     else
                     {
-                        relNode.Value = exisistingRel.Id;
+                        //Check if relationship exists.
+                        var exisistingRel = targetWorksheet._drawings.Part.GetRelationshipsByType(srcsRel.RelationshipType).Where(x => x.TargetUri == srcsRel.TargetUri).FirstOrDefault();
+                        //Create new relation id if no relation exsist or if it's a different worksheet. Otherwise asign the existing relationship Id
+                        if (exisistingRel == null || targetWorksheet != _drawings.Worksheet)
+                        {
+                            newRel = targetWorksheet._drawings.Part.CreateRelationshipFromCopy(srcsRel);
+                            relNode.Value = newRel.Id;
+                        }
+                        else
+                        {
+                            relNode.Value = exisistingRel.Id;
+                        }
                     }
                 }
             }
             if (!isGroupShape)
             {
                 //Set New id on copied picture.
-                var pic = GetDrawing(worksheet._drawings, drawNode) as ExcelPicture;
-                pic.SetNewId(++worksheet.Workbook._nextDrawingId);
-                pic.Name = worksheet._drawings.GetUniqueDrawingName(this.Name);
+                var pic = GetDrawing(targetWorksheet._drawings, drawNode) as ExcelPicture;
+                pic.SetNewId(++targetWorkbook._nextDrawingId);
+                pic.Name = targetWorksheet._drawings.GetUniqueDrawingName(this.Name);
             }
             return drawNode;
         }

src/EPPlus/Drawing/ExcelPicture.cs

@@ -27,6 +27,8 @@ Date               Author                       Change
 
 #if NETFULL
 using System.Drawing.Imaging;
+using System.Xml.Linq;
+
 #endif
 #if !NET35 && !NET40
 using System.Threading.Tasks;
@@ -133,6 +135,13 @@ private void Init()
 			_verticalFlipPath = string.Format(_verticalFlipPath, _topPath);
 		}
 
+        internal string GetRelId()
+        {
+            XmlElement blip = (XmlElement)TopNode.SelectSingleNode($"{_topPath}xdr:blipFill/a:blip", NameSpaceManager);
+
+            return blip.GetAttribute("embed", ExcelPackage.schemaRelationships);
+        }
+
 		internal void SetRelId(XmlNode node, ePictureType type, string relID, string attribute = "embed")
         {
             XmlElement blip = (XmlElement)node.SelectSingleNode($"{_topPath}xdr:blipFill/a:blip", NameSpaceManager);

src/EPPlusTest/Drawing/CopyDrawingTests.cs

@@ -443,5 +443,229 @@ public void ReadAndCopyTwoAnchorImage()
                 SaveAndCleanup(package);
             }
         }
+
+        [TestMethod]
+        public void CopyImagesBetweenTwo_Unsaved_Workbooks()
+        {
+            string sheetName = "AWs";
+            string wbName = "CopyPicture_PictureObject";
+
+            using (var src = OpenPackage($"{wbName}1.xlsx", true))
+            {
+                var wbFirst = src.Workbook;
+                var wsFirst = wbFirst.Worksheets.Add(sheetName);
+
+                var pic1 = wsFirst.Drawings.AddPicture("epplusPicture", GetResourceFile("EPPlus.png"));
+
+                using (var target = OpenPackage($"{wbName}2.xlsx", true))
+                {
+                    var wbSecond = target.Workbook;
+                    var wsSecond = wbSecond.Worksheets.Add(sheetName);
+
+                    var pic2 = wsSecond.Drawings.AddPicture("screenshotPicture", GetResourceFile("screenshot.PNG"));
+                    pic1.Copy(wsSecond, 1, 1, 0, 0);
+                    pic2.Copy(wsFirst, 1, 1, 0, 0);
+
+                    Assert.AreEqual(2, wsSecond.Drawings.Count);
+                    Assert.AreEqual(2, wsFirst.Drawings.Count);
+
+                    Assert.AreEqual("epplusPicture", wsSecond.Drawings[1].Name);
+                    Assert.AreEqual("screenshotPicture", wsFirst.Drawings[1].Name);
+
+                    //Check that we don't create new rels as image already exists in target
+                    Assert.AreEqual(2, wsFirst.Drawings.Part._rels.Count);
+                    Assert.AreEqual(2, wsSecond.Drawings.Part._rels.Count);
+
+                    SaveAndCleanup(target);
+                }
+                SaveAndCleanup(src);
+            }
+        }
+
+        [TestMethod]
+        public void CopyImagesBetweenTwo_UnsavedWorkbooks_NamedRanges()
+        {
+            string sheetName = "AWs";
+            string rangeName = "SomeName";
+            string rangeAddress = "A1:G9";
+            string wbName = "CopyPicture_NamedRanges";
+
+            using (var src = OpenPackage($"{wbName}1.xlsx", true))
+            {
+                var wbFirst = src.Workbook;
+                var wsFirst = wbFirst.Worksheets.Add(sheetName);
+
+                var pic1 = wsFirst.Drawings.AddPicture("epplusPicture", GetResourceFile("EPPlus.png"));
+
+                wsFirst.Names.AddName(rangeName, wsFirst.Cells[rangeAddress]);
+
+                using (var target = OpenPackage($"{wbName}2.xlsx", true))
+                {
+                    var wbSecond = target.Workbook;
+                    var wsSecond = wbSecond.Worksheets.Add(sheetName);
+
+                    var pic2 = wsSecond.Drawings.AddPicture("screenshotPicture", GetResourceFile("screenshot.PNG"));
+                    wsSecond.Names.AddName(rangeName, wsSecond.Cells[rangeAddress]);
+
+                    CopyImagesBetweenWorkbooks(src, target, sheetName, rangeName);
+
+                    //ensure "normal" copying works
+                    Assert.AreEqual(2, wsSecond.Drawings.Count);
+                    Assert.AreEqual("screenshotPicture", wsSecond.Drawings[0].Name);
+                    Assert.AreEqual("epplusPicture", wsSecond.Drawings[1].Name);
+
+                    CopyImagesBetweenWorkbooks(target, src, sheetName, rangeName);
+
+                    //Ensure nothing's changed in the workbook we are copying FROM when copying back
+                    Assert.AreEqual(2, wsSecond.Drawings.Count);
+                    Assert.AreEqual("screenshotPicture", wsSecond.Drawings[0].Name);
+                    Assert.AreEqual("epplusPicture", wsSecond.Drawings[1].Name);
+
+                    //Copies the copy therefore 3
+                    Assert.AreEqual(3, wsFirst.Drawings.Count);
+                    Assert.AreEqual("epplusPicture", wsFirst.Drawings[0].Name);
+                    Assert.AreEqual("screenshotPicture", wsFirst.Drawings[1].Name);
+                    Assert.AreEqual("epplusPicture1", wsFirst.Drawings[2].Name);
+
+                    //Check that we don't create new rels as image already exists in target
+                    Assert.AreEqual(2, wsFirst.Drawings.Part._rels.Count);
+                    Assert.AreEqual(2, wsSecond.Drawings.Part._rels.Count);
+
+                    //Ensure no missmatch of ids
+                    var relIdOriginal = wsFirst.Drawings[0].As.Picture.GetRelId();
+                    var relIdCopiedBack = wsFirst.Drawings[2].As.Picture.GetRelId();
+
+                    Assert.AreEqual(relIdOriginal, relIdCopiedBack);
+                    Assert.AreEqual("rId1", relIdOriginal);
+                    Assert.AreEqual("rId2", wsFirst.Drawings[1].As.Picture.GetRelId());
+
+                    SaveAndCleanup(target);
+                }
+                SaveAndCleanup(src);
+            }
+        }
+
+        [TestMethod]
+        public void CopyImagesBetweenTwo_Saved_Workbooks_NamedRanges()
+        {
+            string sheetName = "AWs";
+            string rangeName = "SomeName";
+            string rangeAddress = "A1:G9";
+            string wbName = "CopyPictureRead_NamedRanges";
+
+            //Create the workbooks
+            using (var src = OpenPackage($"{wbName}1.xlsx", true))
+            {
+                var wbFirst = src.Workbook;
+                var wsFirst = wbFirst.Worksheets.Add(sheetName);
+
+                var pic1 = wsFirst.Drawings.AddPicture("epplusPicture", GetResourceFile("EPPlus.png"));
+
+                wsFirst.Names.AddName(rangeName, wsFirst.Cells[rangeAddress]);
+
+                SaveAndCleanup(src);
+            }
+            using (var target = OpenPackage($"{wbName}2.xlsx", true))
+            {
+                var wbSecond = target.Workbook;
+                var wsSecond = wbSecond.Worksheets.Add(sheetName);
+
+                var pic2 = wsSecond.Drawings.AddPicture("screenshotPicture", GetResourceFile("screenshot.PNG"));
+                wsSecond.Names.AddName(rangeName, wsSecond.Cells[rangeAddress]);
+
+                SaveAndCleanup(target);
+            }
+
+            //Read and copy images between the workbooks
+            using (var src = OpenPackage($"{wbName}1.xlsx"))
+            {
+                var wbFirst = src.Workbook;
+                var wsFirst = wbFirst.Worksheets.First();
+
+                using (var target = OpenPackage($"{wbName}2.xlsx"))
+                {
+                    var wbSecond = target.Workbook;
+                    var wsSecond = wbSecond.Worksheets.First();
+
+                    Assert.AreEqual(1, wsSecond.Drawings.Count);
+                    Assert.AreEqual(1, wsFirst.Drawings.Count);
+
+                    CopyImagesBetweenWorkbooks(src, target, sheetName, rangeName);
+                    CopyImagesBetweenWorkbooks(target, src, sheetName, rangeName);
+
+                    Assert.AreEqual(2, wsSecond.Drawings.Count);
+                    //Copies the copy therefore 3
+                    Assert.AreEqual(3, wsFirst.Drawings.Count);
+
+                    Assert.AreEqual("epplusPicture", wsSecond.Drawings[1].Name);
+                    Assert.AreEqual("screenshotPicture", wsFirst.Drawings[1].Name);
+
+                    //Assert.AreEqual("1", wsFirst.Drawings[2].As.Picture.Part._rels["0"].Id);
+
+                    var outputName = GetOutputFile("", "copy_" + target.File.Name).FullName;
+                    target.SaveAs(outputName);
+                }
+                var outputName2 = GetOutputFile("", "copy_" + src.File.Name).FullName;
+                src.SaveAs(outputName2);
+            }
+        }
+
+        [TestMethod]
+        public void CopyImages_Read_EnsureWorkaround()
+        {
+            using (var src = OpenTemplatePackage("ImageInRange1.xlsx"))
+            {
+                var wb = src.Workbook;
+                using (var target = OpenTemplatePackage("ImageInRange2.xlsx"))
+                {
+                    var wb2 = target.Workbook;
+
+                    CopyNamedRangeToTargetSheetWB(src, target, "sheet1", "SomeName");
+
+                    var outputName = GetOutputFile("", "alt_" + target.File.Name).FullName;
+                    target.SaveAs(outputName);
+                }
+                var outputName2 = GetOutputFile("", "alt_" + src.File.Name).FullName;
+                src.SaveAs(outputName2);
+            }
+        }
+
+        [TestMethod]
+        public void CopyImages_Read()
+        {
+            using (var src = OpenTemplatePackage("ImageInRange1.xlsx"))
+            {
+                var wb = src.Workbook;
+                using (var target = OpenTemplatePackage("ImageInRange2.xlsx"))
+                {
+                    var wb2 = target.Workbook;
+
+                    CopyImagesBetweenWorkbooks(src, target, "sheet1", "SomeName");
+
+                    SaveAndCleanup(target);
+                }
+                SaveAndCleanup(src);
+            }
+        }
+
+        //From i1841 / s814
+        public void CopyImagesBetweenWorkbooks(ExcelPackage src, ExcelPackage target, string sheetName, string namedRange)
+        {
+            var range = src.Workbook.Worksheets[sheetName].Names[namedRange];
+            var targetRange = target.Workbook.Worksheets[sheetName].Names[namedRange];
+            range.Copy(targetRange);
+        }
+
+        public void CopyNamedRangeToTargetSheetWB(ExcelPackage src, ExcelPackage target, string sheetName, string namedRange)
+        {
+            var srcWs = src.Workbook.Worksheets[sheetName];
+            var tmpWs = target.Workbook.Worksheets.Add("tmpCopy", srcWs);
+
+            var range = tmpWs.Names[namedRange];
+            var targetRange = target.Workbook.Worksheets[sheetName].Names[namedRange];
+            range.Copy(targetRange);
+
+            target.Workbook.Worksheets.Delete(tmpWs);
+        }
     }
 }

src/EPPlusTest/Drawing/DrawingTest.cs

@@ -39,6 +39,9 @@ Date               Author                       Change
 using System.Diagnostics;
 using System.Reflection;
 using OfficeOpenXml.Drawing.Theme;
+using System.Linq;
+using System.Collections.Generic;
+using System.Collections;
 
 namespace EPPlusTest
 {