[PATCH libreoffice-4-0] fix basic access to nested uno structures fdo#60117

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/2099

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/99/2099/1

fix basic access to nested uno structures fdo#60117

Although basic might appear to correctly change nested struct elements
fdo#60117 shows that basic can be fooled ( and even the watch(ed) variable
in the debugger shows the expected values ). The problem stems from the
fact that the uno object held by basic isn't infact modified when changed
via the introspection uno service.

Additionally pimp the existing tests to check the actual uno struct to see
if the changes made are *really* reflected in the object

Change-Id: Iff007e17df87148ea81e69d3567c8cf9857fdcaa
---
M basic/qa/cppunit/test_nested_struct.cxx
M basic/source/classes/sbunoobj.cxx
2 files changed, 247 insertions(+), 8 deletions(-)



diff --git a/basic/qa/cppunit/test_nested_struct.cxx b/basic/qa/cppunit/test_nested_struct.cxx
index 21b4913..91978e3 100644
--- a/basic/qa/cppunit/test_nested_struct.cxx
+++ b/basic/qa/cppunit/test_nested_struct.cxx
@@ -12,24 +12,40 @@
 
 #include "basic/sbmod.hxx"
 #include "basic/sbmeth.hxx"
+#include "com/sun/star/awt/WindowDescriptor.hpp"
+#include "com/sun/star/table/TableBorder.hpp"
+#include "basic/sbuno.hxx"
+
 namespace
 {
+    using namespace com::sun::star;
     class Nested_Struct : public BasicTestBase
     {
         public:
         Nested_Struct() {};
         void testAssign1();
+        void testAssign1Alt(); // result is uno-ised and tested
         void testOldAssign();
+        void testOldAssignAlt(); // result is uno-ised and tested
         void testUnfixedVarAssign();
+        void testUnfixedVarAssignAlt(); // result is uno-ised and tested
         void testFixedVarAssign();
+        void testFixedVarAssignAlt(); // result is uno-ised and tested
+        void testUnoAccess(); // fdo#60117 specific test
+
         // Adds code needed to register the test suite
         CPPUNIT_TEST_SUITE(Nested_Struct);
 
         // Declares the method as a test to call
         CPPUNIT_TEST(testAssign1);
+        CPPUNIT_TEST(testAssign1Alt);
         CPPUNIT_TEST(testOldAssign);
+        CPPUNIT_TEST(testOldAssignAlt);
         CPPUNIT_TEST(testUnfixedVarAssign);
+        CPPUNIT_TEST(testUnfixedVarAssignAlt);
         CPPUNIT_TEST(testFixedVarAssign);
+        CPPUNIT_TEST(testFixedVarAssignAlt);
+        CPPUNIT_TEST(testUnoAccess);
 
         // End of test suite definition
         CPPUNIT_TEST_SUITE_END();
@@ -42,6 +58,14 @@
     "Dim b0 as new \"com.sun.star.table.TableBorder\"\n"
     "b0.HorizontalLine.OuterLineWidth = 9\n"
     "simpleNestStructAccess = b0.HorizontalLine.OuterLineWidth\n"
+    "End Function\n"
+);
+
+rtl::OUString sTestSource1Alt(
+    "Function simpleNestStructAccess() as Object\n"
+    "Dim b0 as new \"com.sun.star.table.TableBorder\"\n"
+    "b0.HorizontalLine.OuterLineWidth = 9\n"
+    "simpleNestStructAccess = b0\n"
     "End Function\n"
 );
 
@@ -61,6 +85,15 @@
 "End Function\n"
 );
 
+rtl::OUString sTestSource2Alt(
+    "Function simpleRegressionTestOld()\n"
+    "Dim b0 as new \"com.sun.star.table.TableBorder\", l as new 
\"com.sun.star.table.BorderLine\"\n"
+    "l = b0.HorizontalLine\n"
+    "l.OuterLineWidth = 9\n"
+    "b0.HorizontalLine = l\n"
+    "simpleRegressionTestOld = b0\n"
+"End Function\n"
+);
 // it should be legal to assign a variant to a struct ( and copy by val )
 // make sure we aren't copying by reference, we make sure that l is not
 // a reference copy of b0.HorizontalLine, each one should have an
@@ -77,6 +110,20 @@
 "End Function\n"
 );
 
