From d5a4996d2514dc2737d57aab084bc5743e040d20 Mon Sep 17 00:00:00 2001
From: "Zheng, Lei" <realthunder.dev@gmail.com>
Date: Mon, 16 Dec 2019 11:34:52 +0800
Subject: [PATCH] App: fix Expression _moveCells()

The problem is caused by not refreshing ObjectIdentifier internal cache
after change.
---
 src/App/Expression.cpp                 | 10 +++++----
 src/App/ObjectIdentifier.cpp           | 29 ++++++++++++++++++--------
 src/App/ObjectIdentifier.h             |  5 +++--
 src/Mod/Spreadsheet/TestSpreadsheet.py |  3 +++
 4 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/App/Expression.cpp b/src/App/Expression.cpp
index b91435c7538e..46d0c01ac939 100644
--- a/src/App/Expression.cpp
+++ b/src/App/Expression.cpp
@@ -2716,7 +2716,8 @@ void VariableExpression::_moveCells(const CellAddress &address,
     if(var.hasDocumentObjectName(true))
         return;
 
-    auto &comp = var.getPropertyComponent(0);
+    int idx = 0;
+    const auto &comp = var.getPropertyComponent(0,&idx);
     CellAddress addr = stringToAddress(comp.getName().c_str(),true);
     if(!addr.isValid())
         return;
@@ -2727,7 +2728,7 @@ void VariableExpression::_moveCells(const CellAddress &address,
         v.aboutToChange();
         addr.setRow(thisRow + rowCount);
         addr.setCol(thisCol + colCount);
-        comp = ObjectIdentifier::SimpleComponent(addr.toString());
+        var.setComponent(idx,ObjectIdentifier::SimpleComponent(addr.toString()));
     }
 }
 
@@ -2735,7 +2736,8 @@ void VariableExpression::_offsetCells(int rowOffset, int colOffset, ExpressionVi
     if(var.hasDocumentObjectName(true))
         return;
 
-    auto &comp = var.getPropertyComponent(0);
+    int idx = 0;
+    const auto &comp = var.getPropertyComponent(0,&idx);
     CellAddress addr = stringToAddress(comp.getName().c_str(),true);
     if(!addr.isValid() || (addr.isAbsoluteCol() && addr.isAbsoluteRow()))
         return;
@@ -2745,7 +2747,7 @@ void VariableExpression::_offsetCells(int rowOffset, int colOffset, ExpressionVi
         addr.setCol(addr.col()+colOffset);
     if(!addr.isAbsoluteRow())
         addr.setRow(addr.row()+rowOffset);
-    comp = ObjectIdentifier::SimpleComponent(addr.toString());
+    var.setComponent(idx,ObjectIdentifier::SimpleComponent(addr.toString()));
 }
 
 void VariableExpression::setPath(const ObjectIdentifier &path)
diff --git a/src/App/ObjectIdentifier.cpp b/src/App/ObjectIdentifier.cpp
index ee215063450f..0ab0d86b69f9 100644
--- a/src/App/ObjectIdentifier.cpp
+++ b/src/App/ObjectIdentifier.cpp
@@ -185,25 +185,36 @@ std::string App::ObjectIdentifier::getPropertyName() const
 
 /**
  * @brief Get Component at given index \a i.
- * @param i Index to get
+ * @param i: Index to get
+ * @param idx: optional return of adjusted component index
  * @return A component.
  */
 
-const App::ObjectIdentifier::Component &App::ObjectIdentifier::getPropertyComponent(int i) const
+const App::ObjectIdentifier::Component &App::ObjectIdentifier::getPropertyComponent(int i, int *idx) const
 {
     ResolveResults result(*this);
 
-    assert(result.propertyIndex + i >=0 && static_cast<std::size_t>(result.propertyIndex) + i < components.size());
+    i += result.propertyIndex;
+    if (i < 0 || i >= static_cast<int>(components.size()))
+        FC_THROWM(Base::ValueError, "Invalid property component index");
 
-    return components[result.propertyIndex + i];
+    if (idx)
+        *idx = i;
+
+    return components[i];
 }
 
-App::ObjectIdentifier::Component &App::ObjectIdentifier::getPropertyComponent(int i)
+void App::ObjectIdentifier::setComponent(int idx, Component &&comp)
 {
-    ResolveResults result(*this);
-    assert(result.propertyIndex + i >=0 && 
-            static_cast<std::size_t>(result.propertyIndex) + i < components.size());
-    return components[result.propertyIndex + i];
+    if (idx < 0 || idx >= static_cast<int>(components.size()))
+        FC_THROWM(Base::ValueError, "Invalid component index");
+    components[idx] = std::move(comp);
+    _cache.clear();
+}
+
+void App::ObjectIdentifier::setComponent(int idx, const Component &comp)
+{
+    setComponent(idx, Component(comp));
 }
 
 std::vector<ObjectIdentifier::Component> ObjectIdentifier::getPropertyComponents() const {
diff --git a/src/App/ObjectIdentifier.h b/src/App/ObjectIdentifier.h
index f55fd885fd4a..e77bccd749df 100644
--- a/src/App/ObjectIdentifier.h
+++ b/src/App/ObjectIdentifier.h
@@ -292,9 +292,10 @@ class AppExport ObjectIdentifier {
     template<typename C>
     void addComponents(const C &cs) { components.insert(components.end(), cs.begin(), cs.end()); }
 
-    const Component & getPropertyComponent(int i) const;
+    const Component & getPropertyComponent(int i, int *idx=0) const;
 
-    Component & getPropertyComponent(int i);
+    void setComponent(int idx, Component &&comp);
+    void setComponent(int idx, const Component &comp);
 
     std::vector<Component> getPropertyComponents() const;
     const std::vector<Component> &getComponents() const { return components; }
diff --git a/src/Mod/Spreadsheet/TestSpreadsheet.py b/src/Mod/Spreadsheet/TestSpreadsheet.py
index c2968827e1ab..0f347f973eae 100644
--- a/src/Mod/Spreadsheet/TestSpreadsheet.py
+++ b/src/Mod/Spreadsheet/TestSpreadsheet.py
@@ -664,6 +664,9 @@ def testInsertRows(self):
         sheet = self.doc.addObject('Spreadsheet::Sheet','Spreadsheet')
         sheet.set('B1', '=B2')
         sheet.set('B2', '124')
+        # Calling getContents() here activates ObjectIdentifier internal cache,
+        # which needs to be tested as well.
+        self.assertEqual(sheet.getContents("B1"),"=B2")
         sheet.insertRows('2', 1)
         self.assertEqual(sheet.getContents("B1"),"=B3")