Porting extensions to 5.0

[Emiliano Heyns]

I'm in the process of making Better BibTeX compatible with what will be 5.0 when it's released, and the first thing I am working on is the database calls. I see these are now handled with yield/coroutine pairs; I can't say I fully understand yet what's going on, but the code sort of looks like it acts mostly sync through the yield call. Would it work to have something like 

Zotero.DB.query = Zotero.Promise.coroutine(function*() {

  return (yield Zotero.DB.queryAsync.apply(Zotero.DB, arguments));

});

to mimic the code flow I could use under 4.0? What are the downsides/pitfalls to such?

Thanks,

Emile

[Aurimas Vinckevicius]

While the code layout looks synchronous, every time you see the yield statement, it's actually an asynchronous call. In fact calling what would be your Zotero.DB.query method would return immediately. It would return a promise that is resolved once the "coroutine" finishes, so you cannot transform it into a synchronous call.

I'd suggest taking a look at JavaScript Promises (maybe here), BlueBird promise library API, which is what Zotero is using, and, in this case in particular, promise-yielding generators with BlueBird's coroutine method

--
You received this message because you are subscribed to the Google Groups "zotero-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to zotero-dev+...@googlegroups.com.
To post to this group, send email to zoter...@googlegroups.com.
Visit this group at http://groups.google.com/group/zotero-dev.
For more options, visit https://groups.google.com/d/optout.

[Emiliano Heyns]

Hi,

I've looked at those, but I can't yet resolve them with how I see the queryAsync calls being used in Zotero master. The way I've seen promises used is in 'then' chains, but I see "yield Zotero.DB.queryAsync" calls in the middle of functions in the Zotero code; if those return immediately and the code after it proceeds immediately, how does that work? Or should the coroutine enclose all yield code to have that chunk within the coroutine mimic sync behavior? I've read the link you point to, but I couldn't make it out from that.

You received this message because you are subscribed to a topic in the Google Groups "zotero-dev" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/zotero-dev/30l_4rNyXC0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to zotero-dev+...@googlegroups.com.

[Dan Stillman]

On 7/4/15 6:07 AM, Emiliano Heyns wrote:
> I've looked at those, but I can't yet resolve them with how I see the
> queryAsync calls being used in Zotero master. The way I've seen
> promises used is in 'then' chains, but I see "yield
> Zotero.DB.queryAsync" calls in the middle of functions in the Zotero
> code; if those return immediately and the code after it proceeds
> immediately, how does that work?

That's where the generators come in. Generators can be suspended, so
unlike normal promise-returning function calls it doesn't continue
immediately after those line. The generator is suspended after the yield
call and code elsewhere continues to run. The wrapping Bluebird code
(from coroutine()) waits for the promise to complete, and when it does
it returns control to the generator.

[Emiliano Heyns]

That is really how I had interpreted it, but then I don't understand why my code above wouldn't work. It's a generator, it yields, so why wouldn't it just stop the caller too?

[Aurimas Vinckevicius]

Since javascript is single threaded, stopping anywhere in a synchronous call  to wait for something asynchronous to execute would not work, since the whole javascript engine would have to halt and of course there would be nothing to execute the asynchronous tasks. So a generator does not halt anything. It just returns a value and remembers where it stopped.

Let's try an example

```
var getValue = Zotero.Promise.coroutine(function*() {

  alert("generator started")

  var val = yield Zotero.DB.valueQueryAsync("SELECT 5");

  alert(val);

  return val*5;
});

var foo = getValue();

alert("foo is " + typeof foo);

foo.then(function(v) { alert(v); });

alert("end");
```

