TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
8.02k stars 1.19k forks source link

Way to add/delete a tiddler without triggering the sync mechanism #2698

Open danielo515 opened 7 years ago

danielo515 commented 7 years ago

Hello,

Currently the sync mechanism pulls tiddlers from the sync adaptor periodically and pushes deletions and additions as soon as they happen. This makes a lot of sense for most of the scenarios but there are some others where you would like a bit more of granularity. For example, in case of real time synchronization you don't want to wait until the sync mechanism asks you for changes, you want to push them immediately. But doing that, for example adding a tiddler, will immediately trigger the sync mechanism which will ask the sync adaptor to save a change that is actually coming from the sync adaptor. It could be even worse in case of deletions, where the sync mechanism ask the adapter to delete something that is already deleted. Is there any way to skip this process? Maybe a silent option to the add tiddler method?

Regards

Jermolene commented 7 years ago

For example, in case of real time synchronization you don't want to wait until the sync mechanism asks you for changes, you want to push them immediately

Who is the "you"? The client or the server?

Is the problem here that the polling interval used by the sync adaptor to read changes from the server is too long, and you'd prefer real-time updates from the server to the browser?

danielo515 commented 7 years ago

Who is the "you"? The client or the server?

The sync adapter, which is in turn a client of the server.

Is the problem here that the polling interval used by the sync adaptor to read changes from the server is too long

Not actually. It can happen that no updates takes place in an entire hour, so pooling every minute may be too much, while sometimes several updates can take place within a single minute, then pooling every minute may feel too long. I would like to keep the current scheduled pooling while I have the ability to notify tiddlywiki about incoming changes coming from the server to my client (the sync adapter).

Jermolene commented 7 years ago

I would like to keep the current scheduled pooling while I have the ability to notify tiddlywiki about incoming changes coming from the server to my client (the sync adapter).

So, you want to keep the browser polling the server every 60 seconds, but you want the ability for the sync adaptor to inject tiddlers whenever it wants?

The syncer module keeps track of a bunch of information about each tiddler that is subject to syncing. If your sync adaptor just reaches into the store and makes changes then that data is not going to be updated.

So, what needs to happen here is to arrive at a way for the syncadaptor to tell the syncer manager that there have been changes on the server.

That's basically the same logic as a polling operation, except that it is triggered by the sync adaptor rather than the syncer.

So, it sounds like what we need to do is support asynchronous notifications of tiddler changes from the sync adaptor to the syncer. That is what I described earlier as "real-time updates from the server to the browser"; aka push events.

danielo515 commented 7 years ago

So, it sounds like what we need to do is support asynchronous notifications of tiddler changes from the sync adaptor to the syncer. That is what I described earlier as "real-time updates from the server to the browser"; aka push events.

Now it is crystal clear. What you have described sounds generic enough to handle any use-case, including mine.

Do you have any idea to implement it?

Jermolene commented 7 years ago

Do you have any idea to implement it?

I'm interested in using "Server Sent Events"; they are not as widely used as they should be, but are a good match for our needs:

https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events

They have very good proxy/browser compatibility apart from IE/Edge:

http://caniuse.com/#feat=eventsource

There are a number of polyfills available too:

https://github.com/Modernizr/Modernizr/wiki/HTML5-Cross-Browser-Polyfills#eventsource

danielo515 commented 7 years ago

I was expecting something more generic and TW specific. In my opinion the layer that would use "Server Sent Events" should be on the sync-adaptor, which would be responsible from informing TW sync mechanism. That way we can use any other protocol under the hood, like in my case where the server is a database running on the local browser.

Regards

Jermolene commented 7 years ago

In my opinion the layer that would use "Server Sent Events" should be on the sync-adaptor