+rtl::OUString sTestSource3Alt(
+    "Function testUnfixedVarAssign()\n"
+    "Dim b0 as new \"com.sun.star.table.TableBorder\"\n"
+    "l = b0.HorizontalLine\n"
+    "l.OuterLineWidth = 9\n"
+    "b0.HorizontalLine = l\n"
+    "l.OuterLineWidth = 4\n"
+    "Dim result(1)\n"
+    "result(0) = b0\n"
+    "result(1) = l\n"
+    "testUnfixedVarAssign = result\n"
+"End Function\n"
+);
+
 // nearly the same as above but this time for a fixed type
 // variable
 rtl::OUString sTestSource4(
@@ -89,6 +136,36 @@
     "testFixedVarAssign = b0.HorizontalLine.OuterLineWidth + l.OuterLineWidth\n"
 "End Function\n"
 );
+
+rtl::OUString sTestSource4Alt(
+    "Function testFixedVarAssign()\n"
+    "Dim b0 as new \"com.sun.star.table.TableBorder\", l as new 
\"com.sun.star.table.BorderLine\"\n"
+    "l = b0.HorizontalLine\n"
+    "l.OuterLineWidth = 9\n"
+    "b0.HorizontalLine = l\n"
+    "l.OuterLineWidth = 4\n"
+    "Dim result(1)\n"
+    "result(0) = b0\n"
+    "result(1) = l\n"
+    "testFixedVarAssign = result\n"
+"End Function\n"
+);
+
+// Although basic might appear to correctly change nested struct elements
+// fdo#60117 shows that basic can be fooled ( and even the watch(ed) variable
+// in the debugger shows the expected values )
+// We need to additionally check the actual uno struct to see if the
+// changes made are *really* reflected in the object
+rtl::OUString sTestSource5(
+    "Function testUnoAccess() as Object\n"
+    "Dim aWinDesc as new \"com.sun.star.awt.WindowDescriptor\"\n"
+    "Dim aRect as new \"com.sun.star.awt.Rectangle\"\n"
+    "aRect.X = 200\n"
+    "aWinDesc.Bounds = aRect\n"
+    "testUnoAccess = aWinDesc\n"
+"End Function\n"
+);
+
 
 void Nested_Struct::testAssign1()
 {
@@ -106,6 +183,29 @@
     // forces a broadcast
     SbxVariableRef pNew = new  SbxMethod( *((SbxMethod*)pMeth));
     CPPUNIT_ASSERT(pNew->GetInteger() == 9 );
+}
+
+void Nested_Struct::testAssign1Alt()
+{
+    CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
+    StarBASICRef pBasic = new StarBASIC();
+    ResetError();
+    StarBASIC::SetGlobalErrorHdl( LINK( this, Nested_Struct, BasicErrorHdl ) );
+
+    SbModule* pMod = pBasic->MakeModule( rtl::OUString( "TestModule" ), sTestSource1Alt );
+    pMod->Compile();
+    CPPUNIT_ASSERT_MESSAGE("testAssign1Alt fails with compile error",!HasError() );
+    SbMethod* pMeth = static_cast<SbMethod*>(pMod->Find( rtl::OUString("simpleNestStructAccess"),  
SbxCLASS_METHOD ));
+    CPPUNIT_ASSERT_MESSAGE("testAssign1Alt no method found", pMeth );
+    SbxVariableRef refTemp = pMeth;
+    // forces a broadcast
+    SbxVariableRef pNew = new  SbxMethod( *((SbxMethod*)pMeth));
+    uno::Any aRet = sbxToUnoValue( pNew );
+    table::TableBorder aBorder;
+    aRet >>= aBorder;
+
+    int result = aBorder.HorizontalLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL( 9, result );
 }
 
 void Nested_Struct::testOldAssign()
@@ -126,6 +226,29 @@
     CPPUNIT_ASSERT(pNew->GetInteger() == 9 );
 }
 
+void Nested_Struct::testOldAssignAlt()
+{
+    CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
+    StarBASICRef pBasic = new StarBASIC();
+    ResetError();
+    StarBASIC::SetGlobalErrorHdl( LINK( this, Nested_Struct, BasicErrorHdl ) );
+
+    SbModule* pMod = pBasic->MakeModule( rtl::OUString( "TestModule" ), sTestSource2Alt );
+    pMod->Compile();
+    CPPUNIT_ASSERT_MESSAGE("testOldAssign fails with compile error",!HasError() );
+    SbMethod* pMeth = static_cast<SbMethod*>(pMod->Find( rtl::OUString("simpleRegressionTestOld"), 
 SbxCLASS_METHOD ));
+    CPPUNIT_ASSERT_MESSAGE("testOldAssign no method found", pMeth );
+    SbxVariableRef refTemp = pMeth;
+    // forces a broadcast
+    SbxVariableRef pNew = new  SbxMethod( *((SbxMethod*)pMeth));
+    uno::Any aRet = sbxToUnoValue( pNew );
+    table::TableBorder aBorder;
+    aRet >>= aBorder;
+
+    int result = aBorder.HorizontalLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL( 9, result );
+}
+
 void Nested_Struct::testUnfixedVarAssign()
 {
     CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
@@ -144,6 +267,41 @@
     CPPUNIT_ASSERT(pNew->GetInteger() == 13 );
 }
 
