diff --git a/mdl/backend/mpr/backend.go b/mdl/backend/mpr/backend.go index 77dde029..c4eb74f5 100644 --- a/mdl/backend/mpr/backend.go +++ b/mdl/backend/mpr/backend.go @@ -768,11 +768,11 @@ func (b *MprBackend) OpenWorkflowForMutation(unitID model.ID) (backend.WorkflowM // WidgetSerializationBackend func (b *MprBackend) SerializeWidget(w pages.Widget) (any, error) { - panic("MprBackend.SerializeWidget not yet implemented") // TODO: implement in PR #237 + return mpr.SerializeWidget(w), nil } func (b *MprBackend) SerializeClientAction(a pages.ClientAction) (any, error) { - panic("MprBackend.SerializeClientAction not yet implemented") // TODO: implement in PR #237 + return mpr.SerializeClientAction(a), nil } func (b *MprBackend) SerializeDataSource(ds pages.DataSource) (any, error) { @@ -780,5 +780,5 @@ func (b *MprBackend) SerializeDataSource(ds pages.DataSource) (any, error) { } func (b *MprBackend) SerializeWorkflowActivity(a workflows.WorkflowActivity) (any, error) { - panic("MprBackend.SerializeWorkflowActivity not yet implemented") // TODO: implement in PR #237 + return mpr.SerializeWorkflowActivity(a), nil } diff --git a/mdl/catalog/builder.go b/mdl/catalog/builder.go index b1a27d9b..db945dfe 100644 --- a/mdl/catalog/builder.go +++ b/mdl/catalog/builder.go @@ -79,8 +79,8 @@ type Builder struct { progress ProgressFunc hierarchy *hierarchy tx CatalogTx // Transaction for batched inserts - fullMode bool // If true, do full parsing (activities/widgets) - sourceMode bool // If true, build source FTS table (implies full) + fullMode bool // If true, do full parsing (activities/widgets) + sourceMode bool // If true, build source FTS table (implies full) describeFunc DescribeFunc // Document caches — avoid redundant BSON parsing across builder phases. diff --git a/mdl/catalog/builder_microflows_test.go b/mdl/catalog/builder_microflows_test.go index 073bbf60..8e1b225a 100644 --- a/mdl/catalog/builder_microflows_test.go +++ b/mdl/catalog/builder_microflows_test.go @@ -5,8 +5,8 @@ package catalog import ( "testing" - "github.com/mendixlabs/mxcli/sdk/microflows" "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" ) // unknownMicroflowObject satisfies MicroflowObject but is not in the type switch. diff --git a/mdl/catalog/builder_pages.go b/mdl/catalog/builder_pages.go index 16769ae1..57b99563 100644 --- a/mdl/catalog/builder_pages.go +++ b/mdl/catalog/builder_pages.go @@ -5,6 +5,7 @@ package catalog import ( "database/sql" "encoding/base64" + "fmt" "strings" "go.mongodb.org/mongo-driver/bson/primitive" @@ -103,7 +104,7 @@ func (b *Builder) buildPages() error { // Insert widgets only in full mode if b.fullMode && len(rawWidgets) > 0 { for _, w := range rawWidgets { - _, _ = widgetStmt.Exec( + if _, err := widgetStmt.Exec( w.ID, w.Name, w.WidgetType, @@ -117,7 +118,9 @@ func (b *Builder) buildPages() error { "", projectID, projectName, snapshotID, snapshotDate, snapshotSource, sourceID, sourceBranch, sourceRevision, - ) + ); err != nil { + return fmt.Errorf("insert widget %s for page %s: %w", w.Name, qualifiedName, err) + } widgetCount++ } } diff --git a/mdl/catalog/catalog.go b/mdl/catalog/catalog.go index 00262238..e3612ec4 100644 --- a/mdl/catalog/catalog.go +++ b/mdl/catalog/catalog.go @@ -284,16 +284,10 @@ func (c *Catalog) SaveToFile(path string) error { } rawDB := sdb.RawDB() - // Open destination file database - destDB, err := sql.Open("sqlite", path) - if err != nil { - return fmt.Errorf("failed to create catalog file: %w", err) - } - defer destDB.Close() - // Use SQLite backup API via VACUUM INTO (SQLite 3.27+) // Fall back to manual copy if not available - _, err = rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", path)) + safePath := strings.ReplaceAll(path, "'", "''") + _, err := rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", safePath)) if err != nil { // Fall back: export and import return c.saveToFileManual(path, rawDB) diff --git a/mdl/executor/cmd_catalog.go b/mdl/executor/cmd_catalog.go index a23f35d2..2a25f736 100644 --- a/mdl/executor/cmd_catalog.go +++ b/mdl/executor/cmd_catalog.go @@ -7,9 +7,12 @@ import ( "context" "encoding/json" "fmt" + "io" + "log" "os" "path/filepath" "strings" + "sync" "time" "github.com/mendixlabs/mxcli/mdl/ast" @@ -18,6 +21,19 @@ import ( "github.com/mendixlabs/mxcli/model" ) +// syncWriter wraps an io.Writer with a mutex so concurrent goroutines +// (e.g. background catalog build) can write without racing. +type syncWriter struct { + mu sync.Mutex + w io.Writer +} + +func (sw *syncWriter) Write(p []byte) (int, error) { + sw.mu.Lock() + defer sw.mu.Unlock() + return sw.w.Write(p) +} + // execShowCatalogTables handles SHOW CATALOG TABLES. func execShowCatalogTables(ctx *ExecContext) error { // Build catalog if not already built (fast mode by default) @@ -449,19 +465,29 @@ func execRefreshCatalogStmt(ctx *ExecContext, stmt *ast.RefreshCatalogStmt) erro // Handle background mode — clone ctx so the goroutine doesn't race // with the main dispatch loop (which may syncBack and mutate fields). - // NOTE: bgCtx.Output still shares the underlying writer with the main - // goroutine. This is a pre-existing limitation — the original code also - // wrote to ctx.Output from the goroutine. A synchronized writer would - // fix this but is out of scope for the executor cleanup. if stmt.Background { - bgCtx := *ctx // shallow copy — isolates scalar fields - bgCtx.Cache = nil // detach shared cache so preWarmCache writes stay local + bgCtx := *ctx // shallow copy — isolates scalar fields + bgCtx.Cache = nil // detach shared cache so preWarmCache writes stay local + sw := &syncWriter{w: ctx.Output} // shared mutex-wrapped writer + bgCtx.Output = sw // background goroutine writes through sw + syncCatalog := ctx.SyncCatalog // capture callback before returning go func() { if err := buildCatalog(&bgCtx, stmt.Full, stmt.Source); err != nil { fmt.Fprintf(bgCtx.Output, "Background catalog build failed: %v\n", err) + return + } + // Propagate the built catalog back to the Executor so the next + // command picks it up. syncBack has already run for this statement, + // so we use the SyncCatalog callback to write directly to e.catalog. + if bgCtx.Catalog != nil { + if syncCatalog != nil { + syncCatalog(bgCtx.Catalog) + } else { + bgCtx.Catalog.Close() + } } }() - fmt.Fprintln(ctx.Output, "Catalog build started in background...") + fmt.Fprintln(sw, "Catalog build started in background...") return nil } @@ -618,7 +644,9 @@ func outputCatalogResults(ctx *ExecContext, result *catalog.QueryResult) { } tr.Rows = append(tr.Rows, outRow) } - _ = writeResult(ctx, tr) + if err := writeResult(ctx, tr); err != nil { + log.Printf("warning: failed to write catalog results: %v", err) + } } // formatValue formats a value for display. diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index b65a4e6a..f4d5f4fd 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -23,25 +23,25 @@ type flowBuilder struct { posY int baseY int // Base Y position (for returning after ELSE branches) spacing int - returnValue string // Return value expression for RETURN statement (used by buildFlowGraph final EndEvent) - endsWithReturn bool // True if the flow already ends with EndEvent(s) from RETURN statements - varTypes map[string]string // Variable name -> entity qualified name (for CHANGE statements) - declaredVars map[string]string // Declared primitive variables: name -> type (e.g., "$IsValid" -> "Boolean") - errors []string // Validation errors collected during build - measurer *layoutMeasurer // For measuring statement dimensions - nextConnectionPoint model.ID // For compound statements: the exit point differs from entry point - nextFlowCase string // If set, next connecting flow uses this case value (for merge-less splits) + returnValue string // Return value expression for RETURN statement (used by buildFlowGraph final EndEvent) + endsWithReturn bool // True if the flow already ends with EndEvent(s) from RETURN statements + varTypes map[string]string // Variable name -> entity qualified name (for CHANGE statements) + declaredVars map[string]string // Declared primitive variables: name -> type (e.g., "$IsValid" -> "Boolean") + errors []string // Validation errors collected during build + measurer *layoutMeasurer // For measuring statement dimensions + nextConnectionPoint model.ID // For compound statements: the exit point differs from entry point + nextFlowCase string // If set, next connecting flow uses this case value (for merge-less splits) // nextFlowAnchor carries the branch-specific FlowAnchors that should be // applied to the flow created by the NEXT iteration of buildFlowGraph. // Used by guard-pattern IFs (where one branch returns and the other // continues) so the continuing branch's @anchor survives to the actual // splitID→nextActivity flow — which is emitted one iteration later by the // outer loop, not by addIfStatement. - nextFlowAnchor *ast.FlowAnchors - backend backend.FullBackend // For looking up page/microflow references - hierarchy *ContainerHierarchy // For resolving container IDs to module names - pendingAnnotations *ast.ActivityAnnotations // Pending annotations to attach to next activity - restServices []*model.ConsumedRestService // Cached REST services for parameter classification + nextFlowAnchor *ast.FlowAnchors + backend backend.FullBackend // For looking up page/microflow references + hierarchy *ContainerHierarchy // For resolving container IDs to module names + pendingAnnotations *ast.ActivityAnnotations // Pending annotations to attach to next activity + restServices []*model.ConsumedRestService // Cached REST services for parameter classification // previousStmtAnchor holds the Anchor annotation of the statement that // just emitted an activity, so the next flow's OriginConnectionIndex can // be overridden by the user. Cleared after each flow is created. diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index 96a6caeb..a19f9a33 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -5,6 +5,7 @@ package executor import ( "fmt" + "log" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -174,7 +175,11 @@ func (fb *flowBuilder) addCallJavaActionAction(s *ast.CallJavaActionStmt) model. // Try to look up the Java action definition to detect EntityTypeParameterType parameters var jaDef *javaactions.JavaAction if fb.backend != nil { - jaDef, _ = fb.backend.ReadJavaActionByName(actionQN) + var err error + jaDef, err = fb.backend.ReadJavaActionByName(actionQN) + if err != nil { + log.Printf("warning: could not look up Java action %s: %v (entity type params will be empty)", actionQN, err) + } } // Build a map of parameter name -> param type for the Java action diff --git a/mdl/executor/cmd_page_wireframe.go b/mdl/executor/cmd_page_wireframe.go index a1f50fd5..3be812f1 100644 --- a/mdl/executor/cmd_page_wireframe.go +++ b/mdl/executor/cmd_page_wireframe.go @@ -6,6 +6,7 @@ import ( "context" "encoding/json" "fmt" + "log" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -485,7 +486,9 @@ func pageToMdlString(ctx *ExecContext, name ast.QualifiedName, root []wireframeN var buf strings.Builder origOutput := ctx.Output ctx.Output = &buf - _ = describePage(ctx, name) + if err := describePage(ctx, name); err != nil { + log.Printf("warning: describePage failed for %s.%s: %v", name.Module, name.Name, err) + } ctx.Output = origOutput mdlSource := buf.String() diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 686b45db..a100e453 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -491,7 +491,11 @@ func (pb *pageBuilder) buildDataSourceV3(ds *ast.DataSourceV3) (pages.DataSource // Fallback to lookup if entity name not stored if entityName == "" { - entityName, _ = pb.getEntityNameByID(entityID) + var err error + entityName, err = pb.getEntityNameByID(entityID) + if err != nil { + log.Printf("warning: could not resolve entity name for ID %s: %v", entityID, err) + } } // Use DataViewSource with IsSnippetParameter flag diff --git a/mdl/executor/cmd_sql.go b/mdl/executor/cmd_sql.go index 10b6ef0e..61293e2a 100644 --- a/mdl/executor/cmd_sql.go +++ b/mdl/executor/cmd_sql.go @@ -16,7 +16,7 @@ import ( // ensureSQLManager lazily initializes the SQL connection manager. func ensureSQLManager(ctx *ExecContext) *sqllib.Manager { - return ctx.EnsureSqlMgr() + return ctx.ensureSqlMgr() } // getOrAutoConnect returns an existing connection or auto-connects using connections.yaml. diff --git a/mdl/executor/exec_context.go b/mdl/executor/exec_context.go index a1e1f092..92ef05ee 100644 --- a/mdl/executor/exec_context.go +++ b/mdl/executor/exec_context.go @@ -78,6 +78,11 @@ type ExecContext struct { // FinalizeFn runs post-execution reconciliation (security rule sync). FinalizeFn func() error + + // SyncCatalog propagates an asynchronously built catalog back to the + // Executor. Used by REFRESH CATALOG BACKGROUND so the goroutine can + // deliver the result after syncBack has already run. + SyncCatalog func(*catalog.Catalog) } // Connected returns true if a project is connected via the Backend. @@ -203,8 +208,8 @@ func (ctx *ExecContext) getCreatedPage(qualifiedName string) *createdPageInfo { return ctx.Cache.createdPages[qualifiedName] } -// EnsureSqlMgr lazily initializes and returns the SQL connection manager. -func (ctx *ExecContext) EnsureSqlMgr() *sqllib.Manager { +// ensureSqlMgr lazily initializes and returns the SQL connection manager. +func (ctx *ExecContext) ensureSqlMgr() *sqllib.Manager { if ctx.SqlMgr == nil { ctx.SqlMgr = sqllib.NewManager() } diff --git a/mdl/executor/executor.go b/mdl/executor/executor.go index 0a6f57af..b4446129 100644 --- a/mdl/executor/executor.go +++ b/mdl/executor/executor.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "io" + "sync" "time" "github.com/mendixlabs/mxcli/mdl/ast" @@ -184,6 +185,8 @@ type Executor struct { sqlMgr *sqllib.Manager // external SQL connection manager (lazy init) themeRegistry *ThemeRegistry // cached theme design property definitions (lazy init) registry *Registry // statement dispatch registry + catalogMu sync.RWMutex // protects catalog field from background goroutine writes + catalogGen uint64 // monotonic generation counter for catalog swaps } // New creates a new executor with the given output writer. @@ -300,7 +303,10 @@ func (e *Executor) finalizeProgramExecution() error { // Catalog returns the catalog, or nil if not built. func (e *Executor) Catalog() *catalog.Catalog { - return e.catalog + e.catalogMu.RLock() + c := e.catalog + e.catalogMu.RUnlock() + return c } // Reader returns the MPR reader, or nil if not connected. diff --git a/mdl/executor/executor_dispatch.go b/mdl/executor/executor_dispatch.go index ed417846..302bf7e4 100644 --- a/mdl/executor/executor_dispatch.go +++ b/mdl/executor/executor_dispatch.go @@ -6,6 +6,7 @@ import ( "context" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/catalog" ) // executeInner dispatches a statement to its registered handler. @@ -16,6 +17,8 @@ func (e *Executor) executeInner(ctx context.Context, stmt ast.Statement) error { // executeInner in a goroutine with a wall-clock timeout; if the timeout // fires, the goroutine keeps running but Execute() has already returned. // Syncing stale state back at that point would race with subsequent calls. + // Any handler-side state changes made after cancellation are intentionally + // lost — this is expected behavior, not a regression. if ctx.Err() == nil { e.syncBack(ectx) } @@ -34,7 +37,20 @@ func (e *Executor) syncBack(ctx *ExecContext) { e.backend = ctx.Backend e.mprPath = ctx.MprPath e.cache = ctx.Cache + e.catalogMu.Lock() + old := e.catalog e.catalog = ctx.Catalog + if old != ctx.Catalog { + e.catalogGen++ + } + e.catalogMu.Unlock() + // Close the previously installed catalog whenever ownership moved to a + // different catalog value. This includes transitions to nil; otherwise the + // previous catalog can leak if a handler clears ctx.Catalog without closing + // the old instance itself. + if old != nil && old != ctx.Catalog { + old.Close() + } e.settings = ctx.Settings e.fragments = ctx.Fragments e.sqlMgr = ctx.SqlMgr @@ -43,6 +59,10 @@ func (e *Executor) syncBack(ctx *ExecContext) { // newExecContext builds an ExecContext from the current Executor state. func (e *Executor) newExecContext(ctx context.Context) *ExecContext { + e.catalogMu.RLock() + cat := e.catalog + gen := e.catalogGen + e.catalogMu.RUnlock() return &ExecContext{ Context: ctx, Backend: e.backend, @@ -51,7 +71,7 @@ func (e *Executor) newExecContext(ctx context.Context) *ExecContext { Quiet: e.quiet, Logger: e.logger, Fragments: e.fragments, - Catalog: e.catalog, + Catalog: cat, Cache: e.cache, MprPath: e.mprPath, SqlMgr: e.sqlMgr, @@ -61,6 +81,24 @@ func (e *Executor) newExecContext(ctx context.Context) *ExecContext { ExecuteFn: e.Execute, ExecuteProgramFn: e.ExecuteProgram, FinalizeFn: e.finalizeProgramExecution, + SyncCatalog: func(cat *catalog.Catalog) { + e.catalogMu.Lock() + defer e.catalogMu.Unlock() + // Only apply the background result if no newer catalog has been + // installed since the build started (generation check). This + // prevents an out-of-date background build from overwriting a + // fresher foreground refresh. + if e.catalogGen != gen { + cat.Close() + return + } + old := e.catalog + e.catalog = cat + e.catalogGen++ + if old != nil && old != cat { + old.Close() + } + }, } } diff --git a/mdl/executor/hierarchy.go b/mdl/executor/hierarchy.go index b661bab2..357c045c 100644 --- a/mdl/executor/hierarchy.go +++ b/mdl/executor/hierarchy.go @@ -159,5 +159,3 @@ func invalidateDomainModelsCache(ctx *ExecContext) { ctx.Cache.domainModels = nil } } - - diff --git a/mdl/types/id.go b/mdl/types/id.go index e8ecf219..b5ee5db9 100644 --- a/mdl/types/id.go +++ b/mdl/types/id.go @@ -105,15 +105,3 @@ func ValidateID(id string) bool { } return true } - -// Hash computes a hash for content (used for content deduplication). -// TODO: replace with SHA-256 (or similar) — the current positional checksum is -// weak and produces collisions easily. Deferred to avoid breaking callers that -// may depend on the output format/length. -func Hash(content []byte) string { - var sum uint64 - for i, b := range content { - sum += uint64(b) * uint64(i+1) - } - return fmt.Sprintf("%016x", sum) -} diff --git a/mdl/types/id_test.go b/mdl/types/id_test.go index aa138154..f252e06d 100644 --- a/mdl/types/id_test.go +++ b/mdl/types/id_test.go @@ -168,26 +168,3 @@ func TestValidateID(t *testing.T) { } } } - -func TestHash_Deterministic(t *testing.T) { - h1 := Hash([]byte("hello")) - h2 := Hash([]byte("hello")) - if h1 != h2 { - t.Fatalf("Hash not deterministic: %q vs %q", h1, h2) - } -} - -func TestHash_DifferentInputs(t *testing.T) { - h1 := Hash([]byte("hello")) - h2 := Hash([]byte("world")) - if h1 == h2 { - t.Fatal("Hash collision on different inputs") - } -} - -func TestHash_EmptyInput(t *testing.T) { - h := Hash([]byte{}) - if h != "0000000000000000" { - t.Errorf("expected zero hash for empty input, got %q", h) - } -} diff --git a/sdk/microflows/microflows_actions.go b/sdk/microflows/microflows_actions.go index 710b9810..c36dd1a2 100644 --- a/sdk/microflows/microflows_actions.go +++ b/sdk/microflows/microflows_actions.go @@ -175,8 +175,8 @@ type AggregateListAction struct { Function AggregateFunction `json:"function"` AttributeID model.ID `json:"attributeId,omitempty"` AttributeQualifiedName string `json:"attributeQualifiedName,omitempty"` // BY_NAME_REFERENCE: Module.Entity.Attribute - UseExpression bool `json:"useExpression,omitempty"` // true when Expression is used instead of Attribute - Expression string `json:"expression,omitempty"` // Mendix expression string (when UseExpression=true) + UseExpression bool `json:"useExpression,omitempty"` // true when Expression is used instead of Attribute + Expression string `json:"expression,omitempty"` // Mendix expression string (when UseExpression=true) } func (AggregateListAction) isMicroflowAction() {} diff --git a/sdk/mpr/utils.go b/sdk/mpr/utils.go index 5042cb14..17609ac9 100644 --- a/sdk/mpr/utils.go +++ b/sdk/mpr/utils.go @@ -37,11 +37,6 @@ func BsonBinaryToID(bin primitive.Binary) string { return bsonutil.BsonBinaryToID(bin) } -// Hash computes a hash for content (used for content deduplication). -func Hash(content []byte) string { - return types.Hash(content) -} - // ValidateID checks if an ID is valid. func ValidateID(id string) bool { return types.ValidateID(id) diff --git a/sdk/mpr/writer_units.go b/sdk/mpr/writer_units.go index 4e0662cd..d0fd9b6c 100644 --- a/sdk/mpr/writer_units.go +++ b/sdk/mpr/writer_units.go @@ -30,12 +30,13 @@ func isContentsHashSchemaError(err error) bool { // updateTransactionID updates the _Transaction table with a new UUID. // Studio Pro uses this to detect external changes during F4 sync. // Only applies to MPR v2 projects (Mendix >= 10.18). -func (w *Writer) updateTransactionID() { +func (w *Writer) updateTransactionID() error { if w.reader.version != MPRVersionV2 { - return + return nil } newID := generateUUID() - _, _ = w.reader.db.Exec(`UPDATE _Transaction SET LastTransactionID = ?`, newID) + _, err := w.reader.db.Exec(`UPDATE _Transaction SET LastTransactionID = ?`, newID) + return err } // placeholderBinaryPrefix is the GUID-swapped byte pattern for placeholder IDs generated @@ -102,7 +103,9 @@ func (w *Writer) insertUnit(unitID, containerID, containmentName, unitType strin return err } w.reader.InvalidateCache() - w.updateTransactionID() + if err := w.updateTransactionID(); err != nil { + return fmt.Errorf("failed to update transaction ID: %w", err) + } return nil } @@ -158,7 +161,9 @@ func (w *Writer) updateUnit(unitID string, contents []byte) error { `, contentsHash, unitIDBlob) if err == nil { w.reader.InvalidateCache() - w.updateTransactionID() + if txErr := w.updateTransactionID(); txErr != nil { + return fmt.Errorf("failed to update transaction ID: %w", txErr) + } } return err } @@ -223,7 +228,9 @@ func (w *Writer) deleteUnit(unitID string) error { } w.reader.InvalidateCache() - w.updateTransactionID() + if err := w.updateTransactionID(); err != nil { + return fmt.Errorf("failed to update transaction ID after deleting unit: %w", err) + } return nil }