Absolutely, SSE is kind of an implementation detail for the tiddlywebadaptor. The syncadaptor interface needs to be updated to allow for an asynchronous callback from the syncadaptor to deliver new tiddlers. We can accomplish that by passing a reference to the syncer to the syncadaptor constructor (via options). Then the syncadaptor would just call this.syncer.deliverTiddlers(tiddlersArray) whenever it needs to.

danielo515 commented 7 years ago

That sounds exceptionally well. I'll take a look at the mentioned method of the syncer and experiment with it a bit. Is it an existing method or just an idea about how can it be?

Regards

Jermolene commented 7 years ago

Is it an existing method or just an idea about how can it be

Just an idea.

The processing for this.syncer.deliverTiddlers(tiddlersArray) would be pretty much the same as the callback embedded in Syncer.prototype.syncFromServer(), so we could refactor the former out of the latter.

danielo515 commented 7 years ago

Hello @Jermolene Digging a bit more on the syncer mechanism, I found that syncer.storeTiddler does not trigger the sync mechanism, so it seems like a feasible way of notifying changes from the server to tiddlywiki. What is the actual purpose of such method?

Regards

pmario commented 7 years ago

syncer.storeTiddler is triggered by syncFromServer ... So it's the other way around. ... The syncer polls for changes every 60 seconds atm.

IMO the easiest way to notify the syncer, that something changed on the server is by adding a startup module, that registers an independent eventHandler, that tirggers a "syncFromServer()" or a "loadSingleTiddler()" in the syncer ... I did something similar for the datarchiveadaptor (which is alpha atm.) ... but the mechanism can be made generic, to work for tiddlyweb and couchdb adaptor too.

I just need to prepare a PR ... The mechanism is simple.

danielo515 commented 7 years ago

Hello @pmario,

My tests revealed that the tiddler passed to syncer.storeTiddler is just added to the wiki. How is that the other way around?

About your proposal I don't see why would you need to register an event handler for something that should be straight communication between the sync adaptor and the sunder module

pmario commented 7 years ago

I don't see why would you need to register an event handler for something that should be straight communication between the sync adaptor and the sunder module

I don't understand the sunder module

the "sycer" is the layer between the TW internal memory and the adaptor. Adaptors implement the low level functions, that talk to the underlaying physical storage in an async way. So in your case it's couchdb and/or pouchdb

The syncer-module talks to the TW internal memory store. eg:

At the moment the syncer uses a "polling" mechanism see: pollTimerId. The .syncFromServer() function activates this.syncadaptor.getSkinnyTiddlers() every 60 seconds.

It doesn't listen to the underlaying adaptors for a: "my external store has changed" event.

If we want to implement that, it's pretty straight forward. There are only a hand full of lines, that need to be changed in the syncer, to achieve that functionality in a backwards compatible way.

pouchdb has such an event listener: https://pouchdb.com/api.html#changes ... There even is a code example:

var changes = db.changes({
  since: 'now',
  live: true,
  include_docs: true
}).on('change', function(change) {
  // handle change and inform the syncer with eg: 
var type = "changed";

// prepare your data, to beconsumed by the syncer

$tw.wiki.dispatchEvent("externalEvent", type, params);

}).on('complete', function(info) {
  // changes() was canceled
}).on('error', function (err) {
  console.log(err);
});

The above can be a startup module, that informs the syncer, that something happened. .. The syncer will trigger the next steps. ... It is relatively straight forward. ...

We just need to modify the syncer a little bit, to enable "externalEvent" handling.

Jermolene commented 7 years ago

Hi @danielo515 @pmario

If a sync adaptor wants to asynchronously bring in tiddlers from the server, it should call $tw.syncer.storeTiddler(). As well as putting the tiddler to the store, the function also updates the metadata held by the syncer to mark the tiddler as being in sync with the server (otherwise the newly added tiddler would be synced to the server immediately after being added).

So, a sync adaptor that is using some kind of push mechanism instead of the current polling should (a) not implement their getSkinnyTiddlers() method and (b) call $tw.syncer.storeTiddler() asynchronously whenever updated tiddlers are loaded from the server.

