Re: Review Request 130166: Add initial SQLite-based backend for the history plugin

Paulo Lieuthier <[email protected]> Sun, 25 Jun 2017 17:52:18 -0000
Newsgroups gmane.comp.kde.devel.kopete
Message-ID <[email protected]>

> On June 25, 2017, 2:39 p.m., Pali Rohár wrote:
> > plugins/history/backends/sqlite/sqlitehistorybackend.cpp, line 78
> > <https://git.reviewboard.kde.org/r/130166/diff/1/?file=496733#file496733line78>
> >
> >     Why it is needed before storing message into SQLite database?

I just copied it from the original plugin. It's probably not needed, this is a work in progress.


> On June 25, 2017, 2:39 p.m., Pali Rohár wrote:
> > plugins/history/backends/sqlite/sqlitedatabase.h, line 23
> > <https://git.reviewboard.kde.org/r/130166/diff/1/?file=496730#file496730line23>
> >
> >     Really singleton? This would break unloading plugin.

Humm, I didn't antecipate that. What do you suggest? Every new window creates a new instance of the history logger, but all have to use the sabe database instance. It may also include concurrency bugs. I'm going take a look at the unloading issue.


> On June 25, 2017, 2:39 p.m., Pali Rohár wrote:
> > plugins/history/backends/sqlite/sqlitehistorybackend.cpp, line 49
> > <https://git.reviewboard.kde.org/r/130166/diff/1/?file=496733#file496733line49>
> >
> >     SQL injections

Good point. I'll add that to the timeline.


- Paulo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130166/#review103361
-----------------------------------------------------------


On June 25, 2017, 1:38 p.m., Paulo Lieuthier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130166/
> -----------------------------------------------------------
> 
> (Updated June 25, 2017, 1:38 p.m.)
> 
> 
> Review request for Kopete, Laurent Montel and Pali Rohár.
> 
> 
> Repository: kopete
> 
> 
> Description
> -------
> 
> Having built a façade for the history plugin backends [1], this request proposes the long-awaited SQLite-based backend.
> 
> [1] https://git.reviewboard.kde.org/r/130164/
> 
> 
> Diffs
> -----
> 
>   libkopete/kopetemessage.h b5ed066838dfb708459dfe6e61e1de7bbc82ac70 
>   libkopete/ui/kopeteview.h 070a3b8a227acadc21aab28bcc29223585c81862 
>   plugins/CMakeLists.txt 6216058ef93d1c4ae7860331b6e7ef0d8be8fce2 
>   plugins/history/CMakeLists.txt ce90b84a3b60a15db725c29cdb8b721545503ada 
>   plugins/history/backends/historybackend.h PRE-CREATION 
>   plugins/history/backends/historybackendfactory.h PRE-CREATION 
>   plugins/history/backends/historybackendfactory.cpp PRE-CREATION 
>   plugins/history/backends/sqlite/sqlitedatabase.h PRE-CREATION 
>   plugins/history/backends/sqlite/sqlitedatabase.cpp PRE-CREATION 
>   plugins/history/backends/sqlite/sqlitehistorybackend.h PRE-CREATION 
>   plugins/history/backends/sqlite/sqlitehistorybackend.cpp PRE-CREATION 
>   plugins/history/backends/xml/xmlhistorybackend.h PRE-CREATION 
>   plugins/history/backends/xml/xmlhistorybackend.cpp PRE-CREATION 
>   plugins/history/converter.cpp 3175beac164b673201dff785890bc4d8b1cc68df 
>   plugins/history/historydialog.cpp c88ce515ec11844ea1c1c1c4451fdeeab2616439 
>   plugins/history/historyguiclient.cpp 41745bfe090eeda437c4fca3072f9039b2af52a8 
>   plugins/history/historylogger.h ad60896108d5d02e390d09db49d121fdf2f8517b 
>   plugins/history/historylogger.cpp 66b627030f9baf78d98c010c3cc2dc3e7cc08c99 
>   plugins/history/historyplugin.cpp 48cd98010c2f3e65ff76ce3b6e5a43c92e988478 
>   plugins/history/historypreferences.cpp 9c64b44184923aa92fe9fb252bfc5dc4d868b908 
> 
> Diff: https://git.reviewboard.kde.org/r/130166/diff/
> 
> 
> Testing
> -------
> 
> I only tested 1:1 conversations so far, using the Testbed protocol.
> 
> 
> Thanks,
> 
> Paulo Lieuthier
> 
>