Merge lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into lp:~ubuntu-shorts-dev/ubuntu-rssreader-app/trunk

[Joey Chan]

bugs fixed, especially add a function "addArticles" to avoid the hard drive performance issue

bugs fixed, especially add a function "addArticles" to avoid the hard drive performance issue

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:14
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/6/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-quantal-amd64-ci/3
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-raring-amd64-ci/6

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/6/rebuild

review: Approve (continuous-integration)

[Roman Shchekin (mrqtros)]

Joey, I found some issues in your code, I will explain them later, when I'll have access to PC.

But in the same time I found good solutions, like json.stringify and so on. We must merge our implementations of addArticles to get best one.

19.06.13 11:52 Joey Chan написал(а):

Joey Chan has proposed merging lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into lp:ubuntu-rssreader-app.

Commit message:
bugs fixed, especially add a function "addArticles" to avoid the hard drive performance issue

Requested reviews:
Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)

For more details, see:

https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257

bugs fixed, especially add a function "addArticles" to avoid the hard drive performance issue

--
https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257

Your team Ubuntu RSS Feed Reader Developers is requested to review the proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into lp:ubuntu-rssreader-app.

[Joey Chan (qqworini)]

Good, wait for your further email

2013/6/19 Roman Shchekin <email address hidden>

> Joey, I found some issues in your code, I will explain them later, when
> I'll have access to PC.
>
> But in the same time I found good solutions, like json.stringify and so
> on. We must merge our implementations of addArticles to get best one.
>
>
> 19.06.13 11:52 Joey Chan написал(а):
>
> Joey Chan has proposed merging
> lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> lp:ubuntu-rssreader-app.
>
>
> Commit message:
> bugs fixed, especially add a function "addArticles" to avoid the hard
> drive performance issue
>
>
> Requested reviews:
> Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)
>
>
> For more details, see:
>
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
>
>
> bugs fixed, especially add a function "addArticles" to avoid the hard
> drive performance issue
>
> --
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
>
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> lp:ubuntu-rssreader-app.
>
>
>
>
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
> You are the owner of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed.
>

[lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed]

15. By Joey Chan on 2013-06-19

some stupid bugs fixed >_<

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:15
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/7/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-quantal-amd64-ci/4
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-raring-amd64-ci/7

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/7/rebuild

review: Approve (continuous-integration)

[lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed]

16. By Joey Chan on 2013-06-19

stupid bugs fixed ........ >_<

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:16
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/8/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-quantal-amd64-ci/5
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-rssreader-app-raring-amd64-ci/8

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-rssreader-app-ci/8/rebuild

review: Approve (continuous-integration)

[Roman Shchekin (mrqtros)]

Joey, sry for late answer!

Look this line

if (params.isAll || params == undefined) // or miss params

second condition will never return true. Swap conditions.
Cycle in function addArticles contains a lot of additional temp variables.
I think we must use minimum of them.
But if performance satisfied you, code is ok :)

And last advice - for back compatibility save function addArticle :) Mb it
will be helpful for us later!

BR,
Roman.

2013/6/19 Joey Chan <email address hidden>

> Good, wait for your further email
>
>
> 2013/6/19 Roman Shchekin <email address hidden>
>
> > Joey, I found some issues in your code, I will explain them later, when
> > I'll have access to PC.
> >
> > But in the same time I found good solutions, like json.stringify and so
> > on. We must merge our implementations of addArticles to get best one.
> >
> >
> > 19.06.13 11:52 Joey Chan написал(а):
> >
> > Joey Chan has proposed merging
> > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> > lp:ubuntu-rssreader-app.
> >
> >
> > Commit message:
> > bugs fixed, especially add a function "addArticles" to avoid the hard
> > drive performance issue
> >
> >
> > Requested reviews:
> > Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)
> >
> >
> > For more details, see:
> >
> >
> >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
> >
> >
> > bugs fixed, especially add a function "addArticles" to avoid the hard
> > drive performance issue
> >
> > --
> >
> >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
> >
> > Your team Ubuntu RSS Feed Reader Developers is requested to review the
> > proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed
> into
> > lp:ubuntu-rssreader-app.
> >
> >
> >
> >
> >
> >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
> > You are the owner of
> lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed.
> >
>
> --
>
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-fixed/+merge/170257
> Your team Ubuntu RSS Feed Reader Developers is requested to review the
> proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> lp:ubuntu-rssreader-app.
>

[Joey Chan (qqworini)]

Hi Roman,

1. for "params.isAll"
U R right, so I changed them as below codes
if (params == undefined) // in case someone miss the parameters
        xxxxxxxxxxxx ;
else if (params.isAll)
        xxxxxxxxxxxx ;

