CopyTableWizard: make code less confusing

createTable looked into the requested operation, and depending on
that, either created the table or fetched the existing one. For a
function named createTable, that made for confusing reading.

Split that into:
 * createTable that creates the table
 * getTable that fetches the existing table
 * returnTable that automagically creates or fetches an existing
   table, depending on the requested operation.

Change-Id: I91be67c24026c850530dcaef5ec95ab508e81434
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93882
Tested-by: Jenkins
Reviewed-by: Lionel Elie Mamane <lionel@mamane.lu>

4 files changed, 132 insertions, 108 deletions

diff --git a/dbaccess/source/ui/inc/WCopyTable.hxx b/dbaccess/source/ui/inc/WCopyTable.hxx
index fffc3cf1dee4..0563232451f9 100644
--- a/dbaccess/source/ui/inc/WCopyTable.hxx
+++ b/dbaccess/source/ui/inc/WCopyTable.hxx
@@ -385,6 +385,8 @@ namespace dbaui
         */
         void clearDestColumns();
 
+        css::uno::Reference< css::beans::XPropertySet > returnTable();
+        css::uno::Reference< css::beans::XPropertySet > getTable();
         css::uno::Reference< css::beans::XPropertySet > createTable();
         css::uno::Reference< css::beans::XPropertySet > createView() const;
         sal_Int32 getMaxColumnNameLength() const;
