-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: auto-increment for entity id #244
base: master
Are you sure you want to change the base?
Changes from 3 commits
2735914
99b73f1
05305c2
23698a4
ab03db8
359e10a
d49c317
2e92f08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,4 +61,4 @@ | |
"dist/**/*", | ||
"src/**/*" | ||
] | ||
} | ||
} | ||
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -30,7 +30,7 @@ import { | |||||||||||||||||||||||
singleEntityQueryName, | ||||||||||||||||||||||||
getNonNullType | ||||||||||||||||||||||||
} from '../utils/graphql'; | ||||||||||||||||||||||||
import { CheckpointConfig } from '../types'; | ||||||||||||||||||||||||
import { autoIncrementFieldPattern, CheckpointConfig, tableNamePattern } from '../types'; | ||||||||||||||||||||||||
import { querySingle, queryMulti, ResolverContext, getNestedResolver } from './resolvers'; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
|
@@ -193,10 +193,24 @@ export class GqlEntityController { | |||||||||||||||||||||||
* ); | ||||||||||||||||||||||||
* ``` | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
will execute the following SQL when declaring id as autoincrement: | ||||||||||||||||||||||||
* ```sql | ||||||||||||||||||||||||
* DROP TABLE IF EXISTS votes; | ||||||||||||||||||||||||
* CREATE TABLE votes ( | ||||||||||||||||||||||||
* id integer not null primary key autoincrement, | ||||||||||||||||||||||||
* name VARCHAR(128), | ||||||||||||||||||||||||
* ); | ||||||||||||||||||||||||
* ``` | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
public async createEntityStores(knex: Knex): Promise<{ builder: Knex.SchemaBuilder }> { | ||||||||||||||||||||||||
public async createEntityStores( | ||||||||||||||||||||||||
knex: Knex, | ||||||||||||||||||||||||
schema: string | ||||||||||||||||||||||||
): Promise<{ builder: Knex.SchemaBuilder }> { | ||||||||||||||||||||||||
let builder = knex.schema; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
const autoIncrementFieldsMap = this.extractAutoIncrementFields(schema); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (this.schemaObjects.length === 0) { | ||||||||||||||||||||||||
return { builder }; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
@@ -205,34 +219,79 @@ export class GqlEntityController { | |||||||||||||||||||||||
const tableName = pluralize(type.name.toLowerCase()); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
builder = builder.dropTableIfExists(tableName).createTable(tableName, t => { | ||||||||||||||||||||||||
t.primary(['id']); | ||||||||||||||||||||||||
//if there is no autoIncrement fields on current table, mark the id as primary | ||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||
autoIncrementFieldsMap.size == 0 || | ||||||||||||||||||||||||
autoIncrementFieldsMap.get(tableName)?.length === 0 | ||||||||||||||||||||||||
) { | ||||||||||||||||||||||||
t.primary(['id']); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
this.getTypeFields(type).forEach(field => { | ||||||||||||||||||||||||
const fieldType = field.type instanceof GraphQLNonNull ? field.type.ofType : field.type; | ||||||||||||||||||||||||
if (isListType(fieldType) && fieldType.ofType instanceof GraphQLObjectType) return; | ||||||||||||||||||||||||
const sqlType = this.getSqlType(field.type); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
let column = | ||||||||||||||||||||||||
'options' in sqlType | ||||||||||||||||||||||||
? t[sqlType.name](field.name, ...sqlType.options) | ||||||||||||||||||||||||
: t[sqlType.name](field.name); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (field.type instanceof GraphQLNonNull) { | ||||||||||||||||||||||||
column = column.notNullable(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (!['text', 'json'].includes(sqlType.name)) { | ||||||||||||||||||||||||
column.index(); | ||||||||||||||||||||||||
//Check if field is declared as autoincrement | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||
if (autoIncrementFieldsMap.get(tableName)?.includes(field.name)) { | ||||||||||||||||||||||||
t.increments(field.name, { primaryKey: true }); | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
const sqlType = this.getSqlType(field.type); | ||||||||||||||||||||||||
let column = | ||||||||||||||||||||||||
'options' in sqlType | ||||||||||||||||||||||||
? t[sqlType.name](field.name, ...sqlType.options) | ||||||||||||||||||||||||
: t[sqlType.name](field.name); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (field.type instanceof GraphQLNonNull) { | ||||||||||||||||||||||||
column = column.notNullable(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (!['text', 'json'].includes(sqlType.name)) { | ||||||||||||||||||||||||
column.index(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
await builder; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return { builder }; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Parse schema to extract table and fields witrh annotation autoIncrement | ||||||||||||||||||||||||
* @param schema | ||||||||||||||||||||||||
* @returns A map where key is table name and values a list of fields with annotation autoIncrement | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
private extractAutoIncrementFields(schema: string) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be handled with regexes. Directive can be handled natively with GraphQL. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, can you guide me to some example of what you want. From my understanding autoIncrement directive is not native in GraphQL so i need to implement the logic behind this new directive. Thank you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we extended GraphQL with our custom directive here, we can then access that directive directly from our GraphQL tree. This way you can look at the field (id field in this example, and check if it has checkpoint/src/graphql/resolvers.ts Lines 182 to 186 in e6dd8b3
That means we don't have to bother with writing our own dummy parser for GraphQL to extract autoIncrementFields, and just check for checkpoint/src/graphql/controller.ts Lines 223 to 228 in 23698a4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK thx for the hint. I will check it and modify accordingly. |
||||||||||||||||||||||||
const autoIncrementFieldsMap = new Map<string, string[]>(); | ||||||||||||||||||||||||
let currentTable = ''; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
schema.split('\n').forEach(line => { | ||||||||||||||||||||||||
const matchTable = line.match(tableNamePattern); | ||||||||||||||||||||||||
//check for current table name | ||||||||||||||||||||||||
if (matchTable && matchTable[1]) { | ||||||||||||||||||||||||
const tableName = pluralize.plural(matchTable[1].toLocaleLowerCase()); | ||||||||||||||||||||||||
if (tableName !== currentTable) { | ||||||||||||||||||||||||
currentTable = tableName; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
//check for fields | ||||||||||||||||||||||||
const matchAutoIncrement = line.match(autoIncrementFieldPattern()); | ||||||||||||||||||||||||
if (matchAutoIncrement && matchAutoIncrement[1]) { | ||||||||||||||||||||||||
const field = matchAutoIncrement[1]; | ||||||||||||||||||||||||
// Check if the key already exists in the map | ||||||||||||||||||||||||
if (autoIncrementFieldsMap.has(currentTable)) { | ||||||||||||||||||||||||
const existingFields = autoIncrementFieldsMap.get(currentTable); | ||||||||||||||||||||||||
existingFields?.push(field); | ||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||
autoIncrementFieldsMap.set(currentTable, [field]); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
}); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return autoIncrementFieldsMap; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Generates a query based on the first entity discovered | ||||||||||||||||||||||||
* in a schema. If no entities are found in the schema | ||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
import { GraphQLObjectType, GraphQLSchema, printSchema } from 'graphql'; | ||
import knex from 'knex'; | ||
import { GqlEntityController } from '../../../src/graphql/controller'; | ||
import { autoIncrementTag } from '../../../src/types'; | ||
|
||
const regex = new RegExp(autoIncrementTag.source, 'g'); | ||
|
||
describe('GqlEntityController', () => { | ||
describe('generateQueryFields', () => { | ||
|
@@ -17,7 +20,6 @@ type Vote { | |
name: 'Query', | ||
fields: queryFields | ||
}); | ||
|
||
const schema = printSchema(new GraphQLSchema({ query: querySchema })); | ||
expect(schema).toMatchSnapshot(); | ||
}); | ||
|
@@ -56,7 +58,7 @@ type Vote { | |
}); | ||
|
||
it('should work', async () => { | ||
const controller = new GqlEntityController(` | ||
const schema = ` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd use just a single test case to test everything (just put autoincrement and nested properties in here) or |
||
scalar BigInt | ||
scalar Decimal | ||
scalar BigDecimal | ||
|
@@ -69,10 +71,67 @@ type Vote { | |
decimal: Decimal | ||
big_decimal: BigDecimal | ||
} | ||
`); | ||
const { builder } = await controller.createEntityStores(mockKnex); | ||
`; | ||
|
||
const controller = new GqlEntityController(schema); | ||
const { builder } = await controller.createEntityStores(mockKnex, schema); | ||
|
||
const createQuery = builder.toString(); | ||
expect(createQuery).toMatchSnapshot(); | ||
}); | ||
|
||
it('should work with autoincrement', async () => { | ||
const schema = ` | ||
scalar BigInt | ||
scalar Decimal | ||
scalar BigDecimal | ||
|
||
type Vote { | ||
id: Int! @autoIncrement | ||
name: String | ||
authenticators: [String] | ||
big_number: BigInt | ||
decimal: Decimal | ||
big_decimal: BigDecimal | ||
} | ||
|
||
`; | ||
|
||
const schemaGql = schema.replace(regex, ''); | ||
const controller = new GqlEntityController(schemaGql); | ||
const { builder } = await controller.createEntityStores(mockKnex, schema); | ||
const createQuery = builder.toString(); | ||
expect(createQuery).toMatchSnapshot(); | ||
}); | ||
|
||
it('should work with autoincrement and nested objects', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like it could just be the only test case in this file ( |
||
const schema = ` | ||
scalar BigInt | ||
scalar Decimal | ||
scalar BigDecimal | ||
|
||
type Vote { | ||
id: Int! @autoIncrement | ||
name: String | ||
authenticators: [String] | ||
big_number: BigInt | ||
decimal: Decimal | ||
big_decimal: BigDecimal | ||
poster : Poster | ||
} | ||
|
||
type Poster { | ||
id: ID! @autoIncrement | ||
name: String! | ||
} | ||
|
||
`; | ||
|
||
expect(builder.toString()).toMatchSnapshot(); | ||
const schemaGql = schema.replace(new RegExp(autoIncrementTag.source, 'g'), ''); | ||
const controller = new GqlEntityController(schemaGql); | ||
const { builder } = await controller.createEntityStores(mockKnex, schema); | ||
const createQuery = builder.toString(); | ||
expect(createQuery).toMatchSnapshot(); | ||
}); | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this change?