2. for "addArticles" function
I use those temp variables in order to prevent the js's 'undefined' value stopping the whole function process. And performance so far so good :)

3. for keeping "addArticle" function
I agree with u, I already kept this function in the source code

> Joey, sry for late answer!
>
> Look this line
>
> if (params.isAll || params == undefined) // or miss params
>
> second condition will never return true. Swap conditions.
> Cycle in function addArticles contains a lot of additional temp variables.
> I think we must use minimum of them.
> But if performance satisfied you, code is ok :)
>
> And last advice - for back compatibility save function addArticle :) Mb it
> will be helpful for us later!
>
> BR,
> Roman.
>
>
> 2013/6/19 Joey Chan <email address hidden>
>
> > Good, wait for your further email
> >
> >
> > 2013/6/19 Roman Shchekin <email address hidden>
> >
> > > Joey, I found some issues in your code, I will explain them later, when
> > > I'll have access to PC.
> > >
> > > But in the same time I found good solutions, like json.stringify and so
> > > on. We must merge our implementations of addArticles to get best one.
> > >
> > >
> > > 19.06.13 11:52 Joey Chan написал(а):
> > >
> > > Joey Chan has proposed merging
> > > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> > > lp:ubuntu-rssreader-app.
> > >
> > >
> > > Commit message:
> > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > drive performance issue
> > >
> > >
> > > Requested reviews:
> > > Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)
> > >
> > >
> > > For more details, see:
> > >
> > >
> > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> fixed/+merge/170257
> > >
> > >
> > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > drive performance issue
> > >
> > > --
> > >
> > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> fixed/+merge/170257
> > >
> > > Your team Ubuntu RSS Feed Reader Developers is requested to review the
> > > proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed
> > into
> > > lp:ubuntu-rssreader-app.
> > >
> > >
> > >
> > >
> > >
> > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> fixed/+merge/170257
> > > You are the owner of
> > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed.
> > >
> >
> > --
> >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> fixed/+merge/170257
> > Your team Ubuntu RSS Feed Reader Developers is requested to review the
> > proposed merge of lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> > lp:ubuntu-rssreader-app.
> >

[Roman Shchekin (mrqtros)]

That's cool, so do you need to request merge of new version?

2013/6/20 Joey Chan <email address hidden>

> Hi Roman,
>
> 1. for "params.isAll"
> U R right, so I changed them as below codes
> if (params == undefined) // in case someone miss the parameters
> xxxxxxxxxxxx ;
> else if (params.isAll)
> xxxxxxxxxxxx ;
>
> 2. for "addArticles" function
> I use those temp variables in order to prevent the js's 'undefined' value
> stopping the whole function process. And performance so far so good :)
>
> 3. for keeping "addArticle" function
> I agree with u, I already kept this function in the source code
>
> > Joey, sry for late answer!
> >
> > Look this line
> >
> > if (params.isAll || params == undefined) // or miss params
> >
> > second condition will never return true. Swap conditions.
> > Cycle in function addArticles contains a lot of additional temp
> variables.
> > I think we must use minimum of them.
> > But if performance satisfied you, code is ok :)
> >
> > And last advice - for back compatibility save function addArticle :) Mb
> it
> > will be helpful for us later!
> >
> > BR,
> > Roman.
> >
> >
> > 2013/6/19 Joey Chan <email address hidden>
> >
> > > Good, wait for your further email
> > >
> > >
> > > 2013/6/19 Roman Shchekin <email address hidden>
> > >
> > > > Joey, I found some issues in your code, I will explain them later,
> when
> > > > I'll have access to PC.
> > > >
> > > > But in the same time I found good solutions, like json.stringify and
> so
> > > > on. We must merge our implementations of addArticles to get best one.
> > > >
> > > >
> > > > 19.06.13 11:52 Joey Chan написал(а):
> > > >
> > > > Joey Chan has proposed merging
> > > > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> > > > lp:ubuntu-rssreader-app.
> > > >
> > > >
> > > > Commit message:
> > > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > > drive performance issue
> > > >
> > > >
> > > > Requested reviews:
> > > > Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)
> > > >
> > > >
> > > > For more details, see:
> > > >
> > > >
> > > >
> > >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > fixed/+merge/170257
> > > >
> > > >
> > > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > > drive performance issue
> > > >
> > > > --
> > > >
> > > >
> > >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > fixed/+merge/170257
> > > >
> > > > Your team Ubuntu RSS Feed Reader Developers is requested to review
> the
> > > > proposed merge of
> lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed
> > > into
> > > > lp:ubuntu-rssreader-app.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > fixed/+merge/170257
> > > > You are the owner of
> > > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed.
> > > >
> > >
> > > --
> > >
> > >
> https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > fixed/+merge/170257
> > > Your team Ubuntu RSS Feed Reader Developers is requested to review the
> > > proposed merge of lp:~qqworini/ubuntu-rssreader-...

