Re: [PATCH] Bug 48140 - [EasyHack] remove uno calls from xlsx import

On 04/16/2012 05:13 AM, Noel Power wrote:
> On 15/04/12 05:20, Daniel Bankston [danthedev] wrote:
> > Hello, everyone,
> >
> > I have attached my patches for EasyHack 48140.  I have taken baby 
> > steps, so this just the first part of completing EasyHack 48140.  I 
> > changed WorkbookHelper::createNamedRangeObject and 
> > createLocalNamedRangeObject to return ScRangeData* instead of 
> > Reference<XNamedRanged>, and then I adjusted all callers.  You will 
> > notice I commented the old code out instead of deleting it.  I did 
> > this as this is just partial update for the EasyHack.
> >
> > I was able to successfully make sc with these changes.  "make -sr 
> > slowcheck" was also successful.  My own manual tests were also 
> > successful.  I tested by running calc and opening xlsx spreadsheets 
> > containing ranges with names.  I also created names for ranges and 
> > created names for cells in those spreadsheets.  I referred to those 
> > ranges and cells by name in other cells.
> >
> > Although my tests were successful, I still fear that there is 
> > something that I may have missed due to my current beginner level of 
> > understanding of the sc model.  I am eager to see what suggestions 
> > you all may have.
> >
> > When I have more time, I later plan to make similar changes to the 
> > database ranges and eventually remove all UNO calls in these methods 
> > where possible.  (School semester is still in for me and quite busy 
> > as it nears the end.)
> First please don't comment out code, it just makes the diff even more 
> confusing to look at, git already shows the deletions and annotating 
> that in the source really isn't necessary. Also, it's not a good idea 
> to have patches to patches especially for a standalone task like this. 
> Could you resubmit a cummulative patch  ( I think you can use git 
> commit -i for that ). From a quick look it seems the patch is probably 
> ok, one problem I see with a test document I have is that I now see in 
> the 'Insert | Names | Manage' dialog many many '__sharedxxxx' entries. 
> At a wild guess this would seem to step from the fact that the old 
> code used to set 'IsSharedFormula' on some named object entries and 
> your patch doesn't seem to do that. Markus or Kohei probably would 
> know more about the specifics of that stuff
>
> However I think the patch is most likely "nearly there" so don't give 
> up.  I think at least you have proved that you can download, build and 
> at least manipulate the libreoffice code which is the main purpose of 
> the easy hack :-)
>
> thanks for the patch and effort
> Noel

Hi, Noel,

Thanks for looking at my patch submission and for the helpful 
suggestions.  I have attached another single patch file that combines 
all the patch files and has the commented code removed.

May I please have a copy of that test document so that I can try to 
resolve the problem you mentioned? (When time permits, that is.)

Respectfully,
Daniel Bankston

From 2bf45f47bac97b688d2321fcaaf60f899c347e4b Mon Sep 17 00:00:00 2001
From: Daniel Bankston <daniel.dev.libreoffice@gmail.com>
Date: Mon, 16 Apr 2012 09:34:32 -0500
Subject: [PATCH] Converting UNO calls to direct in WorkbookHelper

---
 sc/inc/tokenuno.hxx                      |    2 +-
 sc/source/filter/inc/defnamesbuffer.hxx  |    4 ++--
 sc/source/filter/inc/sheetdatabuffer.hxx |    1 -
 sc/source/filter/inc/workbookhelper.hxx  |    7 +++----
 sc/source/filter/oox/defnamesbuffer.cxx  |   24 +++++++++++++-----------
 sc/source/filter/oox/sheetdatabuffer.cxx |    9 +++------
 sc/source/filter/oox/workbookhelper.cxx  |   29 ++++++++++++++---------------
 7 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/sc/inc/tokenuno.hxx b/sc/inc/tokenuno.hxx
index 36486c4..a3d8732 100644
--- a/sc/inc/tokenuno.hxx
+++ b/sc/inc/tokenuno.hxx
@@ -53,7 +53,7 @@ public:
                         ScDocument& rDoc,
                         ScTokenArray& rTokenArray,
                         const com::sun::star::uno::Sequence< com::sun::star::sheet::FormulaToken 