+void Nested_Struct::testUnfixedVarAssignAlt()
+{
+    CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
+    StarBASICRef pBasic = new StarBASIC();
+    ResetError();
+    StarBASIC::SetGlobalErrorHdl( LINK( this, Nested_Struct, BasicErrorHdl ) );
+
+    SbModule* pMod = pBasic->MakeModule( rtl::OUString( "TestModule" ), sTestSource3Alt );
+    pMod->Compile();
+    CPPUNIT_ASSERT_MESSAGE("testUnfixedVarAssignAlt fails with compile error",!HasError() );
+    SbMethod* pMeth = static_cast<SbMethod*>(pMod->Find( rtl::OUString("testUnfixedVarAssign"),  
SbxCLASS_METHOD ));
+    CPPUNIT_ASSERT_MESSAGE("testUnfixedVarAssignAlt no method found", pMeth );
+    SbxVariableRef refTemp = pMeth;
+    // forces a broadcast
+    SbxVariableRef pNew = new SbxMethod( *((SbxMethod*)pMeth));
+    uno::Any aRet = sbxToUnoValue( pNew );
+
+    uno::Sequence< uno::Any > aResult;
+    bool bRes = aRet >>= aResult;
+    CPPUNIT_ASSERT_EQUAL(true, bRes );
+
+    // should have 2 elements in a sequence returned
+    CPPUNIT_ASSERT_EQUAL(2, aResult.getLength() );
+
+    table::TableBorder aBorder;
+    aResult[0] >>= aBorder;
+
+    table::BorderLine aBorderLine;
+    aResult[1] >>= aBorderLine;
+    int result = aBorder.HorizontalLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL(9, result );
+    result = aBorderLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL(4, result );
+}
+
 void Nested_Struct::testFixedVarAssign()
 {
     CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
@@ -162,6 +320,63 @@
     CPPUNIT_ASSERT(pNew->GetInteger() == 13 );
 }
 
+void Nested_Struct::testFixedVarAssignAlt()
+{
+    CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
+    StarBASICRef pBasic = new StarBASIC();
+    ResetError();
+    StarBASIC::SetGlobalErrorHdl( LINK( this, Nested_Struct, BasicErrorHdl ) );
+
+    SbModule* pMod = pBasic->MakeModule( rtl::OUString( "TestModule" ), sTestSource4Alt );
+    pMod->Compile();
+    CPPUNIT_ASSERT_MESSAGE("testFixedVarAssignAlt fails with compile error",!HasError() );
+    SbMethod* pMeth = static_cast<SbMethod*>(pMod->Find( rtl::OUString("testFixedVarAssign"),  
SbxCLASS_METHOD ));
+    CPPUNIT_ASSERT_MESSAGE("testFixedVarAssignAlt no method found", pMeth );
+    SbxVariableRef refTemp = pMeth;
+    // forces a broadcast
+    SbxVariableRef pNew = new SbxMethod( *((SbxMethod*)pMeth));
+    uno::Any aRet = sbxToUnoValue( pNew );
+
+    uno::Sequence< uno::Any > aResult;
+    bool bRes = aRet >>= aResult;
+    CPPUNIT_ASSERT_EQUAL(true, bRes );
+
+    // should have 2 elements in a sequence returned
+    CPPUNIT_ASSERT_EQUAL(2, aResult.getLength() );
+
+    table::TableBorder aBorder;
+    aResult[0] >>= aBorder;
+
+    table::BorderLine aBorderLine;
+    aResult[1] >>= aBorderLine;
+    int result = aBorder.HorizontalLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL(9, result );
+    result = aBorderLine.OuterLineWidth;
+    CPPUNIT_ASSERT_EQUAL(4, result );
+}
+
+void Nested_Struct::testUnoAccess()
+{
+    CPPUNIT_ASSERT_MESSAGE( "No resource manager", basicDLL().GetBasResMgr() != NULL );
+    StarBASICRef pBasic = new StarBASIC();
+    ResetError();
+    StarBASIC::SetGlobalErrorHdl( LINK( this, Nested_Struct, BasicErrorHdl ) );
+
+    SbModule* pMod = pBasic->MakeModule( rtl::OUString( "TestModule" ), sTestSource5 );
+    pMod->Compile();
+    CPPUNIT_ASSERT_MESSAGE("testUnoAccess fails with compile error",!HasError() );
+    SbMethod* pMeth = static_cast<SbMethod*>(pMod->Find( rtl::OUString("testUnoAccess"),  
SbxCLASS_METHOD ));
+    CPPUNIT_ASSERT_MESSAGE("testUnoAccess no method found", pMeth );
+    SbxVariableRef refTemp = pMeth;
+    // forces a broadcast
+    SbxVariableRef pNew = new  SbxMethod( *((SbxMethod*)pMeth));
+    uno::Any aRet = sbxToUnoValue( pNew );
+    awt::WindowDescriptor aWinDesc;
+    aRet >>= aWinDesc;
+
+    CPPUNIT_ASSERT_EQUAL(200, aWinDesc.Bounds.X );
+}
+
   // Put the test suite in the registry
   CPPUNIT_TEST_SUITE_REGISTRATION(Nested_Struct);
 } // namespace