[Joey Chan (qqworini)]

yes pls :) just approve it

> That's cool, so do you need to request merge of new version?
>
>
> 2013/6/20 Joey Chan <email address hidden>
>
> > Hi Roman,
> >
> > 1. for "params.isAll"
> > U R right, so I changed them as below codes
> > if (params == undefined) // in case someone miss the parameters
> > xxxxxxxxxxxx ;
> > else if (params.isAll)
> > xxxxxxxxxxxx ;
> >
> > 2. for "addArticles" function
> > I use those temp variables in order to prevent the js's 'undefined' value
> > stopping the whole function process. And performance so far so good :)
> >
> > 3. for keeping "addArticle" function
> > I agree with u, I already kept this function in the source code
> >
> > > Joey, sry for late answer!
> > >
> > > Look this line
> > >
> > > if (params.isAll || params == undefined) // or miss params
> > >
> > > second condition will never return true. Swap conditions.
> > > Cycle in function addArticles contains a lot of additional temp
> > variables.
> > > I think we must use minimum of them.
> > > But if performance satisfied you, code is ok :)
> > >
> > > And last advice - for back compatibility save function addArticle :) Mb
> > it
> > > will be helpful for us later!
> > >
> > > BR,
> > > Roman.
> > >
> > >
> > > 2013/6/19 Joey Chan <email address hidden>
> > >
> > > > Good, wait for your further email
> > > >
> > > >
> > > > 2013/6/19 Roman Shchekin <email address hidden>
> > > >
> > > > > Joey, I found some issues in your code, I will explain them later,
> > when
> > > > > I'll have access to PC.
> > > > >
> > > > > But in the same time I found good solutions, like json.stringify and
> > so
> > > > > on. We must merge our implementations of addArticles to get best one.
> > > > >
> > > > >
> > > > > 19.06.13 11:52 Joey Chan написал(а):
> > > > >
> > > > > Joey Chan has proposed merging
> > > > > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed into
> > > > > lp:ubuntu-rssreader-app.
> > > > >
> > > > >
> > > > > Commit message:
> > > > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > > > drive performance issue
> > > > >
> > > > >
> > > > > Requested reviews:
> > > > > Ubuntu RSS Feed Reader Developers (ubuntu-rssreader-dev)
> > > > >
> > > > >
> > > > > For more details, see:
> > > > >
> > > > >
> > > > >
> > > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > > fixed/+merge/170257
> > > > >
> > > > >
> > > > > bugs fixed, especially add a function "addArticles" to avoid the hard
> > > > > drive performance issue
> > > > >
> > > > > --
> > > > >
> > > > >
> > > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > > fixed/+merge/170257
> > > > >
> > > > > Your team Ubuntu RSS Feed Reader Developers is requested to review
> > the
> > > > > proposed merge of
> > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed
> > > > into
> > > > > lp:ubuntu-rssreader-app.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > https://code.launchpad.net/~qqworini/ubuntu-rssreader-app/database-bug-
> > > fixed/+merge/170257
> > > > > You are the owner of
> > > > lp:~qqworini/ubuntu-rssreader-app/database-bug-fixed.
> > > > >
> > >...

[Roman Shchekin (mrqtros)]

review: Approve

=== modified file 'databasemodule_v2.js'
--- databasemodule_v2.js	2013-06-15 16:01:24 +0000
+++ databasemodule_v2.js	2013-06-19 14:52:27 +0000
@@ -58,7 +58,7 @@
 //                // ...
 //            case "all": // Check all tables existing.
     transaction.executeSql('PRAGMA foreign_keys = ON;')   // enable foreign key support
