fossar / selfoss

multipurpose rss reader, live stream, mashup, aggregation web application
https://selfoss.aditu.de
GNU General Public License v3.0
2.36k stars 343 forks source link

client: Switch from jQuery.ajax to fetch #1215

Closed jtojnar closed 4 years ago

jtojnar commented 4 years ago

This will bring us one step closer to removing jQuery.

Unfortunately the native promises lack built-in support for aborting so we need to somewhat clumsily return AbortController alongside them.

While at it, I also got rid of the jQuery Deferreds so the only remaining use of jQuery is the DOM manipulation. Hopefully, it will not remain for long.

jtojnar commented 4 years ago

@niol will you be available to review this some time? I can also guide you through the Parcel-based build from #1137 if you need to bring up to speed.

niol commented 4 years ago

I should be able to look into it by tomorrow.

jtojnar commented 4 years ago

Thanks for checking it out. It should be fixed now.

The add source button not functioning was caused by forgetting to call .text() on the fetch’s Response. I checked the rest of the code and found only two other places where we expect HTML instead of JSON and those called it correctly. Hopefully, I did not miss any.

I also had to fix the Sources\Write controller to handle missing tags field since the new field does not send it when there are no tags.

niol commented 4 years ago

With offline storage enabled, the client does not chain load items and stops after items_perpage.

niol commented 4 years ago

And then it seems to load the same entries over and over again.

niol commented 4 years ago

I I shut down the server, it keeps the ajax requests unfinished although the "Connection refused" should terminate them immediately.

jtojnar commented 4 years ago

If I manually trigger selfoss.dbOffline.sendNewStatuses(), it seems to fetch another page. I am not sure what is supposed to trigger the fetch of the next page.

niol commented 4 years ago

when selfoss.dbOffline.newerEntriesMissing is true.

jtojnar commented 4 years ago

That seems to be true here but no luck.

niol commented 4 years ago

I would try the following but I'm stuck with a bundler error.

--- a/assets/js/selfoss-db.js
+++ b/assets/js/selfoss-db.js
@@ -157,16 +157,19 @@ selfoss.dbOnline = {
                     selfoss.dbOffline.storeEntries(data.newItems)
                         .then(function() {
                             selfoss.dbOffline.storeLastUpdate(dataDate);
-
                             selfoss.dbOnline._syncDone();
-
-                            // fetch more if server has more
-                            if (selfoss.dbOffline.newerEntriesMissing) {
-                                selfoss.dbOnline.sync();
-                            }
                         });
                 }

+                if (selfoss.dbOffline.newerEntriesMissing
+                    || selfoss.dbOffline.needsSync) {
+                    // There are still new items to fetch
+                    // or statuses to send
+                    syncing.request.promise.then(function () {
+                        selfoss.dbOffline.sendNewStatuses();
+                    });
+                }
+
                 if ('itemUpdates' in data) {
                     // refresh entry statuses in db and dequeue queued
                     // statuses but do not calculate stats as they are taken
jtojnar commented 4 years ago

Yeah, the bundlers are somewhat finicky in my experience. If I do git rebase with npm run dev running it breaks horribly, and I need to delete assets/.cache and restart it. OR sometimes on a syntax error, it is just swallowed and I need to restart it to see the error message. Unfortunately, when I tried Webpack it did not seem much better.

That seems to work with a minor tweak:

--- a/assets/js/selfoss-db.js
+++ b/assets/js/selfoss-db.js
@@ -159,14 +159,18 @@ selfoss.dbOnline = {
                             selfoss.dbOffline.storeLastUpdate(dataDate);

                             selfoss.dbOnline._syncDone();
-
-                            // fetch more if server has more
-                            if (selfoss.dbOffline.newerEntriesMissing) {
-                                selfoss.dbOnline.sync();
-                            }
                         });
                 }

