diff --git a/internal/umodel/import_test.go b/internal/umodel/import_test.go index 85ed675..d37a676 100644 --- a/internal/umodel/import_test.go +++ b/internal/umodel/import_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "github.com/alibaba/UnifiedModel/internal/graphstore" @@ -38,6 +39,72 @@ func TestValidateAcceptsValidUModelElements(t *testing.T) { } } +func TestValidateReportsAuthoringWarnings(t *testing.T) { + svc := NewService(graphstore.NewMemoryStore(), WithImportRoot("/")) + result, err := svc.Validate(context.Background(), "demo", []model.UModelElement{ + { + Kind: "entity_set", + Domain: "devops", + Name: "devops.service", + Spec: map[string]any{}, + }, + { + Kind: "metric_set", + Domain: "devops", + Name: "devops.metric.service", + Spec: map[string]any{"metrics": []any{ + map[string]any{"name": "request_count", "type": "counter", "generator": "sum(rate(http_requests_total[1m]))"}, + map[string]any{"name": "latency_p99", "type": "gauge", "data_format": "ms", "generator": "histogram_quantile(0.99, rate(http_request_duration_seconds_bucket[1m]))"}, + }}, + }, + { + Kind: "entity_set_link", + Domain: "devops", + Name: "devops.service_candidate_related_to_devops.deployment", + Spec: map[string]any{ + "src": map[string]any{"domain": "devops", "kind": "entity_set", "name": "devops.service"}, + "dest": map[string]any{"domain": "devops", "kind": "entity_set", "name": "devops.deployment"}, + "entity_link_type": "candidate_related_to", + }, + }, + { + Kind: "entity_set_link", + Domain: "devops", + Name: "devops.service_related_to_devops.deployment", + Spec: map[string]any{ + "src": map[string]any{"domain": "devops", "kind": "entity_set", "name": "devops.service"}, + "dest": map[string]any{"domain": "devops", "kind": "entity_set", "name": "devops.deployment"}, + "entity_link_type": "related_to", + }, + }, + }) + if err != nil { + t.Fatalf("validate: %v", err) + } + if !result.Valid || len(result.Errors) != 0 { + t.Fatalf("authoring warnings must not reject valid elements, got %+v", result) + } + + want := map[string][]string{ + "elements[0].spec.fields": {"entity_set", "may limit"}, + "elements[1].spec.metrics[0].unit": {"metric", "unit", "data_format"}, + "elements[2].spec.entity_link_type": {"provisional", "explicit"}, + } + for field, parts := range want { + if !hasWarningContaining(result.Warnings, field, parts...) { + t.Fatalf("expected warning %s containing %v, got %+v", field, parts, result.Warnings) + } + } + for _, field := range []string{ + "elements[1].spec.metrics[1].unit", + "elements[3].spec.entity_link_type", + } { + if hasWarningField(result.Warnings, field) { + t.Fatalf("unexpected warning for %s: %+v", field, result.Warnings) + } + } +} + func TestValidateReportsUnsupportedEnvelopeVersionOnVersionField(t *testing.T) { svc := NewService(graphstore.NewMemoryStore(), WithImportRoot("/")) result, err := svc.Validate(context.Background(), "demo", []model.UModelElement{{ @@ -232,3 +299,27 @@ func writeFile(t *testing.T, path, content string) { t.Fatalf("write %s: %v", path, err) } } + +func hasWarningContaining(warnings []model.ErrorDetail, field string, parts ...string) bool { + for _, warning := range warnings { + if warning.Field != field { + continue + } + for _, part := range parts { + if !strings.Contains(warning.Reason, part) { + return false + } + } + return true + } + return false +} + +func hasWarningField(warnings []model.ErrorDetail, field string) bool { + for _, warning := range warnings { + if warning.Field == field { + return true + } + } + return false +} diff --git a/internal/umodel/service.go b/internal/umodel/service.go index 762efdb..991bd6b 100644 --- a/internal/umodel/service.go +++ b/internal/umodel/service.go @@ -106,6 +106,7 @@ func (s *Service) Validate(ctx context.Context, workspace string, elements []mod }) continue } + hasSchemaErrors := len(res.Errors) > 0 for _, e := range res.Errors { errors = append(errors, model.ErrorDetail{ Field: fmt.Sprintf("elements[%d].%s", i, e.Path), @@ -118,6 +119,9 @@ func (s *Service) Validate(ctx context.Context, workspace string, elements []mod Reason: w.Reason, }) } + if !hasSchemaErrors { + warnings = appendAuthoringWarnings(warnings, i, element) + } } return model.ValidationResult{ Valid: len(errors) == 0, @@ -126,6 +130,53 @@ func (s *Service) Validate(ctx context.Context, workspace string, elements []mod }, nil } +func appendAuthoringWarnings(warnings []model.ErrorDetail, index int, element model.UModelElement) []model.ErrorDetail { + switch element.Kind { + case "entity_set": + fields, ok := element.Spec["fields"].([]any) + if !ok || len(fields) == 0 { + warnings = append(warnings, model.ErrorDetail{ + Field: fmt.Sprintf("elements[%d].spec.fields", index), + Reason: "entity_set has no fields; this may limit schema guidance for entity writes and filters", + }) + } + case "metric_set": + metrics, ok := element.Spec["metrics"].([]any) + if !ok { + return warnings + } + for i, metric := range metrics { + spec, ok := metric.(map[string]any) + if !ok { + continue + } + unit, ok := spec["unit"].(string) + dataFormat, hasDataFormat := spec["data_format"].(string) + if (!ok || strings.TrimSpace(unit) == "") && (!hasDataFormat || strings.TrimSpace(dataFormat) == "") { + warnings = append(warnings, model.ErrorDetail{ + Field: fmt.Sprintf("elements[%d].spec.metrics[%d].unit", index, i), + Reason: "metric has no unit or data_format; define display formatting so query results are interpretable", + }) + } + } + case "entity_set_link": + linkType, ok := element.Spec["entity_link_type"].(string) + if ok && weakEntityLinkType(linkType) { + warnings = append(warnings, model.ErrorDetail{ + Field: fmt.Sprintf("elements[%d].spec.entity_link_type", index), + Reason: "entity_link_type is weak or provisional; use explicit topology semantics for hard entity links", + }) + } + } + return warnings +} + +func weakEntityLinkType(value string) bool { + normalized := strings.ToLower(strings.TrimSpace(value)) + return strings.HasPrefix(normalized, "candidate_") || + strings.HasPrefix(normalized, "maybe_") +} + func (s *Service) PutElements(ctx context.Context, batch model.UModelElementBatch) (model.WriteResult, error) { if batch.Workspace == "" { return model.WriteResult{}, apperrors.New(apperrors.CodeInvalidArgument, "workspace is required")