Ok, so the alerts you should get are in the following order:
"foo is object" (since it's a promise)
"end"
"generator started"
"5"
"25"

Following along the flow of the program:
* we create the getValue function, nothing interesting happens
* we call getValue. BlueBird immediately returns a promise that will be fulfilled with the return value of the generator
* we alert that foo is object
* we set up what happens once foo is fulfilled
* we alert "end"
* [some time passes, javascript engine does whatever it wants]
* BlueBird starts processing the generator. It calls it once.
* we alert that generator has started
* generator yields a promise back to BlueBird and it's done (it doesn't wait for BlueBird to return anything. It's stack is cleared). The promise will be fulfilled once SQLite engine returns a value. BlueBird goes to sleep (not explicitly)
* [some time passes and SQLite calls back BlueBird via promise]
* BlueBird calls the generator again, passing back the value from SQLite.
* generator resumes from where it yielded last with the value it was passed back.
* we alert "5"
* generator returns a value
* BlueBird notices that generator is done and it returned a value, so it fulfills foo.
* [some time passes]
* function declared in foo.then is called with foo fulfillment value
* we alert "25"

Hopefully this helps (and hopefully this doesn't come out very mangled)

You received this message because you are subscribed to the Google Groups "zotero-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to zotero-dev+...@googlegroups.com.

[Emiliano Heyns]

On Saturday, July 4, 2015 at 8:06:16 PM UTC+2, Aurimas Vinckevicius wrote:

Since javascript is single threaded, stopping anywhere in a synchronous call  to wait for something asynchronous to execute would not work, since the whole javascript engine would have to halt and of course there would be nothing to execute the asynchronous tasks. So a generator does not halt anything. It just returns a value and remembers where it stopped.

Let's try an example

OK, the example brought it home. Coroutines do not return values but promises, and as such they do not block their caller, but *inside* the coroutine, any direct call to yield "freezes" the coroutine making the inside of a coroutine sort-of-syncish.

[Emiliano Heyns]

Is it possible to do async DB queries on 4.0.27+ in some way? It'd ease my development process if there'd be a way to prepare as much as possible for 5.0 on the 4.0 branch.

[Dan Stillman]

Async DB queries are 5.0 only, but you don't particularly need Zotero
calls to actually be async in order to make your own code async now. Any
function of yours that does a DB query should just be a coroutine, so
that when you switch from query() to queryAsync() or from
beginTransaction()/commitTransaction() to executeTransaction() you just
add yields.

There are a couple complications, though. 4.0 uses Q rather than
Bluebird, so you need to use Q.async() instead of
Zotero.Promise.coroutine(), and the version of Q in 4.0 only supports
old SpiderMonkey generators instead of ES6 generators. SM generators
don't have an asterisk, and they also can't return a value directly and
need to use Q.return(foo) instead.

[Emiliano Heyns]

Would it be problematic to load Bluebird as part of BBT? This would put Promise and P in the global namespace. 

[Dan Stillman]

On 7/6/15 5:00 AM, Emiliano Heyns wrote:
> Would it be problematic to load Bluebird as part of BBT? This would
> put Promise and P in the global namespace.

You certainly shouldn't overwrite the global Promise, but you can put
Bluebird into your own namespace and do a find-and-replace later when
it's available as Zotero.Promise.

[Emiliano Heyns]

Zotero does that with "Components.utils.import("resource://zotero/bluebird.js", this);", right? I could just download the bluebird.js from Zotero master and do the same within my own scope, then? 

[Emiliano Heyns]

On Monday, July 6, 2015 at 7:20:05 AM UTC+2, Dan Stillman wrote:

On 7/5/15 10:09 AM, Emiliano Heyns wrote:
> Is it possible to do async DB queries on 4.0.27+ in some way? It'd
> ease my development process if there'd be a way to prepare as much as
> possible for 5.0 on the 4.0 branch.

Async DB queries are 5.0 only, but you don't particularly need Zotero
calls to actually be async in order to make your own code async now. Any
function of yours that does a DB query should just be a coroutine, so
that when you switch from query() to queryAsync() or from
beginTransaction()/commitTransaction() to executeTransaction() you just
add yields.

OK, I've managed to load bluebird into my own namespace, and I've replaced my Q calls with Bluebird calls. So far so good.

Any general tips from your experience in promisifying Zotero? 

[Emiliano Heyns]

Bluebird flags extensions for manual review from the extension signing validator :(

[Emiliano Heyns]

I'm looking at Zotero.DataObjects.prototype.getAsync; the documentation in the comment above it suggests that it returns a dataobject(s), but looking at the code it looks more like it returns a promise for such. Is that correct? And if so, that means that any code calling it should call it using yield (which is indeed what I'm seeing in the rest of the code).

Everything that intends to call a coroutine must itself (even if only pragmatically) also be a coroutine, right?

[Dan Stillman]

On 7/7/15 2:31 PM, Emiliano Heyns wrote:
> I'm looking at Zotero.DataObjects.prototype.getAsync; the
> documentation in the comment above it suggests that it returns a
> dataobject(s), but looking at the code it looks more like it returns a
> promise for such. Is that correct? And if so, that means that any code
> calling it should call it using yield (which is indeed what I'm seeing
> in the rest of the code).

Yes, sorry. Fixed.

> Everything that intends to call a coroutine must itself (even if only
> pragmatically) also be a coroutine, right?

Well, it doesn't have to be coroutine itself, but it needs to return a
promise.

It should be a coroutine if it needs to operate on the resolved value of
a promise:


/**
* @return {Promise}
*/
var getItemTitle = Zotero.Promise.coroutine(function* getItemTitle(itemID) {
var obj = yield Zotero.Items.getAsync(itemID);
return obj.getField('title');
});

If the promise will be the calling function's return value as well,
though, then it can just return the promise directly:

/**
* @return {Promise}
*/
var getItem = function getItem(itemID) {
return Zotero.Items.getAsync(itemID);
}

Note that if there's any other code in the calling function, if it's not
going to be a coroutine, it should at least use Bluebird's method()
function [1] to ensure that a promise is returned:

/**
* @return {Promise}
*/
var getItem = Zotero.Promise.method(function getItem(itemID) {
if (!itemID) {
throw new Error("itemID wasn't provided");
}
return Zotero.Items.getAsync(itemID);
})

Without method(), the throw would happen inline and become an uncaught
exception rather than rejecting the promise such that it can be caught
with catch().

The upshot of all this is that async code is pretty viral, which is why
we had to rewrite essentially the entire code base. See my explanation
in #518 [2], though, of one way we've tried to reduce inline promises
and the spread of asnyc code — by preloading data and having getters
simply throw an error if data isn't yet loaded.


[1]
https://github.com/petkaantonov/bluebird/blob/master/API.md#promisemethodfunction-fn---function
[2] https://github.com/zotero/zotero/issues/518

[Emiliano Heyns]

On Tuesday, July 7, 2015 at 8:54:15 PM UTC+2, Dan Stillman wrote:

<lots of clarifying examples>

OK, super. That makes things a lot more clear. 

The upshot of all this is that async code is pretty viral, which is why
we had to rewrite essentially the entire code base. See my explanation
in #518 [2], though, of one way we've tried to reduce inline promises
and the spread of asnyc code — by preloading data and having getters
simply throw an error if data isn't yet loaded. 

[1]
https://github.com/petkaantonov/bluebird/blob/master/API.md#promisemethodfunction-fn---function
[2] https://github.com/zotero/zotero/issues/518

 I see. But what is the pragmatic difference between

var item = yield Zotero.Items.get(123);
var title = yield item.getField('title');
var date = yield item.getField('date');

and

var item = yield Zotero.Items.get(123);
try {
    var title = item.getField('title'); // item data not yet loaded; throws an error
}
catch (e) {}
yield item.loadItemData(); // this is a no-op if data is already loaded
var title = item.getField('title');
var date = item.getField('date');

In practice, both snippets will have to be part of an async function; so you lose one yield, but you gain at least one extra line of code to do so (excluding for now the try-catch block -- I don't know whether its intent is to demonstrate getField throwing an error, or that the try-catch has a desirable side-effect that I'm missing).

How much time do I have for the 5.0 transition? I don't really have the time to maintain two separate versions, so ideally I'd want to release a 5.0-compatible BBT within hours of Zotero releasing 5.0.

[Dan Stillman]

On 7/7/15 3:53 PM, Emiliano Heyns wrote:

 I see. But what is the pragmatic difference between

var item = yield Zotero.Items.get(123);
var title = yield item.getField('title');
var date = yield item.getField('date');

and

var item = yield Zotero.Items.get(123);
try {
    var title = item.getField('title'); // item data not yet loaded; throws an error
}
catch (e) {}
yield item.loadItemData(); // this is a no-op if data is already loaded
var title = item.getField('title');
var date = item.getField('date');

In practice, both snippets will have to be part of an async function; so you lose one yield, but you gain at least one extra line of code to do so

That's just a demonstration of the pattern. In real code, 1) the getField() calls might be in another (synchronous) function, with that function requiring that the item it is passed had its data loaded, and 2) there might be a dozen getField() calls, not just two, meaning that you avoid 10 yields, which translates to dozens of avoided functions calls within Bluebird. There are also lots of places where async functions just aren't possible for either API or performance reasons (or both, as in the case of sorting the middle pane), so having getField() be asynchronous isn't an option.

(excluding for now the try-catch block -- I don't know whether its intent is to demonstrate getField throwing an error, or that the try-catch has a desirable side-effect that I'm missing).

The try/catch is just demonstrating the initial call throwing an error, yes.

How much time do I have for the 5.0 transition? I don't really have the time to maintain two separate versions, so ideally I'd want to release a 5.0-compatible BBT within hours of Zotero releasing 5.0.

5.0 will be in beta for a while, and we'll give plenty of warning before a final release, if that's what you mean.

[Emiliano Heyns]

I'll just wait for the announcement then. I tried a rewrite that shimmed in the 5.0 compatible behavior so I could build a single source, but thta quickly turned out to be unfeasible.

I realize this very likely is way too late at this point, but an alternate route could perhaps have been to work with an in-memory DB like LokiJS; it seems plenty fast enough to do everything synchronously, and the bulk-load at the start of Zotero has so far been plenty fast in my tests. It has triggers, transactions, and a changes feed which I use to flush the changes out to disk on idle and every few minutes. It has the downside of course that this chunks transactions (as transactions are really only durable after they're flushed), but this is fast, simple, and as an added benefit, serialization for the translators would be as easy as:

function clone(obj) {
  var cloned = new Zotero.Translator.Item(); // or somesuch
  Object.assign(cloned, JSON.parse(JSON.stringify(obj)));
  return cloned;
};

which is a fair bit faster than the current serializer while still handing the translator a neutered object.

Just putting that out there.