This closes qax-os#2059, support delete one cell anchor image

- Fix delete wrong images in some case which caused by image reference detection issue
- Update unit tests

Author: xuri

chart_test.go

@@ -124,6 +124,11 @@ func TestDeleteDrawing(t *testing.T) {
 	assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
 	f, err = OpenFile(filepath.Join("test", "Book1.xlsx"))
 	assert.NoError(t, err)
+	f.Drawings.Store(path, &xlsxWsDr{OneCellAnchor: []*xdrCellAnchor{{
+		GraphicFrame: string(MacintoshCyrillicCharset),
+	}}})
+	_, err = f.deleteDrawing(0, 0, path, "Chart")
+	assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
 	f.Drawings.Store(path, &xlsxWsDr{TwoCellAnchor: []*xdrCellAnchor{{
 		GraphicFrame: string(MacintoshCyrillicCharset),
 	}}})

drawing.go

@@ -1484,13 +1484,14 @@ func (f *File) addSheetDrawingChart(drawingXML string, rID int, opts *GraphicOpt
 // deleteDrawing provides a function to delete the chart graphic frame and
 // returns deleted embed relationships ID (for unique picture cell anchor) by
 // given coordinates and graphic type.
-func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) (string, error) {
+func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) ([]string, error) {
 	var (
-		err             error
-		rID             string
-		rIDs            []string
-		wsDr            *xlsxWsDr
-		deTwoCellAnchor *decodeCellAnchor
+		err            error
+		rID            string
+		delRID, refRID []string
+		rIDMaps        = map[string]int{}
+		wsDr           *xlsxWsDr
+		deCellAnchor   *decodeCellAnchor
 	)
 	xdrCellAnchorFuncs := map[string]func(anchor *xdrCellAnchor) bool{
 		"Chart": func(anchor *xdrCellAnchor) bool { return anchor.Pic == nil },
@@ -1502,54 +1503,70 @@ func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) (stri
 	}
 	onAnchorCell := func(c, r int) bool { return c == col && r == row }
 	if wsDr, _, err = f.drawingParser(drawingXML); err != nil {
-		return rID, err
-	}
-	for idx := 0; idx < len(wsDr.TwoCellAnchor); idx++ {
-		if err = nil; wsDr.TwoCellAnchor[idx].From != nil && xdrCellAnchorFuncs[drawingType](wsDr.TwoCellAnchor[idx]) {
-			if onAnchorCell(wsDr.TwoCellAnchor[idx].From.Col, wsDr.TwoCellAnchor[idx].From.Row) {
-				rID, _ = extractEmbedRID(wsDr.TwoCellAnchor[idx].Pic, nil, rIDs)
-				wsDr.TwoCellAnchor = append(wsDr.TwoCellAnchor[:idx], wsDr.TwoCellAnchor[idx+1:]...)
-				idx--
+		return delRID, err
+	}
+	deleteCellAnchor := func(ca []*xdrCellAnchor) ([]*xdrCellAnchor, error) {
+		for idx := 0; idx < len(ca); idx++ {
+			if err = nil; ca[idx].From != nil && xdrCellAnchorFuncs[drawingType](ca[idx]) {
+				rID = extractEmbedRID(ca[idx].Pic, nil)
+				rIDMaps[rID]++
+				if onAnchorCell(ca[idx].From.Col, ca[idx].From.Row) {
+					refRID = append(refRID, rID)
+					ca = append(ca[:idx], ca[idx+1:]...)
+					idx--
+					rIDMaps[rID]--
+				}
 				continue
 			}
-			_, rIDs = extractEmbedRID(wsDr.TwoCellAnchor[idx].Pic, nil, rIDs)
-		}
-	}
-	for idx := 0; idx < len(wsDr.TwoCellAnchor); idx++ {
-		deTwoCellAnchor = new(decodeCellAnchor)
-		if err = f.xmlNewDecoder(strings.NewReader("<decodeCellAnchor>" + wsDr.TwoCellAnchor[idx].GraphicFrame + "</decodeCellAnchor>")).
-			Decode(deTwoCellAnchor); err != nil && err != io.EOF {
-			return rID, err
-		}
-		if err = nil; deTwoCellAnchor.From != nil && decodeCellAnchorFuncs[drawingType](deTwoCellAnchor) {
-			if onAnchorCell(deTwoCellAnchor.From.Col, deTwoCellAnchor.From.Row) {
-				rID, _ = extractEmbedRID(nil, deTwoCellAnchor.Pic, rIDs)
-				wsDr.TwoCellAnchor = append(wsDr.TwoCellAnchor[:idx], wsDr.TwoCellAnchor[idx+1:]...)
-				idx--
-				continue
+			deCellAnchor = new(decodeCellAnchor)
+			if err = f.xmlNewDecoder(strings.NewReader("<decodeCellAnchor>" + ca[idx].GraphicFrame + "</decodeCellAnchor>")).
+				Decode(deCellAnchor); err != nil && err != io.EOF {
+				return ca, err
+			}
+			if err = nil; deCellAnchor.From != nil && decodeCellAnchorFuncs[drawingType](deCellAnchor) {
+				rID = extractEmbedRID(nil, deCellAnchor.Pic)
+				rIDMaps[rID]++
+				if onAnchorCell(deCellAnchor.From.Col, deCellAnchor.From.Row) {
+					refRID = append(refRID, rID)
+					ca = append(ca[:idx], ca[idx+1:]...)
+					idx--
+					rIDMaps[rID]--
+				}
 			}
-			_, rIDs = extractEmbedRID(nil, deTwoCellAnchor.Pic, rIDs)
 		}
+		return ca, err
 	}
-	if inStrSlice(rIDs, rID, true) != -1 {
-		rID = ""
+	if wsDr.OneCellAnchor, err = deleteCellAnchor(wsDr.OneCellAnchor); err != nil {
+		return delRID, err
+	}
+	if wsDr.TwoCellAnchor, err = deleteCellAnchor(wsDr.TwoCellAnchor); err != nil {
+		return delRID, err
 	}
 	f.Drawings.Store(drawingXML, wsDr)
-	return rID, err
+	return getUnusedCellAnchorRID(delRID, refRID, rIDMaps), err
 }
 
-// extractEmbedRID returns embed relationship ID and all relationship ID lists
-// for giving cell anchor.
-func extractEmbedRID(pic *xlsxPic, decodePic *decodePic, rIDs []string) (string, []string) {
+// extractEmbedRID returns embed relationship ID by giving cell anchor.
+func extractEmbedRID(pic *xlsxPic, decodePic *decodePic) string {
+	var rID string
 	if pic != nil {
-		rIDs = append(rIDs, pic.BlipFill.Blip.Embed)
-		return pic.BlipFill.Blip.Embed, rIDs
+		rID = pic.BlipFill.Blip.Embed
 	}
 	if decodePic != nil {
-		rIDs = append(rIDs, decodePic.BlipFill.Blip.Embed)
-		return decodePic.BlipFill.Blip.Embed, rIDs
+		rID = decodePic.BlipFill.Blip.Embed
+	}
+	return rID
+}
+
+// getUnusedCellAnchorRID returns relationship ID lists in the cell anchor which
+// for remove.
+func getUnusedCellAnchorRID(delRID, refRID []string, rIDMaps map[string]int) []string {
+	for _, rID := range refRID {
+		if rIDMaps[rID] == 0 && inStrSlice(delRID, rID, false) == -1 {
+			delRID = append(delRID, rID)
+		}
 	}
-	return "", rIDs
+	return delRID
 }
 
 // deleteDrawingRels provides a function to delete relationships in

picture.go

@@ -566,36 +566,41 @@ func (f *File) DeletePicture(sheet, cell string) error {
 	}
 	drawingXML := strings.ReplaceAll(f.getSheetRelationshipsTargetByID(sheet, ws.Drawing.RID), "..", "xl")
 	drawingRels := "xl/drawings/_rels/" + filepath.Base(drawingXML) + ".rels"
-	rID, err := f.deleteDrawing(col, row, drawingXML, "Pic")
+	rIDs, err := f.deleteDrawing(col, row, drawingXML, "Pic")
 	if err != nil {
 		return err
 	}
-	rels := f.getDrawingRelationships(drawingRels, rID)
-	if rels == nil {
-		return err
-	}
-	var used bool
-	checkPicRef := func(k, v interface{}) bool {
-		if strings.Contains(k.(string), "xl/drawings/_rels/drawing") {
-			r, err := f.relsReader(k.(string))
-			if err != nil {
-				return true
-			}
-			for _, rel := range r.Relationships {
-				if rel.ID != rels.ID && rel.Type == SourceRelationshipImage &&
-					filepath.Base(rel.Target) == filepath.Base(rels.Target) {
-					used = true
+	for _, rID := range rIDs {
+		rels := f.getDrawingRelationships(drawingRels, rID)
+		if rels == nil {
+			return err
+		}
+		var used bool
+		checkPicRef := func(k, v interface{}) bool {
+			if strings.Contains(k.(string), "xl/drawings/_rels/drawing") {
+				if k.(string) == drawingRels {
+					return true
+				}
+				r, err := f.relsReader(k.(string))
+				if err != nil {
+					return true
+				}
+				for _, rel := range r.Relationships {
+					if rel.Type == SourceRelationshipImage &&
+						filepath.Base(rel.Target) == filepath.Base(rels.Target) {
+						used = true
+					}
 				}
 			}
+			return true
 		}
-		return true
-	}
-	f.Relationships.Range(checkPicRef)
-	f.Pkg.Range(checkPicRef)
-	if !used {
-		f.Pkg.Delete(strings.Replace(rels.Target, "../", "xl/", -1))
+		f.Relationships.Range(checkPicRef)
+		f.Pkg.Range(checkPicRef)
+		if !used {
+			f.Pkg.Delete(strings.Replace(rels.Target, "../", "xl/", -1))
+		}
+		f.deleteDrawingRels(drawingRels, rID)
 	}
-	f.deleteDrawingRels(drawingRels, rID)
 	return err
 }
 

picture_test.go

@@ -334,9 +334,9 @@ func TestDeletePicture(t *testing.T) {
 	f, err = OpenFile(filepath.Join("test", "TestDeletePicture.xlsx"))
 	assert.NoError(t, err)
 	// Test delete same picture on different worksheet, the images should be removed
-	assert.NoError(t, f.DeletePicture("Sheet1", "F10"))
-	assert.NoError(t, f.DeletePicture("Sheet2", "F1"))
+	assert.NoError(t, f.DeletePicture("Sheet1", "F20"))
 	assert.NoError(t, f.DeletePicture("Sheet1", "I20"))
+	assert.NoError(t, f.DeletePicture("Sheet2", "F1"))
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestDeletePicture2.xlsx")))
 
 	// Test delete picture on not exists worksheet
@@ -364,6 +364,14 @@ func TestDeletePicture(t *testing.T) {
 	assert.NoError(t, f.DeletePicture("Sheet2", "F1"))
 	assert.NoError(t, f.Close())
 
+	f, err = OpenFile(filepath.Join("test", "TestDeletePicture.xlsx"))
+	assert.NoError(t, err)
+	// Test delete picture without drawing relationships
+	f.Relationships.Delete("xl/drawings/_rels/drawing1.xml.rels")
+	f.Pkg.Delete("xl/drawings/_rels/drawing1.xml.rels")
+	assert.NoError(t, f.DeletePicture("Sheet1", "I20"))
+	assert.NoError(t, f.Close())
+
 	f = NewFile()
 	assert.NoError(t, err)
 	assert.NoError(t, f.AddPicture("Sheet1", "A1", filepath.Join("test", "images", "excel.jpg"), nil))