Generating Store Layer methods based on Yaml files #64
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive store layer generator to gofr-cli that generates GoFr-compatible store interfaces and implementations from YAML configuration files. The generator supports multi-store architecture with external model referencing and smart generation features.
- Adds YAML-driven store layer code generation with CRUD operations support
- Implements multi-store architecture allowing multiple isolated stores in a single configuration
- Introduces external model referencing to use existing model files instead of generating new ones
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| store/generator.go | Core generator implementation with YAML parsing, templating, and file generation logic |
| store/example.yaml | Comprehensive example configuration demonstrating multi-store setup with external and generated models |
| store/README.md | Detailed documentation covering features, usage, configuration options, and best practices |
| main.go | Integration of store commands into the CLI application |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@coolwednesday Looks like your PR has some code quality issues: Can we please fix them. |
| @@ -0,0 +1,700 @@ | |||
| # GoFr Store Generator | |||
There was a problem hiding this comment.
The README is currently too long and repetitive - concepts like "registry management" and "appending logic" appear in multiple sections. Consider reorganizing into: 1) Quick Start, 2) Basic Usage, 3) Configuration Reference, and 4) Advanced Topics. Cut duplicate explanations and focus on practical examples rather than implementation details.
| // generateSingleStore generates a single store. | ||
| func generateSingleStore(ctx *gofr.Context, config *Config, store *Info) error { | ||
| outputDir := store.OutputDir | ||
| if outputDir == "" { |
There was a problem hiding this comment.
Shoudn't we have a validation here over store name before it is used to create directories? What if user gives 123store or user-profile?? These are not correct go package naming conventions.
| } | ||
|
|
||
| // collectImports collects all required imports for the generated code. | ||
| func collectImports(config *Config) []string { |
There was a problem hiding this comment.
This method should canonicalize import paths (trimmed, unquoted, no indentation) and add alias support, so comparisons are reliable and duplicates or name collisions are avoided.
|
|
||
| // appendStoreEntries appends new stores to stores/all.go without overwriting existing entries. | ||
| func appendStoreEntries(ctx *gofr.Context, newStores []Entry) error { | ||
| projectModule := detectProjectModule() |
There was a problem hiding this comment.
Normalize imports consistently; current whitespace mismatch can add duplicates. Prefer AST for imports/map insertion and write files atomically via go/format.
| } | ||
|
|
||
| // processExistingAllFile handles the logic for updating an existing all.go file. | ||
| func processExistingAllFile(ctx *gofr.Context, content []byte, |
There was a problem hiding this comment.
Normalize imports consistently; current whitespace mismatch can add duplicates. Prefer AST for imports/map insertion and write files atomically via go/format.
| } | ||
|
|
||
| // handleImportSection adds import section if missing or appends to existing one. | ||
| func handleImportSection(lines, importsToAdd []string) []string { |
There was a problem hiding this comment.
Consolidate scattered import logic into one helper or use astutil.AddImport; reduces duplication and formatting issues.
| } | ||
|
|
||
| // findMapInsertionPoint finds where to insert new store entries in the map. | ||
| func findMapInsertionPoint(lines []string) int { |
There was a problem hiding this comment.
String-based brace counting is fragile; replace with AST parsing to locate func All() return map reliably.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ` | ||
|
|
||
| // ImplementationTemplate is the template for generating store implementations. | ||
| const ImplementationTemplate = `// Code generated by gofr.dev/cli/gofr. |
There was a problem hiding this comment.
Missing 'DO NOT EDIT.' comment in the ImplementationTemplate header. This should match the format used in other templates.
| const ImplementationTemplate = `// Code generated by gofr.dev/cli/gofr. | |
| const ImplementationTemplate = `// Code generated by gofr.dev/cli/gofr. DO NOT EDIT. |
| ) | ||
|
|
||
| // storeRegex matches store entries in all.go file. | ||
| var storeRegex = regexp.MustCompile(`^\s*"([^"]+)"\s*:\s*func\(\)\s*any\s*\{`) |
There was a problem hiding this comment.
The regex pattern should be more robust to handle variations in whitespace and formatting. Consider using a more flexible pattern that accounts for different spacing and line breaks that might exist in real files.
| var storeRegex = regexp.MustCompile(`^\s*"([^"]+)"\s*:\s*func\(\)\s*any\s*\{`) | |
| var storeRegex = regexp.MustCompile(`(?m)\s*"([^"]+)"\s*:\s*func\s*\(\s*\)\s*any\s*\{`) |
| t, err := template.New("model").Funcs(template.FuncMap{ | ||
| "lower": strings.ToLower, | ||
| }).Parse(ModelTemplate) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to parse model template: %w", err) | ||
| } | ||
|
|
||
| file, err := os.Create(modelFile) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create model file: %w", err) | ||
| } | ||
|
|
||
| // Pass the store and model context correctly | ||
| store := config.Stores[0] | ||
| if err := t.Execute(file, struct { | ||
| Store Info | ||
| Model Model | ||
| }{store, model}); err != nil { | ||
| file.Close() // Close file before returning error | ||
| return fmt.Errorf("failed to execute model template: %w", err) | ||
| } | ||
|
|
||
| file.Close() // Close file explicitly instead of defer in loop | ||
| ctx.Logger.Infof("Generated model file: %s", modelFile) | ||
| } | ||
|
|
There was a problem hiding this comment.
The explicit file.Close() calls are good practice in loops, but consider using a function to encapsulate the model generation logic to maintain proper defer usage while avoiding the loop defer issue.
| t, err := template.New("model").Funcs(template.FuncMap{ | |
| "lower": strings.ToLower, | |
| }).Parse(ModelTemplate) | |
| if err != nil { | |
| return fmt.Errorf("failed to parse model template: %w", err) | |
| } | |
| file, err := os.Create(modelFile) | |
| if err != nil { | |
| return fmt.Errorf("failed to create model file: %w", err) | |
| } | |
| // Pass the store and model context correctly | |
| store := config.Stores[0] | |
| if err := t.Execute(file, struct { | |
| Store Info | |
| Model Model | |
| }{store, model}); err != nil { | |
| file.Close() // Close file before returning error | |
| return fmt.Errorf("failed to execute model template: %w", err) | |
| } | |
| file.Close() // Close file explicitly instead of defer in loop | |
| ctx.Logger.Infof("Generated model file: %s", modelFile) | |
| } | |
| store := config.Stores[0] | |
| if err := generateModelFile(modelFile, store, model); err != nil { | |
| return err | |
| } | |
| ctx.Logger.Infof("Generated model file: %s", modelFile) | |
| } | |
| return nil | |
| } | |
| // generateModelFile creates a model file using the provided store and model information. | |
| func generateModelFile(modelFile string, store Info, model Model) error { | |
| t, err := template.New("model").Funcs(template.FuncMap{ | |
| "lower": strings.ToLower, | |
| }).Parse(ModelTemplate) | |
| if err != nil { | |
| return fmt.Errorf("failed to parse model template: %w", err) | |
| } | |
| file, err := os.Create(modelFile) | |
| if err != nil { | |
| return fmt.Errorf("failed to create model file: %w", err) | |
| } | |
| defer file.Close() | |
| if err := t.Execute(file, struct { | |
| Store Info | |
| Model Model | |
| }{store, model}); err != nil { | |
| return fmt.Errorf("failed to execute model template: %w", err) | |
| } |
| // processExistingAllFile handles the logic for updating an existing all.go file. | ||
| func processExistingAllFile(ctx *gofr.Context, content []byte, | ||
| newStores []Entry, projectModule string) error { | ||
| lines := strings.Split(string(content), "\n") | ||
|
|
||
| // Parse existing stores and imports more carefully | ||
| existingStores, existingImports := parseExistingAllFile(lines) | ||
|
|
||
| // Filter out stores that already exist and collect imports to add | ||
| storesToAdd, importsToAdd := filterNewStores(newStores, | ||
| existingStores, existingImports, projectModule) | ||
|
|
||
| if len(storesToAdd) == 0 { | ||
| ctx.Logger.Info("All stores already exist in all.go") | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| return updateAllFileWithNewStores(ctx, lines, storesToAdd, | ||
| importsToAdd, existingStores, projectModule) | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] This function has multiple responsibilities (parsing, filtering, updating). Consider breaking it down into smaller, single-responsibility functions for better maintainability and testing.
| // processExistingAllFile handles the logic for updating an existing all.go file. | |
| func processExistingAllFile(ctx *gofr.Context, content []byte, | |
| newStores []Entry, projectModule string) error { | |
| lines := strings.Split(string(content), "\n") | |
| // Parse existing stores and imports more carefully | |
| existingStores, existingImports := parseExistingAllFile(lines) | |
| // Filter out stores that already exist and collect imports to add | |
| storesToAdd, importsToAdd := filterNewStores(newStores, | |
| existingStores, existingImports, projectModule) | |
| if len(storesToAdd) == 0 { | |
| ctx.Logger.Info("All stores already exist in all.go") | |
| return nil | |
| } | |
| return updateAllFileWithNewStores(ctx, lines, storesToAdd, | |
| importsToAdd, existingStores, projectModule) | |
| } | |
| // splitContentToLines splits the file content into lines. | |
| func splitContentToLines(content []byte) []string { | |
| return strings.Split(string(content), "\n") | |
| } | |
| // handleExistingAllFile orchestrates the update process for an existing all.go file. | |
| func handleExistingAllFile(ctx *gofr.Context, content []byte, newStores []Entry, projectModule string) error { | |
| lines := splitContentToLines(content) | |
| existingStores, existingImports := parseExistingAllFile(lines) | |
| storesToAdd, importsToAdd := filterNewStores(newStores, existingStores, existingImports, projectModule) | |
| if len(storesToAdd) == 0 { | |
| ctx.Logger.Info("All stores already exist in all.go") | |
| return nil | |
| } | |
| return updateAllFileWithNewStores(ctx, lines, storesToAdd, importsToAdd, existingStores, projectModule) | |
| } | |
| // processExistingAllFile delegates to handleExistingAllFile for backward compatibility. | |
| func processExistingAllFile(ctx *gofr.Context, content []byte, newStores []Entry, projectModule string) error { | |
| return handleExistingAllFile(ctx, content, newStores, projectModule) | |
| } |
2 participants
Add Store Layer Generator to gofr-cli
Pull Request: Enhanced Store Generator & Documentation Overhaul
Overview
This PR delivers a fully-refactored GoFr Store Generator that is linter-clean, feature-rich, and thoroughly documented.
Key deliverables:
user,product,order,category) showcasing every query & return type.stores/all.goinit/generate) and multi-store workflowmain.gointegration, registry usage, DI pattern, env overridesHow to Test
gofr store init -name=userthengofr store init -name=product– checkstores/all.goappends without dupes.gofr store generate -config=example.yaml– generates four stores and a clean registry;go vet ./...andgo testsucceed.Backward Compatibility
stores/all.gofiles are preserved—new stores append safely.Next Steps