Allow the use of strings as DataStore IDs

RESOLVED FIXED in Firefox 28

Status

()

defect
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: benfrancis, Assigned: baku)

Tracking

unspecified
mozilla29
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed, b2g-v1.4 fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 1 obsolete attachment)

I have some use cases for DataStore where it would be really useful to be able to use strings as IDs instead of automatically incrementing integers.

Others have also highlighted other use cases on the dev-webapi mailing list.

I understand we'd like to keep DataStore lightweight, but if there aren't serious performance penalties then this would make web developers lives a lot easier :)
For example, I have the need in the system app of Firefox OS to expose a list of records via a DataStore keyed by URL. I need one and only one entry per URL and need to be able to update an entry for a URL rather than create another one if it already exists.

Currently to do this with DataStore I would have to create an IndexedDB database purely dedicated to the function of mapping URIs to DataStore IDs and keep them in sync.
(In reply to Ben Francis [:benfrancis] from comment #1)
> For example, I have the need in the system app of Firefox OS to expose a
> list of records via a DataStore keyed by URL.

That's bug 946778
(In reply to Ben Francis [:benfrancis] from comment #1)
> For example, I have the need in the system app of Firefox OS to expose a
> list of records via a DataStore keyed by URL. I need one and only one entry
> per URL and need to be able to update an entry for a URL rather than create
> another one if it already exists.
> 
> Currently to do this with DataStore I would have to create an IndexedDB
> database purely dedicated to the function of mapping URIs to DataStore IDs
> and keep them in sync.

I had the same problem with FB data but I opted for a different approach which is saving an special record in the Datastore as the index that maps String Ids to DS Ids. 

Nonetheless, +100 to fix this bug
That's a great suggestion thanks Jose, I may need to use that as a temporary solution until this feature lands :)
It's a busy week... I'm almost there with this bug. for the end of this week it should land!
Thanks Andrea, I will have to use the workaround for the time being then. I'm on PTO after this week.
Or perhaps not.

Bug 948945
Posted patch keynew.patch (obsolete) — Splinter Review
Attachment #8347263 - Flags: review?(ehsan)
Comment on attachment 8347263 [details] [diff] [review]
keynew.patch

Review of attachment 8347263 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

::: dom/datastore/DataStore.jsm
@@ +48,5 @@
>    return aWindow.Promise.reject(
>      new aWindow.DOMError("ReadOnlyError", "DataStore in readonly mode"));
>  }
>  
> +function parseId(aId) {

Please rename this function to validateId.
Attachment #8347263 - Flags: review?(ehsan) → review+
Posted patch keynew.patchSplinter Review
Attachment #8347263 - Attachment is obsolete: true
The B2G failure was perma, FWIW.
Blocks: 950881
needinfo myself to keep track of.
Flags: needinfo?(gene.lian)
/me working on it. I think I fix this for today.
Posted patch gaia.patchSplinter Review
This patch is needed in order to have b2g emulator tests green on try.
On b2g emulator, any mochi test runs in a mochitest app. This app must have the right permissions to let the tests to use DataStore.
Attachment #8349461 - Flags: review?(etienne)
Comment on attachment 8349461 [details] [diff] [review]
gaia.patch

Review of attachment 8349461 [details] [diff] [review]:
-----------------------------------------------------------------

Patch here: https://github.com/mozilla-b2g/gaia/pull/14810
Attachment #8349461 - Flags: review?(etienne) → review+
https://hg.mozilla.org/mozilla-central/rev/d44b65359da5
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Hi,

This bug blocks a blocker for v1.3 (Bug 950881), please see comment https://bugzilla.mozilla.org/show_bug.cgi?id=950881#c1. Because of this, nominating this bug for v1.3. Thanks!
blocking-b2g: --- → 1.3?
1.3+ as it blocks a blocker
blocking-b2g: 1.3? → 1.3+
https://hg.mozilla.org/releases/mozilla-aurora/rev/3fe07c50c854
v1.3: b1e86026b3d3b710cd1486c25daca85cffb8a331
Flags: needinfo?(gene.lian)
This patch depends on bug 949264, doesn't it?
Depends on: 949264
(In reply to Boris Zbarsky [:bz] from comment #24)
> This patch depends on bug 949264, doesn't it?

Yup, would be really nice if those deps were noted *before* requesting blocking status.

Gaia commit also reverted:
v1.3: 863205724ebdd0301fe677641a2bb736104e8381
What about now? Should I do something for this patch in order to land it again?
Flags: needinfo?(amarchesini)
Hi guys, shouldn't we set the status of the bug back to REOPENED? I didn't want to do it myself in case I may be missing something :-)
Or does this means that it should be solve once bug 949264 is uplifted to 1.3 (it seems bug 946316 was also backed out because of bug 949264)? Thanks!
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #26)
> (In reply to Boris Zbarsky [:bz] from comment #24)
> > This patch depends on bug 949264, doesn't it?
> 
> Yup, would be really nice if those deps were noted *before* requesting
> blocking status.

It helps if the patch author or other people familiar with the bug are the ones who request blocking status.
(In reply to gtorodelvalle from comment #28)
> Hi guys, shouldn't we set the status of the bug back to REOPENED? I didn't
> want to do it myself in case I may be missing something :-)

Bug resolution follows the status with mozilla-central, where this wasn't backed out. The status flags track branch uplifts.
With bug 949264 and bug 949271 included, this builds and passes tests on Try.
v1.3: 79b40c526fd8ebd332b5e53024ecb59baa496972
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.