Closed Bug 946316 Opened 7 years ago Closed 7 years ago

Allow the use of strings as DataStore IDs

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29
blocking-b2g 1.3+
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: benfrancis, Assigned: baku)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

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
Attached 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+
Attached 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.
Attached 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: 7 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.
Depends on: 949271
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.