> & rSequence );
-    static bool ConvertToTokenSequence(
+    static SC_DLLPUBLIC bool ConvertToTokenSequence(
                         ScDocument& rDoc,
                         com::sun::star::uno::Sequence< com::sun::star::sheet::FormulaToken >& 
rSequence,
                         const ScTokenArray& rTokenArray );
diff --git a/sc/source/filter/inc/defnamesbuffer.hxx b/sc/source/filter/inc/defnamesbuffer.hxx
index 6704f19..2f132d6 100644
--- a/sc/source/filter/inc/defnamesbuffer.hxx
+++ b/sc/source/filter/inc/defnamesbuffer.hxx
@@ -30,6 +30,7 @@
 #define OOX_XLS_DEFINEDNAMESBUFFER_HXX
 
 #include "formulabase.hxx"
+#include "rangenam.hxx"
 
 namespace com { namespace sun { namespace star {
     namespace sheet { class XNamedRange; }
@@ -158,8 +159,7 @@ private:
     typedef ::std::auto_ptr< StreamDataSequence >   StreamDataSeqPtr;
     typedef ::std::auto_ptr< BiffInputStreamPos >   BiffStreamPosPtr;
 
-    ::com::sun::star::uno::Reference< ::com::sun::star::sheet::XNamedRange >
-                        mxNamedRange;       /// XNamedRange interface of the defined name.
+    ScRangeData*        mpScRangeData;       /// ScRangeData of the defined name.
     sal_Int32           mnTokenIndex;       /// Name index used in API token array.
     sal_Int16           mnCalcSheet;        /// Calc sheet index for sheet-local names.
     sal_Unicode         mcBuiltinId;        /// Identifier for built-in defined names.
diff --git a/sc/source/filter/inc/sheetdatabuffer.hxx b/sc/source/filter/inc/sheetdatabuffer.hxx
index dcf42bc..65612bc 100644
--- a/sc/source/filter/inc/sheetdatabuffer.hxx
+++ b/sc/source/filter/inc/sheetdatabuffer.hxx
@@ -35,7 +35,6 @@
 #include "worksheethelper.hxx"
 
 namespace com { namespace sun { namespace star {
-    namespace sheet { class XNamedRange; }
     namespace util { struct DateTime; }
 } } }
 
diff --git a/sc/source/filter/inc/workbookhelper.hxx b/sc/source/filter/inc/workbookhelper.hxx
index 7a52984..c4a4ee1 100644
--- a/sc/source/filter/inc/workbookhelper.hxx
+++ b/sc/source/filter/inc/workbookhelper.hxx
@@ -33,6 +33,7 @@
 #include <rtl/ref.hxx>
 #include "oox/helper/storagebase.hxx"
 #include "biffhelper.hxx"
+#include "rangenam.hxx"
 
 namespace com { namespace sun { namespace star {
     namespace container { class XNameAccess; }
@@ -183,8 +184,7 @@ public:
     /** Creates and returns a defined name on-the-fly in the Calc document.
         The name will not be buffered in the global defined names buffer.
         @param orName  (in/out-parameter) Returns the resulting used name. */
-    ::com::sun::star::uno::Reference< ::com::sun::star::sheet::XNamedRange >
-                        createNamedRangeObject(
+    ScRangeData* createNamedRangeObject(
                             ::rtl::OUString& orName,
                             const ::com::sun::star::uno::Sequence< 
::com::sun::star::sheet::FormulaToken>& rTokens,
                             sal_Int32 nIndex,
@@ -193,8 +193,7 @@ public:
     /** Creates and returns a defined name on-the-fly in the sheet.
         The name will not be buffered in the global defined names buffer.
         @param orName  (in/out-parameter) Returns the resulting used name. */
-    ::com::sun::star::uno::Reference< ::com::sun::star::sheet::XNamedRange >
-                        createLocalNamedRangeObject(
+    ScRangeData* createLocalNamedRangeObject(
                             ::rtl::OUString& orName,
                             const ::com::sun::star::uno::Sequence< 
::com::sun::star::sheet::FormulaToken>& rTokens,
                             sal_Int32 nIndex,
diff --git a/sc/source/filter/oox/defnamesbuffer.cxx b/sc/source/filter/oox/defnamesbuffer.cxx
index ea767da..50fcf99 100644
--- a/sc/source/filter/oox/defnamesbuffer.cxx
+++ b/sc/source/filter/oox/defnamesbuffer.cxx
@@ -44,6 +44,8 @@
 #include "externallinkbuffer.hxx"
 #include "formulaparser.hxx"
 #include "worksheetbuffer.hxx"
+#include "tokenarray.hxx"
+#include "tokenuno.hxx"
 
 namespace oox {
 namespace xls {
@@ -386,9 +388,9 @@ void DefinedName::createNameObject( sal_Int32 nIndex )
 
     // create the name and insert it into the document, maCalcName will be changed to the 
resulting name
     if (maModel.mnSheet >= 0)
-        mxNamedRange = createLocalNamedRangeObject( maCalcName, getTokens(), nIndex, nNameFlags, 
maModel.mnSheet );
+               mpScRangeData = createLocalNamedRangeObject( maCalcName, getTokens(), nIndex, 
nNameFlags, maModel.mnSheet );
     else
-        mxNamedRange = createNamedRangeObject( maCalcName, getTokens(), nIndex, nNameFlags );
+        mpScRangeData = createNamedRangeObject( maCalcName, getTokens(), nIndex, nNameFlags );
     mnTokenIndex = nIndex;
 }
 
@@ -430,9 +432,9 @@ DefinedName::getTokens()
 
 void DefinedName::convertFormula()
 {
-    Reference< XFormulaTokens > xTokens( mxNamedRange, UNO_QUERY );
-    if( !xTokens.is() )
-        return;
+    ScTokenArray* pTokenArray = mpScRangeData->GetCode();
+    Sequence< FormulaToken > aFTokenSeq;
+    (void)ScTokenConversion::ConvertToTokenSequence( this->getScDocument(), aFTokenSeq, 
*pTokenArray );
     // set built-in names (print ranges, repeated titles, filter ranges)
     if( !isGlobalName() ) switch( mcBuiltinId )
     {
@@ -440,7 +442,7 @@ void DefinedName::convertFormula()
         {
             Reference< XPrintAreas > xPrintAreas( getSheetFromDoc( mnCalcSheet ), UNO_QUERY );
             ApiCellRangeList aPrintRanges;
-            getFormulaParser().extractCellRangeList( aPrintRanges, xTokens->getTokens(), false, 
mnCalcSheet );
+            getFormulaParser().extractCellRangeList( aPrintRanges, aFTokenSeq, false, mnCalcSheet 
);
             if( xPrintAreas.is() && !aPrintRanges.empty() )
                 xPrintAreas->setPrintAreas( ContainerHelper::vectorToSequence( aPrintRanges ) );
         }
@@ -449,7 +451,7 @@ void DefinedName::convertFormula()
         {
             Reference< XPrintAreas > xPrintAreas( getSheetFromDoc( mnCalcSheet ), UNO_QUERY );
             ApiCellRangeList aTitleRanges;
-            getFormulaParser().extractCellRangeList( aTitleRanges, xTokens->getTokens(), false, 
mnCalcSheet );
+            getFormulaParser().extractCellRangeList( aTitleRanges, aFTokenSeq, false, mnCalcSheet 
);
             if( xPrintAreas.is() && !aTitleRanges.empty() )
             {
                 bool bHasRowTitles = false;
@@ -480,10 +482,10 @@ void DefinedName::convertFormula()
 
 bool DefinedName::getAbsoluteRange( CellRangeAddress& orRange ) const
 {
-    /*  ScNamedRangeObj::XCellRangeReferrer::getReferredCells is buggy with
-        relative references, so we extract an absolute reference by hand. */
-    Reference< XFormulaTokens > xTokens( mxNamedRange, UNO_QUERY );
-    return xTokens.is() && getFormulaParser().extractCellRange( orRange, xTokens->getTokens(), 
false );
+    ScTokenArray* pTokenArray = mpScRangeData->GetCode();
+    Sequence< FormulaToken > aFTokenSeq;
+    ScTokenConversion::ConvertToTokenSequence( this->getScDocument(), aFTokenSeq, *pTokenArray );
+    return getFormulaParser().extractCellRange( orRange, aFTokenSeq, false );
 }
 
 // ============================================================================
diff --git a/sc/source/filter/oox/sheetdatabuffer.cxx b/sc/source/filter/oox/sheetdatabuffer.cxx
index 4abf946..08a4dce 100644
--- a/sc/source/filter/oox/sheetdatabuffer.cxx
+++ b/sc/source/filter/oox/sheetdatabuffer.cxx
@@ -588,15 +588,12 @@ void SheetDataBuffer::createSharedFormula( const BinAddress& rMapKey, const 
ApiT
         append( static_cast< sal_Int32 >( getSheetIndex() + 1 ) ).
         append( sal_Unicode( '_' ) ).append( rMapKey.mnRow ).
         append( sal_Unicode( '_' ) ).append( rMapKey.mnCol ).makeStringAndClear();
-    Reference< XNamedRange > xNamedRange = createNamedRangeObject( aName, rTokens, 0 );
-    OSL_ENSURE( xNamedRange.is(), "SheetDataBuffer::createSharedFormula - cannot create shared 
formula" );
-    PropertySet aNameProps( xNamedRange );
-    aNameProps.setProperty( PROP_IsSharedFormula, true );
+    ScRangeData* pScRangeData = createNamedRangeObject( aName, rTokens, 0 );
 
     // get and store the token index of the defined name
     OSL_ENSURE( maSharedFormulas.count( rMapKey ) == 0, "SheetDataBuffer::createSharedFormula - 
shared formula exists already" );
-    sal_Int32 nTokenIndex = 0;
-    if( aNameProps.getProperty( nTokenIndex, PROP_TokenIndex ) && (nTokenIndex >= 0) ) try
+    sal_Int32 nTokenIndex = static_cast< sal_Int32 >( pScRangeData->GetIndex() );
+    if( nTokenIndex >= 0 ) try
     {
         // store the token index in the map
         maSharedFormulas[ rMapKey ] = nTokenIndex;
diff --git a/sc/source/filter/oox/workbookhelper.cxx b/sc/source/filter/oox/workbookhelper.cxx
index 5a872c2..479f504 100644
--- a/sc/source/filter/oox/workbookhelper.cxx
+++ b/sc/source/filter/oox/workbookhelper.cxx
@@ -165,9 +165,9 @@ public:
     /** Returns the specified cell or page style from the Calc document. */
     Reference< XStyle > getStyleObject( const OUString& rStyleName, bool bPageStyle ) const;
     /** Creates and returns a defined name on-the-fly in the Calc document. */
-    Reference< XNamedRange > createNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags ) const;
+    ScRangeData* createNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& rTokens, 
sal_Int32 nIndex, sal_Int32 nNameFlags ) const;
     /** Creates and returns a defined name on the-fly in the correct Calc sheet. */
-    Reference< XNamedRange > createLocalNamedRangeObject( OUString& orName, const 
::com::sun::star::uno::Sequence< ::com::sun::star::sheet::FormulaToken>& rTokens, sal_Int32 nIndex, 
sal_Int32 nNameFlags, sal_Int32 nTab ) const;
+    ScRangeData* createLocalNamedRangeObject( OUString& orName, const Sequence< FormulaToken>& 
rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab ) const;
     /** Creates and returns a database range on-the-fly in the Calc document. */
     Reference< XDatabaseRange > createDatabaseRangeObject( OUString& orName, const 
CellRangeAddress& rRangeAddr ) const;
     /** Creates and returns an unnamed database range on-the-fly in the Calc document. */
@@ -358,7 +358,7 @@ Reference< XStyle > WorkbookGlobals::getStyleObject( const OUString& rStyleName,
     OSL_ENSURE( xStyle.is(), "WorkbookGlobals::getStyleObject - cannot access style object" );
     return xStyle;
 }
-void lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& rName, 
const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType )
+ScRangeData* lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const OUString& 
rName, const Sequence<FormulaToken>& rTokens, sal_Int16 nIndex, sal_Int32 nUnoType )
 {
     bool bDone = false;
     sal_uInt16 nNewType = RT_NAME;
@@ -376,12 +376,14 @@ void lcl_addNewByNameAndTokens( ScDocument& rDoc, ScRangeName* pNames, const 
OUS
         bDone = true;
     if (!bDone)
         throw RuntimeException();
+    return pNew;
 }
 
-Reference< XNamedRange > WorkbookGlobals::createNamedRangeObject( OUString& orName, const 
Sequence< FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags ) const
+ScRangeData* WorkbookGlobals::createNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags ) const
 {
     // create the name and insert it into the Calc document
     Reference< XNamedRange > xNamedRange;
+    ScRangeData* pScRangeData;
     if( !orName.isEmpty() ) try
     {
         // find an unused name
@@ -392,20 +394,19 @@ Reference< XNamedRange > WorkbookGlobals::createNamedRangeObject( OUString& 
orNa
         // create the named range
         ScDocument& rDoc =  getScDocument();
         ScRangeName* pNames = rDoc.GetRangeName();
-        lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags );
-        xNamedRange.set( xNamedRanges->getByName( orName ), UNO_QUERY );
+        pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, 
nNameFlags );
     }
     catch( Exception& )
     {
     }
-    OSL_ENSURE( xNamedRange.is(), "WorkbookGlobals::createNamedRangeObject - cannot create defined 
name" );
-    return xNamedRange;
+    return pScRangeData;
 }
 
-Reference< XNamedRange > WorkbookGlobals::createLocalNamedRangeObject( OUString& orName, const 
::com::sun::star::uno::Sequence< ::com::sun::star::sheet::FormulaToken>&  rTokens, sal_Int32 
nIndex, sal_Int32 nNameFlags, sal_Int32 nTab ) const
+ScRangeData* WorkbookGlobals::createLocalNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken >&  rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab ) const
 {
     // create the name and insert it into the Calc document
     Reference< XNamedRange > xNamedRange;
+    ScRangeData* pScRangeData;
     if( !orName.isEmpty() ) try
     {
         // find an unused name
@@ -420,14 +421,12 @@ Reference< XNamedRange > WorkbookGlobals::createLocalNamedRangeObject( 
OUString&
         // create the named range
         ScDocument& rDoc =  getScDocument();
         ScRangeName* pNames = rDoc.GetRangeName( nTab );
-        lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, nNameFlags );
-        xNamedRange.set( xNamedRanges->getByName( orName ), UNO_QUERY );
+        pScRangeData = lcl_addNewByNameAndTokens( rDoc, pNames, orName, rTokens, nIndex, 
nNameFlags );
     }
     catch( Exception& )
     {
     }
-    OSL_ENSURE( xNamedRange.is(), "WorkbookGlobals::createLocalNamedRangeObject - cannot create 
defined name" );
-    return xNamedRange;
+    return pScRangeData;
 }
 
 Reference< XDatabaseRange > WorkbookGlobals::createDatabaseRangeObject( OUString& orName, const 
CellRangeAddress& rRangeAddr ) const
@@ -743,12 +742,12 @@ Reference< XStyle > WorkbookHelper::getStyleObject( const OUString& 
rStyleName,
     return mrBookGlob.getStyleObject( rStyleName, bPageStyle );
 }
 
-Reference< XNamedRange > WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags ) const
+ScRangeData* WorkbookHelper::createNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags ) const
 {
     return mrBookGlob.createNamedRangeObject( orName, rTokens, nIndex, nNameFlags );
 }
 
-Reference< XNamedRange > WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const 
::com::sun::star::uno::Sequence< ::com::sun::star::sheet::FormulaToken>& rTokens, sal_Int32 nIndex, 
sal_Int32 nNameFlags, sal_Int32 nTab ) const
+ScRangeData* WorkbookHelper::createLocalNamedRangeObject( OUString& orName, const Sequence< 
FormulaToken>& rTokens, sal_Int32 nIndex, sal_Int32 nNameFlags, sal_Int32 nTab ) const
 {
     return mrBookGlob.createLocalNamedRangeObject( orName, rTokens, nIndex, nNameFlags, nTab );
 }
-- 
1.7.1