Skinny tiddlers will never be loaded

[danielo515]

Hello,

I think I have spotted a problem in the logic that loads the tiddlers using a sync adaptor. Particularly on the line 220 of syncer.js. When you request skinny tiddlers to the server, they have all the information about the tiddler except the text field. So far so good. If the skinny version of the tiddler does not exist on the wiki, it is saved, and here is where the problem appears. If the tiddler is saved to the current wiki store, this includes the revision field. So the next time the tiddler is required, because we already have the latest revision (despite it is skinny) the tiddler is not going to be loaded. On the following snippet is very clear:

                // Ignore the incoming tiddler if it's the same as the revision we've already got
                if(currRevision !== incomingRevision) { //[1]
                    // Do a full load if we've already got a fat version of the tiddler
                    if(tiddler && tiddler.fields.text !== undefined) {
                        // Do a full load of this tiddler
                        self.enqueueSyncTask({
                            type: "load",
                            title: tiddlerFields.title
                        });
                    } else { //[2]
                        // Load the skinny version of the tiddler
                        self.storeTiddler(tiddlerFields);
                    }

This is what happens:

1- the first time we fetch the tiddler, it is skinny, it is new and we do not have any revision information (1) so it goes directly to (2), 1- On the next skinny cicly, we already have all the information of the tiddler, except the text field. Since the tiddler has not changed, we have already the latest revision (1) but without the text field.

This is avoiding tiddlers to be loaded. This is happening at least on the latest version of TW

Regards

[danielo515]

I think I have found the root of the issue.

The problem is on the handleLazyLoadEvent (or maybe on the other responsible, Syncer.prototype.storeTiddler). When a tiddler has been lazy loaded, the storeTiddler method flags it as hasBeenLazyLoaded: true, which is fine IMO. But then, when the handleLazyLoadEvent method decides if it has to load the tiddler using the syncer, it decides to not doing it if the tiddler was lazy loaded. This is totally wrong from my point of view, because if the tiddler has been lazy loaded then the text field is missing, and it is correct to load it. Or maybe you can understand it the other way round, but in any case the result is that the tiddler is not loaded. Below is the handler:

Syncer.prototype.handleLazyLoadEvent = function(title) {
    // Don't lazy load the same tiddler twice
    var info = this.tiddlerInfo[title];
    if(!info || !info.hasBeenLazyLoaded) {
        this.createTiddlerInfo(title);
        this.tiddlerInfo[title].hasBeenLazyLoaded = true;
        // Queue up a sync task to load this tiddler
        this.enqueueSyncTask({
            type: "load",
            title: title
        });     
    }
};

[danielo515]

No one has experienced this issue? @Jermolene, @felixhayashi, @pmario, @twMat ? This works with the file-system adaptor because it returns fat tiddlers, but should fail in any other remote configuration. No one else is using a server-like config of latest version of TW?

[twMat]

(Way too advanced stuff for me ;-) )

[guillefix]

I tried this fix and it seems to be working for me.

I'm just checking that the tiddler has the text field, and using that to set hasBeenLazyLoaded to true or false.

Does this look like a fix?

[danielo515]

I just removed the exclamation sign in front of hasBeenLazyLoaded. I really think that the logic should be inverse, that's why I did that. In any case, the logic is wrong because the same flag is being used for two slightly different things: was it loaded as skinny? and has it been loaded already?

I think @Jermolene is the person to decide what is a fix because he knows what was trying to achieve.

[danielo515]

@Jermolene did you find time to take a look?

[Jermolene]

Hi @danielo515 good catch, and good diagnosis! Fix upcoming.

[danielo515]

Hello @Jermolene ,

Have you considered releasing a patched version of 5.1.13? It's very uncomfortable to use the GIT workflow to take advantage of this patch, and there are lot's of wikis affected as you may have noticed here on GH issues and the TW group forum also.

[llamaqcom]

Dear @Jermolene,

It looks like the problem with lazy-loading on nodejs is still here. When I start tiddlywiki server, $:/core/save/... ($:/core/save/lazy-images and $:/core/save/lazy-all) option is ignored and by default is always $:/core/save/all.

tiddlywiki --version
5.1.14

nodejs --version
v4.8.2

[Jermolene]

Hi @sancho-one please can you show the command line you're using to start the TW server?

[llamaqcom]

@Jermolene I bring you my deep apologies for the false alarm. That was the tricky problem of passing params in the systemd service file. It doesn't allow spacing for the arguments. If someone is trying to run tiddlywiki nodejs server with systemd and has the similar problem, the solution is:

Environment='PARAMS=/var/www/wiki --server 8080 $:/core/save/lazy-all'
ExecStart=/usr/local/bin/tiddlywiki $PARAMS

And thanks for the great and robust product! I really enjoy it!

[Jermolene]

Hi @sancho-one absolutely no problem, thanks for taking the time to post, and I'm delighted that things are working.

[linonetwo]

https://github.com/Jermolene/TiddlyWiki5/blob/07198b9cda12da82fc66dcf0589d6a9caab1cdf6/core/modules/syncer.js#L227-L228

Why is this issue closed? These lines of code never changed since this issue.

revision is an optional field, if a sync adapter doesn't provide this field, it will always be "", then tiddler will never be updated!

[Jermolene]

Hi @linonetwo the original issue was addressed in 18dd8d4433b3bc58f600f01e74844e9ec4534e11 (see the comment above).

[linonetwo]

I think what solved was this:

I think I have found the root of the issue.

The problem is on the handleLazyLoadEvent (or maybe on the other responsible, Syncer.prototype.storeTiddler).........

Not this:

Hello,

I think I have spotted a problem in the logic that loads the tiddlers using a sync adaptor. Particularly on the line 220 of syncer.js. When you request skinny tiddlers to the server, they have all the information about the tiddler except the text field. So far so good. If the skinny version of the tiddler does not exist on the wiki, it is saved, and here is where the problem appears. If the tiddler is saved to the current wiki store, this includes the revision field. So the next time the tiddler is required, because we already have the latest revision (despite it is skinny) the tiddler is not going to be loaded. On the following snippet is very clear:

                // Ignore the incoming tiddler if it's the same as the revision we've already got
                if(currRevision !== incomingRevision) { //[1]
                    // Do a full load if we've already got a fat version of the tiddler
                    if(tiddler && tiddler.fields.text !== undefined) {
                        // Do a full load of this tiddler
                        self.enqueueSyncTask({
                            type: "load",
                            title: tiddlerFields.title
                        });
                    } else { //[2]
                        // Load the skinny version of the tiddler
                        self.storeTiddler(tiddlerFields);
                    }

This is what happens:

1- the first time we fetch the tiddler, it is skinny, it is new and we do not have any revision information (1) so it goes directly to (2), 1- On the next skinny cicly, we already have all the information of the tiddler, except the text field. Since the tiddler has not changed, we have already the latest revision (1) but without the text field.

This is avoiding tiddlers to be loaded. This is happening at least on the latest version of TW

Regards

I think what @danielo515 said is reasonable, if a syncAdaptor doesn't use revision, then currRevision !== incomingRevision will always false.

Though I decided to assign revision using MD5 of tiddler's fields now. So this won't be a problem for me.