diff --git a/pkg/common/adapters/database/query_metrics_test.go b/pkg/common/adapters/database/query_metrics_test.go index a91bf49..965e9fd 100644 --- a/pkg/common/adapters/database/query_metrics_test.go +++ b/pkg/common/adapters/database/query_metrics_test.go @@ -82,6 +82,20 @@ type queryMetricsBunUser struct { Name string `bun:"name"` } +type queryMetricsBunParent struct { + bun.BaseModel `bun:"table:metrics_bun_parents"` + ID int64 `bun:"id,pk,autoincrement"` + Name string `bun:"name"` + Children []queryMetricsBunChild `bun:"rel:has-many,join:id=parent_id"` +} + +type queryMetricsBunChild struct { + bun.BaseModel `bun:"table:metrics_bun_children"` + ID int64 `bun:"id,pk,autoincrement"` + ParentID int64 `bun:"parent_id"` + Name string `bun:"name"` +} + func TestPgSQLAdapterRecordsSchemaEntityTableMetrics(t *testing.T) { db, mock, err := sqlmock.New() require.NoError(t, err) @@ -346,3 +360,35 @@ func TestBunAdapterRecordsEntityAndTableMetrics(t *testing.T) { assert.Equal(t, "query_metrics_bun_user", calls[0].entity) assert.Equal(t, "metrics_bun_users", calls[0].table) } + +func TestBunSelectQueryScanModelSupportsHasManyPreload(t *testing.T) { + sqldb, err := sql.Open(sqliteshim.ShimName, "file::memory:?cache=shared") + require.NoError(t, err) + defer sqldb.Close() + + db := bun.NewDB(sqldb, sqlitedialect.New()) + defer db.Close() + ctx := context.Background() + + _, err = db.NewCreateTable().Model((*queryMetricsBunParent)(nil)).IfNotExists().Exec(ctx) + require.NoError(t, err) + _, err = db.NewCreateTable().Model((*queryMetricsBunChild)(nil)).IfNotExists().Exec(ctx) + require.NoError(t, err) + + parent := &queryMetricsBunParent{Name: "parent"} + _, err = db.NewInsert().Model(parent).Exec(ctx) + require.NoError(t, err) + _, err = db.NewInsert().Model(&queryMetricsBunChild{ParentID: parent.ID, Name: "child"}).Exec(ctx) + require.NoError(t, err) + + adapter := NewBunAdapter(db) + var parents []queryMetricsBunParent + err = adapter.NewSelect().Model(&parents).PreloadRelation("Children").Scan(ctx, &parents) + require.ErrorContains(t, err, "use Model instead of the dest parameter in Scan") + + parents = nil + err = adapter.NewSelect().Model(&parents).PreloadRelation("Children").ScanModel(ctx) + require.NoError(t, err) + require.Len(t, parents, 1) + require.Len(t, parents[0].Children, 1) +} diff --git a/pkg/resolvemcp/handler.go b/pkg/resolvemcp/handler.go index ac1f3f6..6bb462e 100644 --- a/pkg/resolvemcp/handler.go +++ b/pkg/resolvemcp/handler.go @@ -340,18 +340,28 @@ func (h *Handler) executeRead(ctx context.Context, schema, entity, id string, op var data interface{} if id != "" { - singleResult := reflect.New(modelType).Interface() - pkName := reflection.GetPrimaryKeyName(singleResult) + pkName := reflection.GetPrimaryKeyName(model) query = query.Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), id) - if err := query.Scan(ctx, singleResult); err != nil { + // Scan through the model configured on the query. Bun rejects Scan with + // a destination when the query preloads a has-many relation. + if err := query.ScanModel(ctx); err != nil { if err == sql.ErrNoRows { return nil, nil, fmt.Errorf("record not found") } return nil, nil, fmt.Errorf("query error: %w", err) } - data = singleResult + + // The configured model is a slice so the same query construction works + // for both collection and single-record reads. Extract its one result. + scannedResults := reflect.ValueOf(modelPtr).Elem() + if scannedResults.Len() == 0 { + return nil, nil, fmt.Errorf("record not found") + } + data = scannedResults.Index(0).Interface() } else { - if err := query.Scan(ctx, modelPtr); err != nil { + // Use the model already configured on the query. This is required by + // Bun whenever the query includes a has-many preload. + if err := query.ScanModel(ctx); err != nil { return nil, nil, fmt.Errorf("query error: %w", err) } data = reflect.ValueOf(modelPtr).Elem().Interface() diff --git a/pkg/resolvespec/handler.go b/pkg/resolvespec/handler.go index 85bb5f7..60d8629 100644 --- a/pkg/resolvespec/handler.go +++ b/pkg/resolvespec/handler.go @@ -565,9 +565,7 @@ func (h *Handler) handleRead(ctx context.Context, w common.ResponseWriter, id st logger.Debug("Querying single record with FetchRowNumber ID: %s", targetID) } - // For single record, create a new pointer to the struct type - singleResult := reflect.New(modelType).Interface() - pkName := reflection.GetPrimaryKeyName(singleResult) + pkName := reflection.GetPrimaryKeyName(model) query = query.Where(fmt.Sprintf("%s = ?", common.QuoteIdent(pkName)), targetID) @@ -580,12 +578,21 @@ func (h *Handler) handleRead(ctx context.Context, w common.ResponseWriter, id st } query = hookCtx.Query - if err := query.Scan(ctx, singleResult); err != nil { + // Scan through the model configured on the query. Bun rejects Scan with + // a destination when the query preloads a has-many relation. + if err := query.ScanModel(ctx); err != nil { logger.Error("Error querying record: %v", err) statusCode, errCode, errMsg = http.StatusInternalServerError, "query_error", "Error executing query" return err } - result = singleResult + + // The configured model is a slice so the same query construction works + // for both collection and single-record reads. Extract its one result. + scannedResults := reflect.ValueOf(modelPtr).Elem() + if scannedResults.Len() == 0 { + return sql.ErrNoRows + } + result = scannedResults.Index(0).Interface() } else { logger.Debug("Querying multiple records") @@ -598,8 +605,9 @@ func (h *Handler) handleRead(ctx context.Context, w common.ResponseWriter, id st } query = hookCtx.Query - // Use the modelPtr already created and set on the query - if err := query.Scan(ctx, modelPtr); err != nil { + // Use the model already configured on the query. This is required by + // Bun whenever the query includes a has-many preload. + if err := query.ScanModel(ctx); err != nil { logger.Error("Error querying records: %v", err) statusCode, errCode, errMsg = http.StatusInternalServerError, "query_error", "Error executing query" return err