Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: DbProvisionsStorage putMany doesnt error on cid col conflict #517

Merged
merged 7 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 97 additions & 21 deletions packages/access-api/src/models/provisions.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,31 @@ export function createProvisions(storage = []) {
const hasRowWithSpace = storage.some(({ space }) => space === consumerId)
return hasRowWithSpace
}
/** @type {Provisions['putMany']} */
const putMany = async (...items) => {
storage.push(...items)
/** @type {Provisions['put']} */
const put = async (item) => {
storage.push(item)
}
/** @type {Provisions['count']} */
const count = async () => {
return BigInt(storage.length)
}
return {
count,
putMany,
put,
hasStorageProvider,
}
}

/**
* @typedef ProvsionsRow
* @typedef ProvisionsRow
* @property {string} cid
* @property {string} consumer
* @property {string} provider
* @property {string} sponsor - did of actor who authorized for this provision
*/

/**
* @typedef {import("../types/database").Database<{ provisions: ProvsionsRow }>} ProvisionsDatabase
* @typedef {import("../types/database").Database<{ provisions: ProvisionsRow }>} ProvisionsDatabase
*/

/**
Expand Down Expand Up @@ -68,24 +68,68 @@ export class DbProvisions {
return BigInt(size)
}

/** @type {Provisions['putMany']} */
async putMany(...items) {
if (items.length === 0) {
return
/** @type {Provisions['put']} */
async put(item) {
/** @type {ProvisionsRow} */
const row = {
cid: item.invocation.cid.toString(),
consumer: item.space,
provider: item.provider,
sponsor: item.account,
}
/** @type {ProvsionsRow[]} */
const rows = items.map((item) => {
return {
cid: item.invocation.cid.toString(),
consumer: item.space,
provider: item.provider,
sponsor: item.account,
}
})
await this.#db
/** @type {Array<keyof ProvisionsRow>} */
const rowColumns = ['cid', 'consumer', 'provider', 'sponsor']
const insert = this.#db
.insertInto(this.tableNames.provisions)
.values(rows)
.values(row)
.returning(rowColumns)

let primaryKeyError
try {
await insert.executeTakeFirstOrThrow()
} catch (error) {
const d1Error = extractD1Error(error)
switch (d1Error?.code) {
case 'SQLITE_CONSTRAINT_PRIMARYKEY': {
primaryKeyError = error
break
}
default: {
throw error
}
}
}

if (!primaryKeyError) {
// no error inserting, we're done with put
return
}

// there was already a row with this invocation cid
// as long as the row we tried to insert is same as one already there, no need to error.
// so let's compare the existing row with that cid to the row we tried to insert.
const existing = await this.#db
.selectFrom(this.tableNames.provisions)
.select(rowColumns)
.where('cid', '=', row.cid)
.executeTakeFirstOrThrow()
if (deepEqual(existing, row)) {
// the insert failed, but the existing row is identical to the row that failed to insert.
// so the put is a no-op, and we can consider it a success despite encountering the primaryKeyError
return
}

// this is a sign of something very wrong. throw so error reporters can report on it
// and determine what led to a put() with same invocation cid but new non-cid column values
throw Object.assign(
new Error(
`Provision with cid ${item.invocation.cid} already exists with different field values`
),
{
insertion: row,
existing,
}
)
}

/** @type {Provisions['hasStorageProvider']} */
Expand All @@ -112,3 +156,35 @@ export class DbProvisions {
return rows
}
}

/**
* @param {Record<string,any>} x
* @param {Record<string,any>} y
* @returns {boolean}
*/
function deepEqual(x, y) {
const ok = Object.keys
const tx = typeof x
const ty = typeof y
return x && y && tx === 'object' && tx === ty
? ok(x).length === ok(y).length &&
ok(x).every((key) => deepEqual(x[key], y[key]))
: x === y
}

/**
* @param {unknown} error
*/
function extractD1Error(error) {
const isD1 = /D1_ALL_ERROR/.test(String(error))
if (!isD1) return
const cause =
error && typeof error === 'object' && 'cause' in error && error.cause
const code =
cause &&
typeof cause === 'object' &&
'code' in cause &&
typeof cause.code === 'string' &&
cause.code
return { cause, code }
}
2 changes: 1 addition & 1 deletion packages/access-api/src/service/provider-add.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function createProviderAddHandler(options) {
message: 'Issuer must be a mailto DID',
}
}
await options.provisions.putMany({
await options.provisions.put({
invocation,
space: consumer,
provider,
Expand Down
6 changes: 3 additions & 3 deletions packages/access-api/src/types/provisions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ export interface Provision {
export interface ProvisionsStorage {
hasStorageProvider: (consumer: Ucanto.DID<'key'>) => Promise<boolean>
/**
* write several items into storage
* ensure item is stored
*
* @param items - provisions to store
* @param item - provision to store
*/
putMany: (...items: Provision[]) => Promise<void>
put: (item: Provision) => Promise<void>

/**
* get number of stored items
Expand Down
44 changes: 39 additions & 5 deletions packages/access-api/test/provisions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import { CID } from 'multiformats'
describe('DbProvisions', () => {
it('should persist provisions', async () => {
const { d1 } = await context()
const storage = new DbProvisions(createD1Database(d1))
const count = Math.round(Math.random() * 10)
const db = createD1Database(d1)
const storage = new DbProvisions(db)
const count = 2 + Math.round(Math.random() * 3)
const spaceA = await principal.ed25519.generate()
const provisions = await Promise.all(
const [firstProvision, ...lastProvisions] = await Promise.all(
Array.from({ length: count }).map(async () => {
const issuerKey = await principal.ed25519.generate()
const issuer = issuerKey.withDID('did:mailto:example.com:foo')
Expand All @@ -37,8 +38,8 @@ describe('DbProvisions', () => {
return provision
})
)
await storage.putMany(...provisions)
assert.deepEqual(await storage.count(), provisions.length)
await Promise.all(lastProvisions.map((p) => storage.put(p)))
assert.deepEqual(await storage.count(), lastProvisions.length)

const spaceHasStorageProvider = await storage.hasStorageProvider(
spaceA.did()
Expand All @@ -52,5 +53,38 @@ describe('DbProvisions', () => {
'can parse provision.cid as CID'
)
}

// ensure no error if we try to store same provision twice
// all of lastProvisions are duplicate, but firstProvision is new so that should be added
await storage.put(lastProvisions[0])
await storage.put(firstProvision)
assert.deepEqual(await storage.count(), count)

// but if we try to store the same provision (same `cid`) with different
// fields derived from invocation, it should error
const modifiedFirstProvision = {
...firstProvision,
space: /** @type {const} */ ('did:key:foo'),
account: /** @type {const} */ ('did:mailto:foo'),
// note this type assertion is wrong, but useful to set up the test
provider:
/** @type {import('../src/types/provisions.js').AlphaStorageProvider} */ (
'did:provider:foo'
),
}
const putModifiedFirstProvision = () => storage.put(modifiedFirstProvision)
await assert.rejects(
putModifiedFirstProvision(),
'cannot put with same cid but different derived fields'
)
const provisionForFakeConsumer = await storage.findForConsumer(
modifiedFirstProvision.space
)
assert.deepEqual(provisionForFakeConsumer.length, 0)
assert.deepEqual(
await storage.count(),
count,
'count was not increased by put w/ existing cid'
)
})
})