pmario commented 7 years ago

So, a sync adaptor that is using some kind of push mechanism instead of the current polling should (a) not implement their getSkinnyTiddlers()

I found out, that the getSkinnyTiddlers() name is a bit confusing. .. It actually doesn't matter, if you provide "fat" tiddlers. ... It just works if the adaptor provides "fat" tiddlers.

danielo515 commented 7 years ago

@pmario I'm quite familiar with PouchDB methods and examples. That is the reason why I originally open this issue/feature request.

I don't understand the sunder module

It was a typo, I wanted to write syncer module.

You were close to convince me with your detailed explanation, but seems that Jeremy is validating my assumption about using store tiddler method. I don't know what to think about this now. As I said, introduced an event way of communicating for this is not necessary

pmario commented 7 years ago

IMO your assumption, that the storeTiddler() method doesn't trigger the sync mechanism is wrong.

have a closer look:

pmario commented 7 years ago

If a sync adaptor wants to asynchronously bring in tiddlers from the server, it should call $tw.syncer.storeTiddler().

IMO that's not good. It's possible, that several syncers are active at the same time. see: savetrail.js line 30 The same is true for $tw.syncer ... It depends, which adaptor activated the $tw.syncer variable. ... It's not guaranteed, that it's the same adaptor, that calls $tw.syncer.storeTiddler() ... So calling the global function, without knowing who actually initialized it can cause very subtle problems, that are really hard to find.

IMO my suggested method is save here, Since it implements a new eventListener, that exactly defines, who receives and acts to the message. ... If several syncers catch the event, it's still save. ...

Jermolene commented 7 years ago

Hi @pmario

IMO your assumption, that the storeTiddler() method doesn't trigger the sync mechanism is wrong.

The crucial thing that it does is update the state information held by the syncer so that the subsequent tiddler change event doesn't trigger a PUT to the server.

IMO that's not good. It's possible, that several syncers are active at the same time. see: savetrail.js line 30

Yes, it would be better if the syncadaptor gets a reference to the syncer via options in its constructor.

Parenthetically, I have discovered that the savetrail plugin breaks ordinary saving, because the syncer module is still designed to be global; the synctrail instance stomps on various global state maintained by the saver mechanism. I'll push a fix soon.

danielo515 commented 7 years ago

Yes, it would be better if the syncadaptor gets a reference to the syncer via options in its constructor.

I agree on that , the more scoped the better

tobibeer commented 6 years ago

@danielo515 is there any progress on this / implementation for this. Also, I feel like the title doesn't adequately represent what is being asked / discussed. Does it?

danielo515 commented 6 years ago

hello @tobibeer , that depends on what you understand from my description. Do you think that based on my very first description the title is accurate ? Maybe is the ongoing discussion what invalidates the title ?

Thanks

linonetwo commented 4 years ago

Is there a way to trigger $tw.syncer.syncFromServer() from the server?

I have implemented a filesystem monitor, now the last step is trigger sync immediately after the change.

https://github.com/linonetwo/wiki/blob/80ba0c3f6ba768bbb31ce26e73a1f2d3f895bdd1/Meme-of-LinOnetwo/plugins/linonetwo/watch-fs/FileSystemMonitor.js#L236-L237

linonetwo commented 4 years ago

Oh, I achieve this by adding this file to my plugin:

title: $:/config/SyncPollingInterval
type: text/vnd.tiddlywiki

1000
pmario commented 4 years ago

@linonetwo Have you had a look, what happens in your dev-panel : network tab? You poll a skinny list of tiddlers every second. So if you want to do this on a real and eventually slower internet connection, you'll be screwed. ... IMO this is a ticking bomb, that calls for trouble in the future.

linonetwo commented 4 years ago

@pmario I agree, but seems there is no server push API implemented in the tiddlywiki5, sadly.

I will see if I'm capable to add one.