diff --git a/basic/source/classes/sbunoobj.cxx b/basic/source/classes/sbunoobj.cxx
index 54d2a26e..4ac95f0 100644
--- a/basic/source/classes/sbunoobj.cxx
+++ b/basic/source/classes/sbunoobj.cxx
@@ -2102,18 +2102,27 @@
                 {
                     try
                     {
-                        if ( pProp->isUnoStruct() && maStructInfo.get()  )
+                        if ( maStructInfo.get()  )
                         {
-                            StructRefInfo aMemberStruct = maStructInfo->getStructMember( 
pProp->GetName() );
-                            if ( aMemberStruct.isEmpty() )
+                            StructRefInfo aMember = maStructInfo->getStructMember( 
pProp->GetName() );
+                            if ( aMember.isEmpty() )
                             {
                                  StarBASIC::Error( SbERR_PROPERTY_NOT_FOUND );
                             }
                             else
                             {
-                                SbUnoStructRefObject* pSbUnoObject = new SbUnoStructRefObject( 
pProp->GetName(), aMemberStruct );
-                                SbxObjectRef xWrapper = (SbxObject*)pSbUnoObject;
-                                pVar->PutObject( xWrapper );
+                                if ( pProp->isUnoStruct() )
+                                {
+                                    SbUnoStructRefObject* pSbUnoObject = new SbUnoStructRefObject( 
pProp->GetName(), aMember );
+                                    SbxObjectRef xWrapper = (SbxObject*)pSbUnoObject;
+                                    pVar->PutObject( xWrapper );
+                                }
+                                else
+                                {
+                                    Any aRetAny = aMember.getValue();
+                                    // take over the value from Uno to Sbx
+                                    unoToSbxValue( pVar, aRetAny );
+                                }
                                 return;
                             }
                         }
@@ -2170,7 +2179,20 @@
                         StarBASIC::Error( SbERR_PROP_READONLY );
                         return;
                     }
-
+                    if (  maStructInfo.get()  )
+                    {
+                        StructRefInfo aMember = maStructInfo->getStructMember( pProp->GetName() );
+                        if ( aMember.isEmpty() )
+                        {
+                            StarBASIC::Error( SbERR_PROPERTY_NOT_FOUND );
+                        }
+                        else
+                        {
+                            Any aAnyValue = sbxToUnoValue( pVar, pProp->aUnoProp.Type, 
&pProp->aUnoProp );
+                            aMember.setValue( aAnyValue );
+                        }
+                        return;
+                   }
                     // take over the value from Uno to Sbx
                     Any aAnyValue = sbxToUnoValue( pVar, pProp->aUnoProp.Type, &pProp->aUnoProp );
                     try
@@ -2877,7 +2899,9 @@
 {
     Any aRetAny;
     if( bNeedIntrospection ) doIntrospection();
-    if( mxMaterialHolder.is() )
+    if ( maStructInfo.get() )
+       aRetAny = maTmpUnoObj;
+    else if( mxMaterialHolder.is() )
         aRetAny = mxMaterialHolder->getMaterial();
     else if( mxInvocation.is() )
         aRetAny <<= mxInvocation;

-- 
To view, visit https://gerrit.libreoffice.org/2099
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iff007e17df87148ea81e69d3567c8cf9857fdcaa
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Noel Power <noel.power@suse.com>