brantje / nextnote

A full Evernote / OneNote style WYSIWYG note editor for Nextcloud / ownCloud. Join our telegram at: https://t.me/NextNote
GNU Affero General Public License v3.0
163 stars 19 forks source link

downloading all data before display renders the interface unusable #37

Closed mulander closed 6 years ago

mulander commented 7 years ago

Hello,

I am in progress of migrating an owncloud installation to nextcloud.

I was dumbfounded why my ownnotes render properly but my wife has an empty list. It appears that she is a far more detailed note taker and while my list is tiny she has a quite hefty list.

Her list of notes is empty even though the db reveals a lot of notes

nextcloud=# select uid, count(1) from oc_ownnote group by uid;         
    uid    | count 
-----------+-------
 nemessica |   702
 mulander  |     8
(2 rows)

she was able to use those notes under ownnote before the nextcloud move without issues.

I started to diagnose and I think the reason is nextnote trying to download ALL of the note data before rendering any UI and hence failing the request silently. Here is a screenshot from the chrome inspector tool.

2017-08-26-001455_1758x329_scrot

you can see the request failing with no feedback after around 7 MB of data and slightly above 1 minute of request time passes.

I did check that I can add new notes (they show up in the DB) so the interaction does work. The only trouble is the interface being unusable as it's not able to download all of notes in a single request.

I'm not sure which side terminates the connection (server or browser) but I think it's meaningless - the plugin is useless if it needs to download a full list of notes and their contents before rendering anything - huge bandwidth waster.

Looking forward to help diagnose and fix this.

mulander commented 7 years ago

In findNotesFromUserthe notes list is turned into entities https://github.com/brantje/nextnote/blame/master/lib/Db/NextNoteMapper.php#L98

by calling getNoteParts https://github.com/brantje/nextnote/blame/master/lib/Db/NextNoteMapper.php#L218

this was not the case in the old ownnote plugin https://github.com/Fmstrat/ownnote/blob/master/lib/backend.php#L267 which apparently only fetched the parts when working on a single note (edit,save,delete).

mulander commented 7 years ago

To test my assumption I commented out the fetch here: https://github.com/brantje/nextnote/blame/master/lib/Db/NextNoteMapper.php#L218

with that change my wife's notes load the listing properly in a fraction of time and bandwith required

2017-08-26-102919_818x272_scrot

now, obviously this leads to no notes being properly displayed when clicked as there is no fetch mechanism/endpoint when a note is clicked.

enoch85 commented 7 years ago

@mulander Just have to say, I love your debugging. Really great stuff!

@brantje Still much workload on your end?

mulander commented 7 years ago

With the following diff I have working notes. This disables the cache when a note is clicked to be edited resulting in a direct fetch and I moved the note parts fetching to be only executed in the single note fetch path.

I assume a more throughout solution could be employed with an additional flag denoting if the note parts were already loaded but I'm fairly happy with this so far. I will open up a pull request with the diff in a moment.

The screenshot present a fetch being made when the note is clicked 2017-08-26-111408_658x94_scrot

diff --git a/js/app/services/NoteService.js b/js/app/services/NoteService.js
index 809322a..caa78ff 100644
--- a/js/app/services/NoteService.js
+++ b/js/app/services/NoteService.js
@@ -43,14 +43,10 @@
                                getNoteById: function(noteId) {
                                        noteId = parseInt(noteId);
                                        var deferred = $q.defer();
-                                       if ($rootScope.notes && $rootScope.notes.hasOwnProperty(noteId)) {
-                                               deferred.resolve(new NoteFactory($rootScope.notes[noteId]));
-                                       } else {
-                                               NoteFactory.get({id: noteId}, function(note) {
-                                                       $rootScope.notes[note.id] = note;
-                                                       deferred.resolve(note);
-                                               });
-                                       }
+                                       NoteFactory.get({id: noteId}, function(note) {
+                                               $rootScope.notes[note.id] = note;
+                                               deferred.resolve(note);
+                                       });
                                        return deferred.promise;
                                },
                                save: NoteFactory.save,
diff --git a/lib/Db/NextNoteMapper.php b/lib/Db/NextNoteMapper.php
index c56aa8e..2fb7d7d 100644
--- a/lib/Db/NextNoteMapper.php
+++ b/lib/Db/NextNoteMapper.php
@@ -64,6 +64,13 @@ class NextNoteMapper extends Mapper {
                         * @var $note NextNote
                         */
                        $note = $this->makeEntityFromDBResult($item);
+                       /* fetch note parts */
+                       $noteParts = $this->getNoteParts($note);
+                       $partsTxt = implode('', array_map(function ($part) {
+                               return $part['note'];
+                       }, $noteParts));
+                       $note->setNote($arr['note'] . $partsTxt);
+
                        $results[] = $note;
                }
                return array_shift($results);
@@ -215,11 +222,6 @@ class NextNoteMapper extends Mapper {
                $note->setMtime($arr['mtime']);
                $note->setDeleted($arr['deleted']);
                $note->setUid($arr['uid']);
-               $noteParts = $this->getNoteParts($note);
-               $partsTxt = implode('', array_map(function ($part) {
-                       return $part['note'];
-               }, $noteParts));
-               $note->setNote($arr['note'] . $partsTxt);
                return $note;
        }
 }
mulander commented 7 years ago

Pull request sent https://github.com/brantje/nextnote/pull/38

I am running this on our home instance, wife didn't kill me yet so looks like it works OK ;)