diff --git a/dbaccess/source/ui/misc/DExport.cxx b/dbaccess/source/ui/misc/DExport.cxx
index 384703a6b573..cc24fe114c63 100644
--- a/dbaccess/source/ui/misc/DExport.cxx
+++ b/dbaccess/source/ui/misc/DExport.cxx
@@ -687,7 +687,7 @@ bool ODatabaseExport::executeWizard(const OUString& _rTableName, const Any& _aTe
                 case CopyTableOperation::CopyDefinitionAndData:
                 case CopyTableOperation::AppendData:
                     {
-                        m_xTable = aWizard.createTable();
+                        m_xTable = aWizard.returnTable();
                         bError = !m_xTable.is();
                         if(m_xTable.is())
                         {
diff --git a/dbaccess/source/ui/misc/WCopyTable.cxx b/dbaccess/source/ui/misc/WCopyTable.cxx
index 0a54f4982d0c..30ebf6f6a642 100644
--- a/dbaccess/source/ui/misc/WCopyTable.cxx
+++ b/dbaccess/source/ui/misc/WCopyTable.cxx
@@ -1135,6 +1135,28 @@ Reference< XPropertySet > OCopyTableWizard::createView() const
     return ::dbaui::createView( m_sName, m_xDestConnection, sCommand );
 }
 
+Reference< XPropertySet > OCopyTableWizard::returnTable()
+{
+    if ( getOperation() == CopyTableOperation::AppendData )
+        return getTable();
+    else
+        return createTable();
+}
+
+Reference< XPropertySet > OCopyTableWizard::getTable()
+{
+    Reference< XPropertySet > xTable;
+
+    Reference<XTablesSupplier> xSup( m_xDestConnection, UNO_QUERY );
+    Reference< XNameAccess > xTables;
+    if(xSup.is())
+        xTables = xSup->getTables();
+    if(xTables.is() && xTables->hasByName(m_sName))
+        xTables->getByName(m_sName) >>= xTable;
+
+    return xTable;
+}
+
 Reference< XPropertySet > OCopyTableWizard::createTable()
 {
     Reference< XPropertySet > xTable;
@@ -1143,126 +1165,122 @@ Reference< XPropertySet > OCopyTableWizard::createTable()
     Reference< XNameAccess > xTables;
     if(xSup.is())
         xTables = xSup->getTables();
-    if ( getOperation() != CopyTableOperation::AppendData )
+    Reference<XDataDescriptorFactory> xFact(xTables,UNO_QUERY);
+    OSL_ENSURE(xFact.is(),"No XDataDescriptorFactory available!");
+    if(!xFact.is())
+        return nullptr;
+
+    xTable = xFact->createDataDescriptor();
+    OSL_ENSURE(xTable.is(),"Could not create a new object!");
+    if(!xTable.is())
+        return nullptr;
+
+    OUString sCatalog,sSchema,sTable;
+    Reference< XDatabaseMetaData> xMetaData = m_xDestConnection->getMetaData();
+    ::dbtools::qualifiedNameComponents(xMetaData,
+                                       m_sName,
+                                       sCatalog,
+                                       sSchema,
+                                       sTable,
+                                       ::dbtools::EComposeRule::InDataManipulation);
+
+    if ( sCatalog.isEmpty() && xMetaData->supportsCatalogsInTableDefinitions() )
     {
-        Reference<XDataDescriptorFactory> xFact(xTables,UNO_QUERY);
-        OSL_ENSURE(xFact.is(),"No XDataDescriptorFactory available!");
-        if(!xFact.is())
-            return nullptr;
-
-        xTable = xFact->createDataDescriptor();
-        OSL_ENSURE(xTable.is(),"Could not create a new object!");
-        if(!xTable.is())
-            return nullptr;
-
-        OUString sCatalog,sSchema,sTable;
-        Reference< XDatabaseMetaData> xMetaData = m_xDestConnection->getMetaData();
-        ::dbtools::qualifiedNameComponents(xMetaData,
-                                            m_sName,
-                                            sCatalog,
-                                            sSchema,
-                                            sTable,
-                                            ::dbtools::EComposeRule::InDataManipulation);
+        sCatalog = m_xDestConnection->getCatalog();
+    }
 
-        if ( sCatalog.isEmpty() && xMetaData->supportsCatalogsInTableDefinitions() )
+    if ( sSchema.isEmpty() && xMetaData->supportsSchemasInTableDefinitions() )
+    {
+        // query of current schema is quite inconsistent. In case of some
+        // DBMS's each user has their own schema.
+        sSchema = xMetaData->getUserName();
+        // In case of mysql it is not that simple
+        if(xMetaData->getDatabaseProductName() == "MySQL")
         {
-            sCatalog = m_xDestConnection->getCatalog();
+            Reference< XStatement > xSelect = m_xDestConnection->createStatement();
+            Reference< XResultSet > xRs = xSelect->executeQuery("select database()");
+            (void)xRs->next(); // first and only result
+            Reference< XRow > xRow( xRs, UNO_QUERY_THROW );
+            sSchema = xRow->getString(1);
         }
+    }
 
-        if ( sSchema.isEmpty() && xMetaData->supportsSchemasInTableDefinitions() )
-        {
-            // query of current schema is quite inconsistent. In case of some
-            // DBMS's each user has their own schema.
-            sSchema = xMetaData->getUserName();
-            // In case of mysql it is not that simple
-            if(xMetaData->getDatabaseProductName() == "MySQL")
-            {
-                Reference< XStatement > xSelect = m_xDestConnection->createStatement();
-                Reference< XResultSet > xRs = xSelect->executeQuery("select database()");
-                (void)xRs->next(); // first and only result
-                Reference< XRow > xRow( xRs, UNO_QUERY_THROW );
-                sSchema = xRow->getString(1);
-            }
-        }
+    xTable->setPropertyValue(PROPERTY_CATALOGNAME,makeAny(sCatalog));
+    xTable->setPropertyValue(PROPERTY_SCHEMANAME,makeAny(sSchema));
+    xTable->setPropertyValue(PROPERTY_NAME,makeAny(sTable));
 
-        xTable->setPropertyValue(PROPERTY_CATALOGNAME,makeAny(sCatalog));
-        xTable->setPropertyValue(PROPERTY_SCHEMANAME,makeAny(sSchema));
-        xTable->setPropertyValue(PROPERTY_NAME,makeAny(sTable));
-
-        Reference< XColumnsSupplier > xSuppDestinationColumns( xTable, UNO_QUERY );
-        // now append the columns
-        const ODatabaseExport::TColumnVector& rVec = getDestVector();
-        appendColumns( xSuppDestinationColumns, &rVec );
-        // now append the primary key
-        Reference<XKeysSupplier> xKeySup(xTable,UNO_QUERY);
-        appendKey(xKeySup, &rVec);
-
-        Reference<XAppend> xAppend(xTables,UNO_QUERY);
-        if(xAppend.is())
-            xAppend->appendByDescriptor(xTable);
-
-        //  xTable = NULL;
-        // we need to reget the table because after appending it, it is no longer valid
-        if(xTables->hasByName(m_sName))
-            xTables->getByName(m_sName) >>= xTable;
-        else
+    Reference< XColumnsSupplier > xSuppDestinationColumns( xTable, UNO_QUERY );
+    // now append the columns
+    const ODatabaseExport::TColumnVector& rVec = getDestVector();
+    appendColumns( xSuppDestinationColumns, &rVec );
+    // now append the primary key
+    Reference<XKeysSupplier> xKeySup(xTable,UNO_QUERY);
+    appendKey(xKeySup, &rVec);
+
+    Reference<XAppend> xAppend(xTables,UNO_QUERY);
+    if(xAppend.is())
+        xAppend->appendByDescriptor(xTable);
+
+    //  xTable = NULL;
+    // we need to reget the table because after appending it, it is no longer valid
+    if(xTables->hasByName(m_sName))
+        xTables->getByName(m_sName) >>= xTable;
+    else
+    {
+        OUString sComposedName(
+            ::dbtools::composeTableName( m_xDestConnection->getMetaData(), xTable, ::dbtools::EComposeRule::InDataManipulation, false ) );
+        if(xTables->hasByName(sComposedName))
         {
-            OUString sComposedName(
-                ::dbtools::composeTableName( m_xDestConnection->getMetaData(), xTable, ::dbtools::EComposeRule::InDataManipulation, false ) );
-            if(xTables->hasByName(sComposedName))
-            {
-                xTables->getByName(sComposedName) >>= xTable;
-                m_sName = sComposedName;
-            }
-            else
-                xTable = nullptr;
+            xTables->getByName(sComposedName) >>= xTable;
+            m_sName = sComposedName;
         }
-        if(xTable.is())
+        else
+            xTable = nullptr;
+    }
+
+    if(xTable.is())
+    {
+        xSuppDestinationColumns.set( xTable, UNO_QUERY_THROW );
+        // insert new table name into table filter
+        ::dbaui::appendToFilter(m_xDestConnection, m_sName, GetComponentContext(), m_xAssistant.get());
+
+        // copy ui settings
+        m_rSourceObject.copyUISettingsTo( xTable );
+        //copy filter and sorting
+        m_rSourceObject.copyFilterAndSortingTo(m_xDestConnection,xTable);
+        // set column mappings
+        Reference<XNameAccess> xNameAccess = xSuppDestinationColumns->getColumns();
+        Sequence< OUString> aSeq = xNameAccess->getElementNames();
+        const OUString* pIter = aSeq.getConstArray();
+        const OUString* pEnd   = pIter + aSeq.getLength();
+
+        for(sal_Int32 nNewPos=1;pIter != pEnd;++pIter,++nNewPos)
         {
-            xSuppDestinationColumns.set( xTable, UNO_QUERY_THROW );
-            // insert new table name into table filter
-            ::dbaui::appendToFilter(m_xDestConnection, m_sName, GetComponentContext(), m_xAssistant.get());
-
-            // copy ui settings
-            m_rSourceObject.copyUISettingsTo( xTable );
-            //copy filter and sorting
-            m_rSourceObject.copyFilterAndSortingTo(m_xDestConnection,xTable);
-            // set column mappings
-            Reference<XNameAccess> xNameAccess = xSuppDestinationColumns->getColumns();
-            Sequence< OUString> aSeq = xNameAccess->getElementNames();
-            const OUString* pIter = aSeq.getConstArray();
-            const OUString* pEnd   = pIter + aSeq.getLength();
-
-            for(sal_Int32 nNewPos=1;pIter != pEnd;++pIter,++nNewPos)
+            ODatabaseExport::TColumns::const_iterator aDestIter = m_vDestColumns.find(*pIter);
+
+            if ( aDestIter != m_vDestColumns.end() )
             {
-                ODatabaseExport::TColumns::const_iterator aDestIter = m_vDestColumns.find(*pIter);
+                ODatabaseExport::TColumnVector::const_iterator aFind = std::find(m_aDestVec.begin(),m_aDestVec.end(),aDestIter);
+                sal_Int32 nPos = (aFind - m_aDestVec.begin())+1;
+
+                ODatabaseExport::TPositions::iterator aPosFind = std::find_if(
+                    m_vColumnPositions.begin(),
+                    m_vColumnPositions.end(),
+                    [nPos] (const ODatabaseExport::TPositions::value_type& tPos) {
+                        return tPos.first == nPos;
+                    }
+                );
 
-                if ( aDestIter != m_vDestColumns.end() )
+                if ( m_vColumnPositions.end() != aPosFind )
                 {
-                    ODatabaseExport::TColumnVector::const_iterator aFind = std::find(m_aDestVec.begin(),m_aDestVec.end(),aDestIter);
-                    sal_Int32 nPos = (aFind - m_aDestVec.begin())+1;
-
-                    ODatabaseExport::TPositions::iterator aPosFind = std::find_if(
-                        m_vColumnPositions.begin(),
-                        m_vColumnPositions.end(),
-                        [nPos] (const ODatabaseExport::TPositions::value_type& tPos) {
-                            return tPos.first == nPos;
-                        }
-                    );
-
-                    if ( m_vColumnPositions.end() != aPosFind )
-                    {
-                        aPosFind->second = nNewPos;
-                        OSL_ENSURE( m_vColumnTypes.size() > o3tl::make_unsigned( aPosFind - m_vColumnPositions.begin() ),
-                            "Invalid index for vector!" );
-                        m_vColumnTypes[ aPosFind - m_vColumnPositions.begin() ] = (*aFind)->second->GetType();
-                    }
+                    aPosFind->second = nNewPos;
+                    OSL_ENSURE( m_vColumnTypes.size() > o3tl::make_unsigned( aPosFind - m_vColumnPositions.begin() ),
+                        "Invalid index for vector!" );
+                    m_vColumnTypes[ aPosFind - m_vColumnPositions.begin() ] = (*aFind)->second->GetType();
                 }
             }
         }
     }
-    else if(xTables.is() && xTables->hasByName(m_sName))
-        xTables->getByName(m_sName) >>= xTable;
 
     return xTable;
 }
diff --git a/dbaccess/source/ui/uno/copytablewizard.cxx b/dbaccess/source/ui/uno/copytablewizard.cxx
index d72b85da196f..22591dedc00d 100644
--- a/dbaccess/source/ui/uno/copytablewizard.cxx
+++ b/dbaccess/source/ui/uno/copytablewizard.cxx
@@ -1328,13 +1328,17 @@ void CopyTableWizard::impl_doCopy_nothrow()
 
             case CopyTableOperation::AppendData:
             {
-
+                // note that the CopyDefinitionAndData case falls through to here.
+                assert((rWizard.getOperation() == CopyTableOperation::CopyDefinitionAndData) ||
+                       (rWizard.getOperation() == CopyTableOperation::AppendData));
+                assert((rWizard.getOperation() == CopyTableOperation::CopyDefinitionAndData) == xTable.is());
                 if ( !xTable.is() )
                 {
-                    xTable = rWizard.createTable();
+                    assert(rWizard.getOperation() == CopyTableOperation::AppendData);
+                    xTable = rWizard.getTable();
                     if ( !xTable.is() )
                     {
-                        SAL_WARN("dbaccess.ui", "CopyTableWizard::impl_doCopy_nothrow: createTable should throw here, shouldn't it?" );
+                        SAL_WARN("dbaccess.ui", "CopyTableWizard::impl_doCopy_nothrow: getTable should throw here, shouldn't it?" );
                         break;
                     }
                 }