Pluginize SQL storage
ClosedPublic

Authored by wojnilowicz on Feb 9 2018, 2:41 PM.

Details

Summary

This patch makes database support in KMyMoney optional instead of mandatory. I believe most users use XML storage, so they could switch off SQL storage to save startup time and memory. Developers will benefit too, because they can switch off building of this plugin and save compilation time.

Following safty checks were implemented:

  1. user tries to open database with SQL storage plugin switched off,
  2. user opens database and switches SQL storage plugin off and tries to save his storage

I think that in future we could lower our core dependencies by droping off Qt5SQL.
I also think, that we could switch between XML and SQL on the fly.

Diff Detail

Repository
R261 KMyMoney
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
wojnilowicz requested review of this revision.Feb 9 2018, 2:41 PM
wojnilowicz created this revision.
tbaumgart requested changes to this revision.Feb 10 2018, 7:34 PM

For some reason the open and save database options are in a weird spot (at the bottom) of the file menu. Do you see the same thing? Can you check why?

kmymoney/kmymoney.cpp
94

Remove this line

102

Remove this line

156

Remove this line

This revision now requires changes to proceed.Feb 10 2018, 7:34 PM

For some reason the open and save database options are in a weird spot (at the bottom) of the file menu. Do you see the same thing? Can you check why?

I see it correctly. I took special care for those entries to appear in the right place. It could be that it doesn't work as expected on your old KDE Frameworks.

This patch fixes the problem:

diff --git a/kmymoney/kmymoneyui.rc b/kmymoney/kmymoneyui.rc
index 14c2591a..66e6e0c2 100644
--- a/kmymoney/kmymoneyui.rc
+++ b/kmymoney/kmymoneyui.rc
@@ -1,5 +1,5 @@
 <!DOCTYPE kpartgui>
-<kpartgui version="43" name="kmymoney" >
+<kpartgui version="44" name="kmymoney" >
  <MenuBar>
   <Menu name="file" >
    <DefineGroup name="open_db_group" append="open_merge"/>
wojnilowicz updated this revision to Diff 26903.EditedFeb 11 2018, 8:22 AM

This patch fixes the problem:

diff --git a/kmymoney/kmymoneyui.rc b/kmymoney/kmymoneyui.rc
index 14c2591a..66e6e0c2 100644
--- a/kmymoney/kmymoneyui.rc
+++ b/kmymoney/kmymoneyui.rc
@@ -1,5 +1,5 @@
 <!DOCTYPE kpartgui>
-<kpartgui version="43" name="kmymoney" >
+<kpartgui version="44" name="kmymoney" >
  <MenuBar>
   <Menu name="file" >
    <DefineGroup name="open_db_group" append="open_merge"/>

Good finding. It probably has nothing to do with KDE Frameworks version.
All changes has been incorporated in the patch.

Save as database does not work for me :(

Here's what I see on CLI

WebConnect: Try to connect to WebConnect server
WebConnect: Connect to server failed
WebConnect: Running in server mode
Plugins: checkprinting loaded
Plugins: csvexporter loaded
Plugins: csvimporter loaded
Plugins: gncimporter loaded
Plugins: icalendarexporter loaded
Plugins: qifexporter loaded
Plugins: qifimporter loaded
Plugins: reconciliation report loaded
Plugins: sqlstorage loaded
Cost center model created with items 0
Payees model created with items 0
reading file
start parsing file
startDocument
reading securities
endDocument
Start loading splits
Loaded 3286 elements
Loaded 34 elements
Loaded 3321 elements

Save as database does not work for me :(

Here's what I see on CLI

WebConnect: Try to connect to WebConnect server
WebConnect: Connect to server failed
WebConnect: Running in server mode
Plugins: checkprinting loaded
Plugins: csvexporter loaded
Plugins: csvimporter loaded
Plugins: gncimporter loaded
Plugins: icalendarexporter loaded
Plugins: qifexporter loaded
Plugins: qifimporter loaded
Plugins: reconciliation report loaded
Plugins: sqlstorage loaded
Cost center model created with items 0
Payees model created with items 0
reading file
start parsing file
startDocument
reading securities
endDocument
Start loading splits
Loaded 3286 elements
Loaded 34 elements
Loaded 3321 elements

IMHO it's not possible right now and is not scope of this patch. KMM fails in mymoneystoragesql_p.h in below method. As you can see it cannot be successful until ported. Saving as database without IBAN data works.

bool setupStoragePlugin(QString iid)
{
  Q_Q(MyMoneyStorageSql);
  // setupDatabase has to be called every time because this simple technique to check if was updated already
  // does not work if a user opens another file
  // also the setup is removed if the current database transaction is rolled back
  if (iid.isEmpty() /*|| m_loadedStoragePlugins.contains(iid)*/)
    return false;

  QString errorMsg;
  // TODO: port KF5 (needed for payeeidentifier plugin)
#if 0
  KMyMoneyPlugin::storagePlugin* plugin = KServiceTypeTrader::createInstanceFromQuery<KMyMoneyPlugin::storagePlugin>(
    QLatin1String("KMyMoney/sqlStoragePlugin"),
    QString("'%1' ~in [X-KMyMoney-PluginIid]").arg(iid.replace(QLatin1Char('\''), QLatin1String("\\'"))),
    0,
    QVariantList(),
    &errorMsg
  );
#else
  KMyMoneyPlugin::storagePlugin* plugin = 0;
#endif

  if (plugin == 0)
    throw MYMONEYEXCEPTION(QString("Could not load sqlStoragePlugin '%1', (error: %2)").arg(iid, errorMsg));

  MyMoneyDbTransaction t(*q, Q_FUNC_INFO);
  if (plugin->setupDatabase(*q)) {
    m_loadedStoragePlugins.insert(iid);
    return true;
  }

  throw MYMONEYEXCEPTION(QString("Could not install sqlStoragePlugin '%1' in database.").arg(iid));
}
tbaumgart accepted this revision.Feb 18 2018, 7:10 AM

Is there a reason, why you keep code in KMyMoneyApp commented and don't remove it? E.g. slotOpenDatabase() ) Other than that it looks OK to me.

I leave it up to you to decide if you want to change the stuff I remarked or not before you land this change.

This revision is now accepted and ready to land.Feb 18 2018, 7:10 AM
This revision was automatically updated to reflect the committed changes.

Is there a reason, why you keep code in KMyMoneyApp commented and don't remove it? E.g. slotOpenDatabase() ) Other than that it looks OK to me.

It's because I plan more changes in that area and find it useful to be there in case something goes wrong and I haven't foreseen it.