SC-66: Fix case when linked excel file is removed

YRobles · Feb 4, 2022 · bf50205 · bf50205
1 parent 4e512e2
commit bf50205
Show file tree

Hide file tree

Showing 16 changed files with 92 additions and 43 deletions.
diff --git a/LICENSE b/LICENSE
@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2019-2021 Adam Shakhabov
+Copyright (c) 2019-2022 Adam Shakhabov
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

diff --git a/ShapeCrawler.Tests.Unit/ChartPointTests.cs b/ShapeCrawler.Tests.Unit/ChartPointTests.cs
@@ -1,4 +1,5 @@
-using System.IO;
+using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using FluentAssertions;
 using ShapeCrawler.Charts;
@@ -86,28 +87,52 @@ public void Value_Getter_returns_chart_point()
             seriesPointValueCase5.Should().Be(3.2);
         }
 
-        [Fact]
-        public void Value_Setter_updates_chart_point()
+        [Theory]
+        [MemberData(nameof(TestCasesValueSetter))]
+        public void Value_Setter_updates_chart_point(string filename, int slideNumber, string shapeName)
         {
             // Arrange
-            var chart = this.GetShape<IChart>("024_chart.pptx", 3, 5);
+            var pptxStream = GetPptxStream(filename);
+            var presentation = SCPresentation.Open(pptxStream, true);
+            var chart = presentation.Slides[--slideNumber].Shapes.GetByName<IChart>(shapeName);
             var point = chart.SeriesCollection[0].Points[0];
-            const int newValue = 6;
+            const int newChartPointValue = 6;
 
             // Act
-            point.Value = newValue;
+            point.Value = newChartPointValue;
 
             // Assert
-            point.Value.Should().Be(newValue);
+            point.Value.Should().Be(newChartPointValue);
 
             var stream = new MemoryStream();
-            chart.ParentSlide.ParentPresentation.SaveAs(stream);
-            chart = this.GetShape<IChart>(stream, 3, 5);
-            var savedChartPoint = chart.SeriesCollection[0].Points[0];
-            savedChartPoint.Value.Should().Be(newValue);
+            presentation.SaveAs(stream);
+            chart = presentation.Slides[slideNumber].Shapes.GetByName<IChart>(shapeName);
+            point = chart.SeriesCollection[0].Points[0];
+            point.Value.Should().Be(newChartPointValue);
+        }
 
-            var pointCellValue = this.GetCellValue<double>(chart.WorkbookByteArray, "B2");
-            pointCellValue.Should().Be(newValue);
+        public static IEnumerable<object[]> TestCasesValueSetter()
+        {
+            yield return new object[] {"024_chart.pptx", 3, "Chart 4"};
+            yield return new object[] {"009_table.pptx", 3, "Chart 5"};
+        }
+
+        [Fact]
+        public void Value_Setter_updates_chart_point_in_Embedded_excel_workbook()
+        {
+            // Arrange
+            var pptxStream = GetPptxStream("024_chart.pptx");
+            var presentation = SCPresentation.Open(pptxStream, true);
+            var chart = presentation.Slides[2].Shapes.GetById<IChart>(5);
+            var point = chart.SeriesCollection[0].Points[0];
+            const int newChartPointValue = 6;
+
+            // Act
+            point.Value = newChartPointValue;
+
+            // Assert
+            var pointCellValue = this.GetWorksheetCellValue<double>(chart.WorkbookByteArray, "B2");
+            pointCellValue.Should().Be(newChartPointValue);
         }
     }
 }
diff --git a/ShapeCrawler.Tests.Unit/Resource/009_table.pptx b/ShapeCrawler.Tests.Unit/Resource/009_table.pptx
diff --git a/ShapeCrawler.Tests.Unit/ShapeCrawlerTest.cs b/ShapeCrawler.Tests.Unit/ShapeCrawlerTest.cs
@@ -36,7 +36,7 @@ protected IAutoShape GetAutoShape(string presentation, int slideNumber, int shap
             return (IAutoShape) shape;
         }
 
