Use the keyspace schema to define the graphql names beforehand #33
Conversation
|
Rebased against master. |
| keyspaceSchema := &KeyspaceGraphQLSchema{ | ||
| ignoredTables: make(map[string]bool), | ||
| schemaGen: sg, | ||
| naming: sg.namingFn(ksNaming), |
There was a problem hiding this comment.
This is so cool! Naming gets updated properly with the latest version of the schema.
config/naming.go
Outdated
| func (n *defaultNaming) ToGraphQLOperation(prefix string, name string) string { | ||
| func (n *snakeCaseToCamelNaming) ToCQLTable(name string) string { | ||
| // lookup table name by entity name | ||
| tableName := n.tablesByEntities[name] |
There was a problem hiding this comment.
minor: Use the tableName, found := n.tablesByEntities[name] syntax? Same in couple other places. If you disagree I really don't mind at all.
There was a problem hiding this comment.
agree, I'll change it
| tableName := n.tablesByEntities[name] | ||
| if tableName == "" { | ||
| // Default to snake_case for tables that doesn't exist yet (DDL) | ||
| return strcase.ToSnake(name) |
There was a problem hiding this comment.
If we use use solution A) from issue #32 then this is an error case, and shouldn't ever happen right? Same applies to ToCQLColumn(), ToGraphQLType(), etc.?
db/db.go
Outdated
| // KeyspaceNamingInfo Retrieves the keyspace naming information | ||
| func (db *Db) KeyspaceNamingInfo(ks *gocql.KeyspaceMetadata) config.KeyspaceNamingInfo { | ||
| result := keyspaceNamingInfo{ | ||
| tables: make(map[string][]string, 0), |
There was a problem hiding this comment.
minor: Reserve capacity (len(ks.Tables)) like for the column below.
| @@ -472,7 +481,11 @@ func (sg *SchemaGenerator) mutationFieldResolver(keyspace *gocql.KeyspaceMetadat | |||
| } | |||
| } | |||
|
|
|||
| func (sg *SchemaGenerator) getModificationResult(rs db.ResultSet, err error) (*types.ModificationResult, error) { | |||
| func (s *KeyspaceGraphQLSchema) getModificationResult( | |||
There was a problem hiding this comment.
These func (s *KeyspaceGraphQLSchema) methods eventually going to be moved to keyspace_schema.go?
If so, thanks for making the commit diff look nice. :)
There was a problem hiding this comment.
yes, it will be moved :)
|
I've addressed the suggestions from the review. |
Generates all the table->entity names beforehand to make the naming conversion operations reversible and avoids name collisions with built-in types.
Fixes #27 and #31 .
I didn't make the code reorg as part of this patch to isolate the changes and make it easier to review.
Also note that this doesn't include the changes discussed in #32 .