-    transaction.executeSql("CREATE TABLE IF NOT EXISTS feed  (id  INTEGER  NOT NULL PRIMARY KEY AUTOINCREMENT,source  VARCHAR(99)  NULL,title  VARCHAR(99)  NULL,link  VARCHAR(99)  NULL, description  TEXT  NULL, status  char(1)  NULL DEFAULT '0', pubdate  INTEGER  NULL,image  VARCHAR(99)  NULL);") // <- pubDate? I saw only lastBuildDate
+    transaction.executeSql("CREATE TABLE IF NOT EXISTS feed  (id  INTEGER  NOT NULL PRIMARY KEY AUTOINCREMENT,source  VARCHAR(99)  NULL,title  VARCHAR(99)  NULL,link  VARCHAR(99)  NULL, description  TEXT  NULL, status  char(1)  NULL DEFAULT '0', pubdate  INTEGER  NULL,image  VARCHAR(99)  NULL, count INTEGER NULL DEFAULT 0);") // add "count", count user click
     transaction.executeSql("CREATE TABLE IF NOT EXISTS tag  (id  INTEGER  NOT NULL PRIMARY KEY AUTOINCREMENT,name  VARCHAR(99)  NOT NULL UNIQUE );")
     transaction.executeSql("CREATE TABLE IF NOT EXISTS feed_tag  (id  INTEGER  NOT NULL PRIMARY KEY AUTOINCREMENT,feed_id  INTEGER  NULL,tag_id  INTEGER  NULL,FOREIGN KEY(feed_id) REFERENCES feed(id) on delete cascade);")
     transaction.executeSql("CREATE TABLE IF NOT EXISTS article ( id  INTEGER  PRIMARY KEY AUTOINCREMENT NOT NULL, title  VARCHAR(99)  NULL, content TEXT NULL, link  VARCHAR(99)  NULL, description  TEXT  NULL, pubdate  INTEGER  NULL, status  char(1)  NULL DEFAULT '0', favourite  char(1)  NULL DEFAULT '0', image  VARCHAR(99)  NULL, guid VARCHAR(99) NULL, feed_id  INTEGER  NULL,count INTEGER NULL DEFAULT 0);")
@@ -153,14 +153,14 @@
     return dbResult
 }
 
-function updateFeedByXml(id, link, description, title)   // from xml file
+function updateFeedByXml(id, link, description, title, pubdate)   // from xml file
 {
     var db = openStdDataBase()
     var dbResult
     db.transaction(function (tx) {
 //        ensureFeedTableExists(tx)
-        dbResult = tx.executeSql('UPDATE feed SET link=?, description=?, title=? WHERE id=?',
-                                 [link, description, title, id])
+        dbResult = tx.executeSql('UPDATE feed SET link=?, description=?, title=?, pubdate=? WHERE id=?',
+                                 [link, description, title, pubdate, id])
         console.log("feed updateFeedByXml, AFFECTED ROWS: ", dbResult.rowsAffected)
     }
     )
@@ -233,14 +233,17 @@
 //    return dbResult;
 //}
 
-function loadArticles(params) {
+function loadArticles(params)   // params = {"isAll": true/false, "feedId": id}
+{
     var db = openStdDataBase()
     var dbResult
 
     db.transaction(function(tx) {
-        if (params.isAll)
-            dbResult = tx.executeSql('SELECT * FROM article')
-        else dbResult = tx.executeSql('SELECT * FROM article WHERE feed_id = ?', [params.feedId])
+        if (params == undefined) // miss params
+            dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name FROM article inner join feed on article.feed_id = feed.id ')
+        else if (params.isAll)
+            dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name FROM article inner join feed on article.feed_id = feed.id')
+        else dbResult = tx.executeSql('SELECT article.*, feed.title as feed_name FROM article inner join feed on article.feed_id = feed.id WHERE article.feed_id = ?', [params.feedId])
         console.log("loadArticles, SELECTED ROWS", dbResult.rows.length)
     }
     )
@@ -269,6 +272,47 @@
     return dbResult;
 }
 
+/*
+  this function is for avoiding hard drive performance issue,
+  pass model and feed id as parameters to this function, it will automaticly insert all the articles into database
+ */
+function addArticles(model, feed_id)
+{
+    var dbResult
+
+    var db = openStdDataBase()
+    db.transaction(function (tx) {
+//        ensureFeedTableExists(tx)
+
+        var article;
+        for (var i = 0; i < model.count; i++) {
+
+            article = model.get(i)
+            var title =  article.title == undefined ? "" : article.title
+            var guid =  article.guid == undefined ? Qt.md5(title) : article.guid
+            var link =  article.link == undefined ? "" : article.link
+            var pubDate =  article.pubDate == undefined ? "" : article.pubDate
+            var description =  article.description == undefined ? "" : article.description
+            var content =  article.content == undefined ? "" : article.content
+
+            /* Check uniqueness.
+             */
+            dbResult = tx.executeSql("SELECT * FROM article WHERE guid=? AND feed_id=?", [guid, feed_id])
+            if (dbResult.rows.length > 0) {
+                console.log("Database, add article: already exist article with source: ", guid)
+                continue;
+            }
+            dbResult = tx.executeSql('INSERT INTO article (title, content, link, description, pubdate, guid, feed_id) VALUES(?, ?, ?, ?, ?, ?, ?)',
+                                     [title, content, link, description, pubDate, guid, feed_id])
+            console.log("article INSERT ID: ", JSON.stringify(dbResult) )
+        }
+
+//        console.log("article INSERT ID: ", JSON.stringify(dbResult) )
+    }
+    )
+    return dbResult;
+}
+
 // update
 function updateArticleStatus(id, status)
 {