-        protected T GetCellValue<T>(byte[] workbookByteArray, string cellAddress)
+        protected T GetWorksheetCellValue<T>(byte[] workbookByteArray, string cellAddress)
         {
             var stream = new MemoryStream(workbookByteArray);
             var xlWorkbook = new XLWorkbook(stream);

diff --git a/ShapeCrawler.sln.DotSettings b/ShapeCrawler.sln.DotSettings
@@ -8,6 +8,7 @@
 	<s:String x:Key="/Default/CodeStyle/Generate/=Implementations/Options/=Mutable/@EntryIndexedValue">False</s:String>
 	<s:Boolean x:Key="/Default/CodeStyle/Naming/CSharpAutoNaming/IsNotificationDisabled/@EntryValue">True</s:Boolean>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=SC/@EntryIndexedValue">SC</s:String>
+	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="False" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FCONSTANT/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/JavaScriptNaming/UserRules/=JS_005FBLOCK_005FSCOPE_005FFUNCTION/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>

diff --git a/ShapeCrawler/Charts/ChartWorkbook.cs b/ShapeCrawler/Charts/ChartWorkbook.cs
@@ -11,11 +11,15 @@ internal class ChartWorkbook // TODO: implement IDispose to correctly dispose _p
         private readonly SCChart chart;
         private readonly Lazy<SpreadsheetDocument> spreadsheetDocument;
         private Stream embeddedPackagePartStream;
+
+        private EmbeddedPackagePart embeddedPackagePart;
+
         private bool closed;
 
-        internal ChartWorkbook(SCChart chart)
+        internal ChartWorkbook(SCChart chart, EmbeddedPackagePart embeddedPackagePart)
         {
             this.chart = chart;
+            this.embeddedPackagePart = embeddedPackagePart;
             this.spreadsheetDocument = new Lazy<SpreadsheetDocument>(this.GetSpreadsheetDocument);
         }
 
@@ -46,7 +50,7 @@ internal X.Cell GetXCell(string sheetName, string cellAddress)
 
         private SpreadsheetDocument GetSpreadsheetDocument()
         {
-            this.embeddedPackagePartStream = this.chart.SdkChartPart.EmbeddedPackagePart.GetStream();
+            this.embeddedPackagePartStream = this.embeddedPackagePart.GetStream();
             var spreadsheetDocument = SpreadsheetDocument.Open(this.embeddedPackagePartStream, this.chart.PresentationInternal.Editable);
             this.chart.PresentationInternal.ChartWorkbooks.Add(this);
 

diff --git a/ShapeCrawler/Charts/IChartPoint.cs b/ShapeCrawler/Charts/IChartPoint.cs
@@ -73,13 +73,17 @@ private double GetValue()
 
         private void UpdateValue(double value)
         {
-            // Try update cache
             if (this.cNumericValue != null)
             {
                 this.cNumericValue.Text = value.ToString(CultureInfo.InvariantCulture);
             }
 
-            // Update spreadsheet
+            if (this.parentChart.ChartWorkbook == null)
+            {
+                // Chart can have Linked file instead of Embedded. This Linked file can be removed
+                return;
+            }
+
             var xCell = this.parentChart.ChartWorkbook.GetXCell(this.sheetName, this.address);
             xCell.DataType = new EnumValue<X.CellValues>(X.CellValues.Number);
             xCell.CellValue = new X.CellValue(value);

diff --git a/ShapeCrawler/Charts/IChartPointCollection.cs b/ShapeCrawler/Charts/IChartPointCollection.cs
@@ -1,7 +1,5 @@
-using System;
-using System.Collections;
+using System.Collections;
 using System.Collections.Generic;
-using System.Globalization;
 using System.Linq;
 using System.Text.RegularExpressions;
 using DocumentFormat.OpenXml;

diff --git a/ShapeCrawler/Charts/SCChart.cs b/ShapeCrawler/Charts/SCChart.cs
@@ -27,14 +27,13 @@ internal class SCChart : SlideShape, IChart
 
         internal SCChart(P.GraphicFrame pGraphicFrame, SCSlide parentSlideInternal)
             : base(pGraphicFrame, parentSlideInternal, null)
-        { 
+        {
             this.pGraphicFrame = pGraphicFrame;
             this.firstSeries = new Lazy<OpenXmlElement>(this.GetFirstSeries);
             this.xValues = new Lazy<LibraryCollection<double>>(this.GetXValues);
             this.seriesCollection = new Lazy<SeriesCollection>(() => Collections.SeriesCollection.Create(this, this.cXCharts));
             this.categories = new Lazy<ICategoryCollection>(() => CategoryCollection.Create(this, this.firstSeries.Value, this.Type));
             this.chartType = new Lazy<ChartType>(this.GetChartType);
-            this.ChartWorkbook = new ChartWorkbook(this);
 
             this.Init(); // TODO: convert to lazy loading
         }
@@ -61,7 +60,7 @@ public bool HasTitle
             }
         }
 
-        public bool HasCategories => categories.Value != null;
+        public bool HasCategories => this.categories.Value != null;
 
         public ISeriesCollection SeriesCollection => this.seriesCollection.Value;
 
@@ -88,9 +87,9 @@ public LibraryCollection<double> XValues
 
         #endregion Public Properties
 
-        internal ChartWorkbook ChartWorkbook { get; }
+        internal ChartWorkbook? ChartWorkbook { get; set; }
 
-        internal ChartPart SdkChartPart { get; private set; }
+        internal ChartPart ChartPart { get; private set; }
 
         private void Init()
         {
@@ -99,10 +98,12 @@ private void Init()
                 .GetFirstChild<C.ChartReference>();
 
             var slide = this.Slide;
-            this.SdkChartPart = (ChartPart)slide.SlidePart.GetPartById(cChartReference.Id);
+            this.ChartPart = (ChartPart)slide.SlidePart.GetPartById(cChartReference.Id);
 
-            C.PlotArea cPlotArea = this.SdkChartPart.ChartSpace.GetFirstChild<C.Chart>().PlotArea;
+            C.PlotArea cPlotArea = this.ChartPart.ChartSpace.GetFirstChild<C.Chart>().PlotArea;
             this.cXCharts = cPlotArea.Where(e => e.LocalName.EndsWith("Chart", StringComparison.Ordinal));
+
+            this.ChartWorkbook = this.ChartPart.EmbeddedPackagePart != null ? new ChartWorkbook(this, this.ChartPart.EmbeddedPackagePart) : null;
         }
 
         private ChartType GetChartType()
@@ -120,7 +121,7 @@ private ChartType GetChartType()
 
         private string GetTitleOrDefault()
         {
-            C.Title cTitle = this.SdkChartPart.ChartSpace.GetFirstChild<C.Chart>().Title;
+            C.Title cTitle = this.ChartPart.ChartSpace.GetFirstChild<C.Chart>().Title;
             if (cTitle == null)
             {
                 // chart has not title

diff --git a/ShapeCrawler/Collections/IShapeCollection.cs b/ShapeCrawler/Collections/IShapeCollection.cs
@@ -35,5 +35,10 @@ public interface IShapeCollection : IEnumerable<IShape>
         /// <param name="yPixels">The Y coordinate for the left side of the shape.</param>
         /// <param name="videoStream">Video stream data.</param>
         IVideoShape AddNewVideo(int xPixel, int yPixels, Stream videoStream);
+
+        T GetById<T>(int shapeId)
+            where T : IShape;
+
+        T GetByName<T>(string shapeName);
     }
 }
diff --git a/ShapeCrawler/Collections/ShapeCollection.cs b/ShapeCrawler/Collections/ShapeCollection.cs
@@ -417,6 +417,19 @@ public IVideoShape AddNewVideo(int xPixels, int yPixels, Stream videoStream)
             return new VideoShape(this.slide, this.shapeTree);
         }
 
+        public T GetById<T>(int shapeId)
+            where T : IShape
+        {
+            var shape = this.CollectionItems.First(shape => shape.Id == shapeId);
+            return (T)shape;
+        }
+
+        public T GetByName<T>(string shapeName)
+        {
+            var shape = this.CollectionItems.First(shape => shape.Name == shapeName);
+            return (T)shape;
+        }
+
         public Shape? GetReferencedShapeOrDefault(P.PlaceholderShape inpPPlaceholderShape)
         {
             var collectionShapes = this.CollectionItems.Where(sp => sp.Placeholder != null).OfType<Shape>();

diff --git a/ShapeCrawler/OLEObjects/IOLEObject.cs b/ShapeCrawler/OLEObjects/IOLEObject.cs
@@ -1,6 +1,7 @@
 using ShapeCrawler.Shapes;
+// ReSharper disable CheckNamespace
 
-namespace ShapeCrawler.OLEObjects
+namespace ShapeCrawler
 {
     public interface IOLEObject : IShape
     {

diff --git a/ShapeCrawler/Shapes/IConnectionShape.cs b/ShapeCrawler/Shapes/IConnectionShape.cs
@@ -1,6 +1,8 @@
 using DocumentFormat.OpenXml;
+using ShapeCrawler.Shapes;
+// ReSharper disable CheckNamespace
 
-namespace ShapeCrawler.Shapes
+namespace ShapeCrawler
 {
     /// <summary>
     ///     Represents a connection shape.

diff --git a/ShapeCrawler/Shapes/IGroupShape.cs b/ShapeCrawler/Shapes/IGroupShape.cs
@@ -1,6 +1,8 @@
 using System.Collections.Generic;
+using ShapeCrawler.Shapes;
+// ReSharper disable CheckNamespace
 
-namespace ShapeCrawler.Shapes
+namespace ShapeCrawler
 {
     /// <summary>
     ///     Represents a group shape on a slide.

diff --git a/ShapeCrawler/Video/IVideoShape.cs b/ShapeCrawler/Video/IVideoShape.cs
@@ -2,6 +2,9 @@
 
 namespace ShapeCrawler.Video
 {
+    /// <summary>
+    ///     Represents a shape containing video content.
+    /// </summary>
     public interface IVideoShape : IShape
     {
         /// <summary>

diff --git a/net5-unit-tests.playlist b/net5-unit-tests.playlist