Hello all.
Commenting in-line:
On 10/07/14 07:32, Tim Chien wrote:
> To recap what I have discussed with Vicamo before I advice him to send
> this e-mail:
>
> 1. We want the same API from asyncStorage to hide the complexity to
> handle the transactions. Introducing transactions to asyncStorage
> reduce the value of the library to something simply converting DOM
> requests-based API to chainable/Promise-based API, which is valuable
> but not that valuable IMHO.
Partially true. The get-then-set pattern is common enough to take into
account inside the *AsyncStorage* utility but it is true it introduces
concepts out of the scope for a *localStorage* adapter.
>
> 2. Issued identified around asyncStorage can be handled by proper
> application logic. Application might need to implement a queue itself
> to make sure there isn't any concurrent transactions or block the UI
> when transactions is taking place. I think the UI blocking is
> implemented in many apps and they should be able to use asyncStroage
> as-is, happily.
I think the solution is the queue. It is not very complicated (I
implemented one yesterday [1] for a shim replacing mozSettings) but if
you want to simply cover the get-then-set pattern simply add a method
`update(key, updateFunction)`. It behaves like `read()` but it sets the
key to the value returned by the `updateFunction` parameter.
>
> 3. For the specific issues like Cost Control issue Vicamo is handling
> (thank you!), maybe people will realize wrapping asyncStorage calls in
> a queue is too complex and it might as well as do proper IndexedDB
> transaction management -- in that case it's perfectly fine the said
> app should move away from asyncStorage.
The naive form of the queue (just "enqueue everything") is not
complicated but could lead to slow access due to the queue nature.
Introducing parallelism is possible but it can lead to a complex
implementation based on dependency graphs.
>
> 4. During offline discussion yesterday, I actually realized we could
> implement the said queue inside asyncStorage. I am not sure if it make
> sense though (nor it could solve anything), but that's something we
> can think of.
>
> To recap, I am all for exposing transactions in the wrapping library,
> but please do it somewhere else because asyncStorage is still useful
> for many of the use cases. I am looking forward to see a chainable or
> Promise-based API which will be very useful.
Why to not continue with "thenables" (not real ones) as Julien said but
with a little modification? Instead of make
asyncStorage.get('key').then()... which lead to the problem Vicamo
pointed, you could do:
asyncStorage.get('key')
.thenSet('key', function(value) { return value + 1; })
.thenGet('anotherKey', function _continuation() { ... })
.catch(function (error) { ... });
Now you know when the chain ends.
As a final note. Most of developers simply don't use or try to wrap
IndexedDB cause the API is quite complicated to use.
Hope it helps.
[1]
https://github.com/lodr/shimmozsettings/
>
> djf is OOO so we might want to wait for him to come back before doing anything.
>
>
> On Thu, Jul 10, 2014 at 6:53 AM, Jonas Sicking <
jo...@sicking.cc> wrote:
>> Generally speaking I think it's a bad idea to have to "opt in" to a
>> transaction. Something like:
>>
>> asyncStorage.getItem(..., function() {
>> asyncStorage.setItem(...);
>> });
>>
>> Looks very safe while it's not. Would people have called
>> .transaction() every time that the above pattern appears if it had
>> been available?
>>
>> I think instead we should enforce that all get/set operations happen
>> within a transaction.
>>
>> We wouldn't have to completely roll back the fix for bug 994819.
>> Adding the ability to do .commit().then(function() { closePage() })
>> might help.
>>
>> / Jonas
>>
>> On Wed, Jul 9, 2014 at 7:22 AM, Julien Wajsberg <
jwaj...@mozilla.com> wrote:
>>> Le 09/07/2014 15:41, Vicamo Yang a écrit :
>>>
>>>> On 7/9/14, 8:40 PM, Julien Wajsberg wrote:
>>>>> We need a simple API to do localStorage-like things. So I'd advise to
>>>>> introduce a transaction-based API to asyncStorage that would be backward
>>>>> compatible (read: use the same indexeddb data) with what we have now.
>>>>>
>>>>> The transaction-based API would be used for cases like
>>>>> getItem-then-setItem, but other simple cases wouldn't need it.
>>>>>
>>>>> I could imagine an API like this:
>>>>>
>>>>> asyncStorage.transaction().then(function(worker) {
>>>>> return worker.getItem('xxx').then(function(value) {
>>>>> return worker.setItem('yyy', value+1);
>>>>> });
>>>>> });
>>>> I was thinking almost the same thing, except we might need a
>>>> read-only/read-write flag for that transaction() call because we'll
>>>> never know if we should instantiate a read-write transaction instead of
>>>> a read-only one.
>>>>
>>>> The biggest problem for inventing such transactional API is that it
>>>> implies these callbacks are called in IDBRequest onsuccess event
>>>> handler, not transaction oncomplete handler. Effectively revert of bug
>>>> 936703. But I think this can be resolved by moving such lines into an
>>>> additional oncomplete callback.
>>>>
>>>> Another problem is the success callback of a write operation is no
>>>> longer a strong guarantee that it has been done. Any further error that
>>>> aborts the transaction will result in all previous write operations
>>>> being rolled back. Therefore setItem callbacks should only manipulate
>>>> the records to be written, and one have to move modifications to foreign
>>>> variables into oncomplete.
>>>
>>>
>>> Right. I think we definitely need to use another name than "then" for the
>>> transaction worker. So how about:
>>>
>>> asyncStorage.transaction().work(function(worker) {
>>> // we're inside a transaction, don't assume anything is
>>> // written yet.
>>>
>>> return worker.getItem('xxx').then(function(value) {
>>> return worker.setItem('yyy', value+1);
>>>>> "transaction()" would not return a simple native Promise object. Rather,
>>>>> its "then" method would transparently add a `.then(worker.commit,
>>>>> worker.abort)` at the end of the chain. This way it's easy to complete
>>>>> or abort the transaction, simply returning a resolved or rejected
>>>>> promise.
>>>> But we will never know when is the end of the chain. For example:
>>>>
>>>> asyncStorage.transaction().then(function(worker) {
>>>> yet_another_async_api(function() {
>>>> return worker.getItem('xxx');
>>>> });
>>>> });
>>>>
>>>> Original AsyncStorage API "works" in this case.
>>>
>>> You mean, if yet_another_async_api starts calls transaction() too?
>>>
>>>
________________________________
Este mensaje y sus adjuntos se dirigen exclusivamente a su destinatario, puede contener información privilegiada o confidencial y es para uso exclusivo de la persona o entidad de destino. Si no es usted. el destinatario indicado, queda notificado de que la lectura, utilización, divulgación y/o copia sin autorización puede estar prohibida en virtud de la legislación vigente. Si ha recibido este mensaje por error, le rogamos que nos lo comunique inmediatamente por esta misma vía y proceda a su destrucción.
The information contained in this transmission is privileged and confidential information intended only for the use of the individual or entity named above. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this transmission in error, do not read it. Please immediately reply to the sender that you have received this communication in error and then delete it.
Esta mensagem e seus anexos se dirigem exclusivamente ao seu destinatário, pode conter informação privilegiada ou confidencial e é para uso exclusivo da pessoa ou entidade de destino. Se não é vossa senhoria o destinatário indicado, fica notificado de que a leitura, utilização, divulgação e/ou cópia sem autorização pode estar proibida em virtude da legislação vigente. Se recebeu esta mensagem por erro, rogamos-lhe que nos o comunique imediatamente por esta mesma via e proceda a sua destruição