+                if (selfoss.dbOffline.newerEntriesMissing
+                    || selfoss.dbOffline.needsSync) {
+                    // There are still new items to fetch
+                    // or statuses to send
+                    selfoss.dbOnline.syncing.request.promise.then(function () {
+                        selfoss.dbOffline.sendNewStatuses();
+                    });
+                }
+
                 if ('itemUpdates' in data) {
                     // refresh entry statuses in db and dequeue queued
                     // statuses but do not calculate stats as they are taken

Thanks.

jtojnar commented 4 years ago

But that patch triggers a sync loop you mention, the last page does not seem to get picked up:

GET http://localhost/selfoss/items/sync?since=2020-09-05T15%3A53%3A06.000Z&tags=true&itemsStatuses=true&itemsSinceId=696&itemsNotBefore=2020-09-05T03%3A06%3A45.277Z&itemsHowMany=51
key value
lastUpdate "2020-09-05 17:53:06"
newItems [ {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, … ]
0 Object { id: 88, datetime: "2020-08-05T16:02:52+02:00", unread: true, … }
1 Object { id: 478, datetime: "2020-08-27T23:00:17+02:00", unread: false, … }
2 Object { id: 631, datetime: "2020-08-27T19:00:03+02:00", unread: true, … }
3 Object { id: 648, datetime: "2020-09-04T09:31:00+02:00", unread: true, … }
4 Object { id: 649, datetime: "2020-08-28T11:19:00+02:00", unread: true, … }
5 Object { id: 650, datetime: "2020-08-27T10:00:00+02:00", unread: true, … }
6 Object { id: 651, datetime: "2020-08-27T08:57:00+02:00", unread: true, … }
7 Object { id: 653, datetime: "2020-08-19T12:45:00+02:00", unread: true, … }
8 Object { id: 654, datetime: "2020-08-06T12:40:00+02:00", unread: true, … }
9 Object { id: 655, datetime: "2020-07-29T07:36:00+02:00", unread: true, … }
10 Object { id: 656, datetime: "2020-07-29T07:22:00+02:00", unread: true, … }
11 Object { id: 657, datetime: "2020-07-28T15:57:00+02:00", unread: true, … }
12 Object { id: 658, datetime: "2020-07-27T09:30:00+02:00", unread: true, … }
13 Object { id: 659, datetime: "2020-07-21T09:55:00+02:00", unread: true, … }
14 Object { id: 660, datetime: "2020-07-17T07:52:00+02:00", unread: true, … }
15 Object { id: 661, datetime: "2020-07-10T17:20:00+02:00", unread: true, … }
16 Object { id: 662, datetime: "2020-07-09T17:17:00+02:00", unread: true, … }
17 Object { id: 663, datetime: "2020-06-24T14:59:00+02:00", unread: true, … }
18 Object { id: 664, datetime: "2020-06-24T12:29:00+02:00", unread: true, … }
19 Object { id: 665, datetime: "2020-06-24T11:52:00+02:00", unread: true, … }
20 Object { id: 666, datetime: "2020-06-22T07:47:00+02:00", unread: true, … }
21 Object { id: 667, datetime: "2020-06-18T13:53:00+02:00", unread: true, … }
22 Object { id: 668, datetime: "2020-06-10T07:36:00+02:00", unread: true, … }
23 Object { id: 669, datetime: "2020-05-15T12:20:00+02:00", unread: true, … }
24 Object { id: 670, datetime: "2020-05-15T08:16:00+02:00", unread: true, … }
25 Object { id: 671, datetime: "2020-05-14T09:23:00+02:00", unread: true, … }
26 Object { id: 672, datetime: "2020-05-11T08:31:00+02:00", unread: true, … }
27 Object { id: 673, datetime: "2020-05-08T16:34:00+02:00", unread: true, … }
28 Object { id: 674, datetime: "2020-05-06T15:11:00+02:00", unread: true, … }
29 Object { id: 675, datetime: "2020-04-30T15:57:00+02:00", unread: true, … }
30 Object { id: 676, datetime: "2020-04-30T10:38:00+02:00", unread: true, … }
31 Object { id: 677, datetime: "2020-04-21T08:42:00+02:00", unread: true, … }
32 Object { id: 678, datetime: "2020-04-12T15:05:00+02:00", unread: true, … }
33 Object { id: 679, datetime: "2020-04-03T12:02:00+02:00", unread: true, … }
34 Object { id: 680, datetime: "2020-04-01T12:35:00+02:00", unread: true, … }
35 Object { id: 681, datetime: "2020-03-17T08:45:00+01:00", unread: true, … }
36 Object { id: 682, datetime: "2020-03-02T16:12:00+01:00", unread: true, … }
37 Object { id: 683, datetime: "2020-02-26T09:25:00+01:00", unread: true, … }
38 Object { id: 684, datetime: "2020-02-19T07:12:00+01:00", unread: true, … }
39 Object { id: 685, datetime: "2020-02-14T09:02:00+01:00", unread: true, … }
40 Object { id: 686, datetime: "2020-02-14T07:00:00+01:00", unread: true, … }
41 Object { id: 687, datetime: "2020-02-13T09:30:00+01:00", unread: true, … }
42 Object { id: 688, datetime: "2020-09-04T17:36:24+02:00", unread: true, … }
43 Object { id: 689, datetime: "2020-09-04T17:32:50+02:00", unread: true, … }
44 Object { id: 690, datetime: "2020-08-19T16:01:39+02:00", unread: true, … }
45 Object { id: 691, datetime: "2020-09-03T08:19:17+02:00", unread: true, … }
46 Object { id: 692, datetime: "2020-09-03T08:19:12+02:00", unread: true, … }
47 Object { id: 693, datetime: "2020-09-03T08:19:06+02:00", unread: true, … }
48 Object { id: 694, datetime: "2020-09-03T08:18:59+02:00", unread: true, … }
49 Object { id: 695, datetime: "2020-07-21T12:13:14+02:00", unread: true, … }
50 Object { id: 696, datetime: "2020-07-13T19:00:37+02:00", unread: true, … }
lastId 754
niol commented 4 years ago

The sync loop seems to be specific to sqlite and how we compare dates as simple strings.

We should either use specific syntax for sqlite queries or ensure compared dates share the same timezone.

For instance, the following seems to be fixing the problem.

--- a/src/controllers/Items/Sync.php
+++ b/src/controllers/Items/Sync.php
@@ -56,11 +56,12 @@ class Sync {
         }

         $since = new \DateTime($params['since']);
+        $since->setTimeZone(new \DateTimeZone(date_default_timezone_get()));

         $last_update = new \DateTime($this->itemsDao->lastUpdate());

         $sync = [
-            'lastUpdate' => $last_update->format('Y-m-d H:i:s'),
+            'lastUpdate' => $last_update->format(\DateTime::ATOM),
         ];

         $sinceId = 0;
@@ -73,6 +74,7 @@ class Sync {
                     // only send 1 day worth of items
                     $notBefore = new \DateTime();
                     $notBefore->sub(new \DateInterval('P1D'));
+                    $notBefore->->setTimeZone(new \DateTimeZone(date_default_timezone_get()));
                 }

                 $itemsHowMany = $f3->get('items_perpage');