Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Initial minimal logins API connection to datastore #62

Merged
merged 2 commits into from Jan 18, 2019
Merged

Initial minimal logins API connection to datastore #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f749f2
Initial minimal logins API connection to datastore
lmorchard Jan 10, 2019
3a839cb
Revise event/message flow in datastore to rely on updates from Login …
lmorchard Jan 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 54 additions & 10 deletions src/background/datastore.js
View file
Open in desktop
Expand Up @@ -4,9 +4,10 @@

import UUID from "uuid";

import { broadcast } from "./message-ports";
import telemetry from "./telemetry";

function convertInfo2Item(info) {
export function convertInfo2Item(info) {
let title;
try {
title = (new URL(info.hostname)).host;
Expand Down Expand Up @@ -36,7 +37,7 @@ function convertInfo2Item(info) {
};
return item;
}
function convertItem2Info(item) {
export function convertItem2Info(item) {
// dropped on the floor (for now ...)
// * title
// * tags
Expand Down Expand Up @@ -133,14 +134,16 @@ class DataStore {
// assume formSubmitURL === hostname
info.formSubmitURL = info.hostname;
}
// TODO: call logins API

let added = {
...info,
guid: UUID(),
timesUsed: 0,
timeLastUsed: Date.now(),
timeCreated: Date.now(),
timePasswordChanged: Date.now(),
};
this._all[added.guid] = added;
await browser.experiments.logins.add(added);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other logins-modifying API methods do throw if something goes wrong, so I'd wrap them in try/catch blocks and just log the errors for now. I think we can handle the errors in the relevant issues (looks like #18, #30, #34 might all be relevant).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I can just silently log them, though datastore.js does throw errors for other validation problems and such. Kind of figured it was useful to let these throw too, though it doesn't look like we do anything useful with them yet. (e.g. report an error over a message port, etc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datastore.js does throw errors for other validation problems and such

Ah, ok. Probably fine as is, then.


added = convertInfo2Item(added);
recordMetric("added", added.id);
Expand All @@ -159,12 +162,11 @@ class DataStore {
throw new Error(`no existing item for ${id}`);
}

// TODO: call API
let updated = {
...info,
timePasswordChanged: Date.now(),
};
this._all[updated.guid] = updated;
await browser.experiments.logins.update(updated);

updated = convertInfo2Item(updated);
recordMetric("updated", item.id);
Expand All @@ -174,20 +176,62 @@ class DataStore {
async remove(id) {
const item = await this.get(id);
if (item) {
// TODO: call API
delete this._all[item.id];

await browser.experiments.logins.remove(item.id);
recordMetric("deleted", item.id);
}
return item || null;
}

// TODO: Until issue #21 is resolved, this stuff handles raw info from the
// API rather than UI items.

loadInfo(logins) {
this._all = {};
for (let login of logins) {
this._all[login.guid] = login;
}
}

addInfo(info) {
this._all[info.guid] = info;
const item = convertInfo2Item(info);
broadcast({ type: "added_item", item });
}

updateInfo(info) {
this._all[info.guid] = info;
const item = convertInfo2Item(info);
broadcast({ type: "updated_item", item });
}

removeInfo({ guid }) {
delete this._all[guid];
broadcast({ type: "removed_item", id: guid });
}
}

let bootstrap;
export default async function openDataStore() {
export async function openDataStore() {
if (!bootstrap) {
bootstrap = new DataStore();
}
return bootstrap;
}

export async function closeDataStore() {
bootstrap = null;
}

export async function initializeDataStore() {
const { logins } = browser.experiments;

const entries = await logins.getAll();
const dataStore = await openDataStore();
dataStore.loadInfo(entries);

logins.onAdded.addListener(({ login }) => dataStore.addInfo(login));
logins.onUpdated.addListener(({ login }) => dataStore.updateInfo(login));
logins.onRemoved.addListener(({ login }) => dataStore.removeInfo(login));
}

export default openDataStore;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this pattern better

2 changes: 2 additions & 0 deletions src/background/index.js
View file
Open in desktop
Expand Up @@ -7,9 +7,11 @@
import updateBrowserAction from "./browser-action";
import initializeMessagePorts from "./message-ports";
import { initializeTelemetry } from "./telemetry";
import { initializeDataStore } from "./datastore";

Promise.resolve().then(async () => {
initializeTelemetry();
initializeMessagePorts();
await initializeDataStore();
await updateBrowserAction();
});
5 changes: 1 addition & 4 deletions src/background/message-ports.js
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@ import clipboard from "./clipboard";

const ports = new Set();

function broadcast(message, excludedSender) {
export function broadcast(message, excludedSender) {
for (let p of ports) {
if (!excludedSender || p.sender.contextId !== excludedSender.contextId) {
p.postMessage(message);
Expand Down Expand Up @@ -44,19 +44,16 @@ export default function initializeMessagePorts() {
case "add_item":
return openDataStore().then(async (ds) => {
const item = await ds.add(message.item);
broadcast({type: "added_item", item}, sender);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realized that since I've got the datastore now broadcasting these on the Logins API event handlers, all these messages got doubled up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a lot more sense.

I hope it's clear the code was in an "evolutionary transitional" phase when you picked it up (-:

return {item};
});
case "update_item":
return openDataStore().then(async (ds) => {
const item = await ds.update(message.item);
broadcast({ type: "updated_item", item }, sender);
return { item };
});
case "remove_item":
return openDataStore().then(async (ds) => {
await ds.remove(message.id);
broadcast({type: "removed_item", id: message.id}, sender);
return {};
});

Expand Down
8 changes: 7 additions & 1 deletion src/list/reducers.js
View file
Open in desktop
Expand Up @@ -37,7 +37,13 @@ export function cacheReducer(state = {items: [], currentItem: null}, action) {
case actions.ADD_ITEM_COMPLETED:
return {
...state,
items: [...state.items, makeItemSummary(action.item)],
items: [
// HACK: add dispatched via Logins API events may be a duplicate
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small hacky wrinkle here: Since I'm broadcasting datastore changes based on Logins API events without any source, the UI source that made the changes will get its own change dispatched back to it. This will look like a duplicate. So, I papered over that by making the "add" action idempotent here.

// of an add we just completed in this UI - this filter makes it
// idempotent
...state.items.filter(x => x.id != action.item.id),
makeItemSummary(action.item),
],
...maybeAddCurrentItem(state, action),
};
case actions.UPDATE_ITEM_COMPLETED:
Expand Down