Closed Bug 1319884 Opened 3 years ago Closed 3 years ago

Address FirefoxAdapter feedback from kinto.js#589

Categories

(WebExtensions :: Untriaged, defect)

defect
Not set

Tracking

(firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: glasserc, Assigned: glasserc)

Details

Attachments

(2 files)

:n1k0 in https://github.com/Kinto/kinto.js/pull/589 commented on a few changes I made to the FirefoxAdapter storage adapter for Kinto.js. (That PR was the one that allowed FirefoxAdapter to use an externally-managed Sqlite connection rather than creating its own; this was necessary to address feedback from bug 1253740.) :n1k0 observed that this caused two issues for FirefoxAdapter:

- It now exports _init, which is simultaneously an implementation detail AND something that callers are supposed to know that they need to use.

- FirefoxAdapter is now responsible for functioning whether or not a Sqlite connection is present. This adds a certain amount of complexity to the module. :n1k0 suggested I pick one or the other and convert existing uses.

This bug represents the work that fixes these issues.
In doing this work, I looked at the other main client of kinto.js in Gecko is blocklist-clients.js, and it mainly calls collection.db.open() and collection.db.close(). In this approach, the Sqlite connection is internal to the Collection object. This does have a certain advantage in that 1. it's impossible to "forget" to _init the Sqlite connection (to create tables and so forth), 2. it makes the "common" case, where you don't care about the lifetime of the Sqlite connection, a little easier to manage.

An ideal API would cover all use cases to exist. So far, I haven't been able to find such an API.
Assignee: nobody → eglassercamp
Comment on attachment 8813810 [details]
Bug 1319884 - Address FirefoxAdapter feedback from kinto.js#589,

https://reviewboard.mozilla.org/r/95170/#review96770
Attachment #8813810 - Flags: review?(mgoodwin) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/836efdd6c83e
Address FirefoxAdapter feedback from kinto.js#589, r=mgoodwin
https://hg.mozilla.org/integration/autoland/rev/2648cedc9591
Leftover cleanups, r=MattN
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/836efdd6c83e
https://hg.mozilla.org/mozilla-central/rev/2648cedc9591
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.