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

Partial Key Queries #33

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions src/main/codegen/generateGSIs.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Fields, Table } from "./types"
import {buildKey, replaceKeyDollar} from "./util"
Copy link
Contributor

Choose a reason for hiding this comment

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

These imports seem not be used?


export function generateGSIs(tables: Table[]): string {
return tables.map(generateGSIsForTable).join("\n\n")
Expand Down
6 changes: 4 additions & 2 deletions src/main/codegen/generateModels.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Model } from "./types"
import { buildKey } from "./util"

export function generateModels(models: Model[]) {
return models.map(generateModel)
Expand All @@ -10,7 +11,8 @@ function generateModel(model: Model): string {

return `export const ${model.name}Model = ${model.tableName}Table
.model<${model.name}>(ModelType.${model.name})
.partitionKey("${pkPrefix}", "${pk.replace("$", "")}")
.sortKey("${skPrefix}", "${sk.replace("$", "")}")
.partitionKey("${pkPrefix}", ${buildKey(pk)})
.sortKey("${skPrefix}", ${buildKey(sk)})
`
}

14 changes: 10 additions & 4 deletions src/main/codegen/generateTables.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Table } from "./types"
import { buildKeyArray } from "./util"

export function generateTables(tables: Table[]): string {
const tableCode = tables.map((table) => {
Expand All @@ -24,13 +25,18 @@ function generateEncryptionBlacklist(table: Table): string {
.forEach((model) => {
const [, pk] = model.keys.partitionKey
const [, sk] = model.keys.sortKey
encryptionBlacklistSet.add(pk.replace("$", ""))
encryptionBlacklistSet.add(sk.replace("$", ""))

const pks = buildKeyArray(model.keys.partitionKey[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you can just use the pk and sk variables above as they're already bound to model.keys.partitionKey[1] and model.keys.sortKey[1]

const sks = buildKeyArray(model.keys.sortKey[1])
pks.forEach((key: string) => encryptionBlacklistSet.add(key))
sks.forEach((key: string) => encryptionBlacklistSet.add(key))
})

table.gsis.forEach(({ name, partitionKey, sortKey }) => {
encryptionBlacklistSet.add(partitionKey.replace("$", ""))
encryptionBlacklistSet.add(sortKey.replace("$", ""))
const pk = buildKeyArray(partitionKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: would just inline to: buildKeyArray(partitionKey).forEach(key => encryptionBlacklist.add(key))

const sk = buildKeyArray(sortKey)
pk.forEach((key: string) => encryptionBlacklistSet.add(key))
sk.forEach((key: string) => encryptionBlacklistSet.add(key))
})

return JSON.stringify(Array.from(encryptionBlacklistSet))
Expand Down
24 changes: 24 additions & 0 deletions src/main/codegen/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,27 @@ export function groupBy<T extends { [key: string]: any }, U extends keyof T>(

return Object.entries(groupedItems)
}

export function replaceKeyDollar(key: string | string[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm...we're threading this string | string[] type through multiple places in the code. I'd prefer we only do this check once at the "top level" and just transform string types into string[]. That way, everything that comes after can just assume the type is string[]

Copy link
Contributor

Choose a reason for hiding this comment

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

Style: all function types outside of tests should explicitly specify their return types. This avoids accidentally getting the wrong type inferred.

For inline lambdas, e.g. const f = () => { ... }, it's fine to leave the return type off since they're usually very simple .

if (Array.isArray(key)) {
return key.map((k) => k.replace("$", ""))
}
return key.replace("$", "")
}

export function buildKey(key: string | string[]) {
const cleanKey = replaceKeyDollar(key);
if (Array.isArray(cleanKey)) {
const keys = cleanKey.map((k) => `"${k}"`).join(', ')
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear on why we put a dollar sign on each key part, and then just remove the first one below.

return `[${keys}]`
}
return `"${cleanKey.replace("$", "")}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to do what your replaceKeyDollar function does above. I think replaceKeyDollar can go away if we transform string types to arrays at the top per my previous comment.

}

export function buildKeyArray(key: string | string[]) {
const keyOrKeys = replaceKeyDollar(key)
if (Array.isArray(keyOrKeys)) {
return keyOrKeys
}
return [keyOrKeys]
}
2 changes: 1 addition & 1 deletion src/main/dynamo/Beyonce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class Beyonce {
})
.promise()

if (unprocessedKeys !== undefined) {
if (unprocessedKeys?.entries !== undefined) {
console.error("Some keys didn't process", unprocessedKeys)
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/dynamo/GSI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class GSI<T extends Model<any, any, any>> {
this.modelTags = models.map((_) => _.modelTag)
}

key(partitionKey: string): PartitionKey<ExtractFields<T>> {
key(partitionKey: string | string[]): PartitionKey<ExtractFields<T>> {
return new PartitionKey(this.partitionKeyName, partitionKey, this.modelTags)
}
}
Expand Down
98 changes: 82 additions & 16 deletions src/main/dynamo/Model.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
import { PartitionAndSortKey, PartitionKeyAndSortKeyPrefix } from "./keys"
import { Table } from "./Table"
import { TaggedModel } from "./types"
import { TaggedModel, AtLeastOne } from "./types"
import { key } from "aws-sdk/clients/signer"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think you intended to import this?


export class Model<
T extends TaggedModel,
U extends keyof T,
V extends keyof T
> {
> {
constructor(
private table: Table,
private partitionKeyPrefix: string,
private partitionKeyField: U,
private partitionKeyField: U | U[],
private sortKeyPrefix: string,
private sortKeyField: V,
private sortKeyField: V | V[],
readonly modelTag: string
) {}
) { }

key(
params: { [X in U]: string } & { [Y in V]: string }
Expand All @@ -27,10 +28,36 @@ export class Model<
} = this
return new PartitionAndSortKey(
this.table.partitionKeyName,
this.buildKey(partitionKeyPrefix, params[partitionKeyField]),
this.buildKey(
partitionKeyPrefix,
this._buildPartitionKey(params, partitionKeyField)),

this.table.sortKeyName,
this.buildKey(sortKeyPrefix, params[sortKeyField]),
this.buildKey(
sortKeyPrefix,
this._buildSortKey(params, sortKeyField)),
this.modelTag
)
}

partialKey(
params: AtLeastOne<{ [X in U]: string }> & AtLeastOne<{ [Y in V]: string }>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where I think I could possibly use a generated type of only valid combinations of keys.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... I think the types we'd want to express here would be a union like:

{ country: string } | { country: string, state: string } | { country: string, state: string, city: string }

This is tricky to do as is since:

  1. We have a generic model class
  2. All it knows inside here is that partitionKeys / sortKeys are a union type.
  3. Afaik there's not a way to extract keys of a union type in-order and re-combine them to what we'd want (maybe there is, I just don't know about it).

This would be easier to express if we explicitly codegenned each model vs having a generic model class as we do today. Since then, we could just codegen the right types directly

): PartitionKeyAndSortKeyPrefix<T> {
const {
partitionKeyPrefix,
sortKeyPrefix,
partitionKeyField,
sortKeyField,
} = this
return new PartitionKeyAndSortKeyPrefix(
this.table.partitionKeyName,
this.buildKey(
partitionKeyPrefix,
this._buildPartitionKey(params, partitionKeyField)),
this.table.sortKeyName,
this.buildKey(
sortKeyPrefix,
this._buildSortKey(params, sortKeyField)),
this.modelTag
)
}
Expand All @@ -39,7 +66,9 @@ export class Model<
const { partitionKeyPrefix, partitionKeyField } = this
return new PartitionKeyAndSortKeyPrefix(
this.table.partitionKeyName,
this.buildKey(partitionKeyPrefix, params[partitionKeyField]),
this.buildKey(
partitionKeyPrefix,
this._buildPartitionKey(params, partitionKeyField)),
this.table.sortKeyName,
this.sortKeyPrefix,
this.modelTag
Expand All @@ -59,9 +88,10 @@ export class Model<

const pk = this.buildKey(
partitionKeyPrefix,
fieldsWithTag[partitionKeyField]
)
const sk = this.buildKey(sortKeyPrefix, fieldsWithTag[sortKeyField])
this._buildPartitionKey(fieldsWithTag, partitionKeyField))
const sk = this.buildKey(
sortKeyPrefix,
this._buildSortKey(fieldsWithTag, sortKeyField))

return {
...fieldsWithTag,
Expand All @@ -70,13 +100,49 @@ export class Model<
}
}

private buildKey(prefix: string, key: string): string {
/* TODO: Had issue mapping unioned fields of T & V */
private _buildPartitionKey(model: AtLeastOne<{ [X in U]: string }>, fields: U | U[]): string | string[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

The _ prefix in function names is usually a result of host languages not supporting a private keyword / feature -- e.g. Python. Since TypeScript already has a private keyword, there's no need to prefix private methods with _.

if (Array.isArray(fields)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is another place we're we'd benefit from normalizing non-array keys into arrays. For example, we could transform keys to arrays internally in the model's constructor.

const pkey: string[] = [];
for (const field of fields) {
const value = model[field]
if (value) {
pkey.push(value)
} else {
break
}
};
return pkey
}
return model[fields as U]
}

private _buildSortKey(model: { [Y in V]: string }, fields: V | V[]): string | string[] {
if (Array.isArray(fields)) {
const skey: string[] = [];
for (const field of fields) {
const value = model[field]
if (value) {
skey.push(value)
} else {
break
}
};
return skey
}
return model[fields as V]
}

private buildKey(prefix: string, key: string | string[]): string {
if (Array.isArray(key)) {
return `${prefix}-${key.join('-')}`
}
return `${prefix}-${key}`
}
}

export class PartitionKeyBuilder<T extends TaggedModel> {
constructor(private table: Table, private modelTag: string) {}
constructor(private table: Table, private modelTag: string) { }
partitionKey<U extends keyof T>(
prefix: string,
partitionKeyField: U
Expand All @@ -94,11 +160,11 @@ export class SortKeyBuilder<T extends TaggedModel, U extends keyof T> {
constructor(
private table: Table,
private partitionKeyPrefix: string,
private partitionKeyField: U,
private partitionKeyField: U | U[],
private modelTag: string
) {}
) { }

sortKey<V extends keyof T>(prefix: string, sortKeyField: V): Model<T, U, V> {
sortKey<V extends keyof T>(prefix: string, sortKeyField: V | V[]): Model<T, U, V> {
return new Model(
this.table,
this.partitionKeyPrefix,
Expand Down
2 changes: 1 addition & 1 deletion src/main/dynamo/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type RawQueryResults<T extends TaggedModel> = {
/** Builds and executes parameters for a DynamoDB Query operation */
export class QueryBuilder<T extends TaggedModel> extends QueryExpressionBuilder<
T
> {
> {
private scanIndexForward: boolean = true
private modelTags: string[] = getModelTags(this.config)

Expand Down
16 changes: 8 additions & 8 deletions src/main/dynamo/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,31 @@ import { TaggedModel } from "./types"
export class PartitionKey<T extends TaggedModel> {
constructor(
readonly partitionKeyName: string,
readonly partitionKey: string,
readonly partitionKey: string | string[],
readonly modelTags: T["model"][]
) {}
) { }
}

export class PartitionKeyAndSortKeyPrefix<T extends TaggedModel> {
constructor(
readonly partitionKeyName: string,
readonly partitionKey: string,
readonly partitionKey: string | string[],
readonly sortKeyName: string,
readonly sortKeyPrefix: string,
readonly modelTag: T["model"]
) {}
) { }
}

export class PartitionAndSortKey<T extends TaggedModel> {
constructor(
readonly partitionKeyName: string,
readonly partitionKey: string,
readonly partitionKey: string | string[],
readonly sortKeyName: string,
readonly sortKey: string,
readonly sortKey: string | string[],
readonly modelTag: T["model"]
) {}
) { }

// Required to avoid TypeScript's structural typing from collapsing
// PartitionAndSortKey<T> and PartitionAndSortKey<U> into the same object.
private dummy(item: T) {}
private dummy(item: T) { }
}
2 changes: 2 additions & 0 deletions src/main/dynamo/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export type ExtractKeyType<T> = T extends PartitionAndSortKey<infer U>
export type GroupedModels<T extends TaggedModel> = {
[K in T["model"]]: T extends { model: K } ? T[] : never
}

export type AtLeastOne<T, U = { [K in keyof T]: Pick<T, K> }> = Partial<T> & U[keyof U]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... This ends up generating a pretty crazy type that's hard to read. And it appears to be basically equivalent to Partial<{ [X in U]: string }> -- i.e. you can specify one or more of the key parts.

46 changes: 46 additions & 0 deletions src/test/codegen/generateCode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,49 @@ Tables:

expect(lines).toContainEqual(`name: BestNameEvah`)
})

it("should generate a complex key model", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test, thanks!

const result = generateCode(`
Tables:
ComplexLibrary:
Partitions:
ComplexAuthors:
ComplexAuthor:
partitionKey: [Author, $id]
Copy link
Contributor

Choose a reason for hiding this comment

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

Would like to add a test for partition keys as well, since it looks like this PR supports complex keys for both.

sortKey: [Author, [$id, $name]]
id: string
name: string
`)

expect(result).toEqual(`import { Table } from "@ginger.io/beyonce"

export const ComplexLibraryTable = new Table({
name: "ComplexLibrary",
partitionKeyName: "pk",
sortKeyName: "sk",
encryptionBlacklist: ["id", "name"]
})

export enum ModelType {
ComplexAuthor = "ComplexAuthor"
}

export interface ComplexAuthor {
model: ModelType.ComplexAuthor
id: string
name: string
}

export const ComplexAuthorModel = ComplexLibraryTable.model<ComplexAuthor>(
ModelType.ComplexAuthor
)
.partitionKey("Author", "id")
.sortKey("Author", ["id", "name"])

export type Model = ComplexAuthor

export const ComplexAuthorsPartition = ComplexLibraryTable.partition([
ComplexAuthorModel
])
`)
})
Loading