[duplicate-code] ML resource Delete/Read boilerplate duplicated across filter, calendar, calendar_event, calendar_job packages

[github-actions]

Analysis of recent commit history (semantic-code duplication sweep)

Summary

The four "simple, ID-addressed ML sub-resource" packages under internal/elasticsearch/ml/ (filter, calendar, calendar_event, calendar_job) each implement their delete<X>/read<X> entitycore lifecycle callbacks with the identical control flow: log a debug message, call the typed ES client method, treat elasticsearch.IsNotFoundElasticsearchError(err) as a successful no-op, otherwise emit an AddError with the same message shape, then log success. This is the same class of "CRUD wrapper boilerplate" this repo has already extracted into shared entitycore/typeutils helpers for other resource families (see recent commits #4600, #4587, #4586) — these four ML packages appear to have been missed.

Duplication Details

Pattern: "call API → 404-is-success → typed error" wrapper in ML sub-resource delete/read callbacks

• Severity: Medium

• Occurrences: 4 packages (delete.go) + at least 2 confirmed matching read.go (calendar_event/calendar_job read.go use the same IsNotFoundElasticsearchError idiom per grep, not individually re-read)

• Locations:

  • internal/elasticsearch/ml/filter/delete.go:30-53 (24 lines) — verified
  • internal/elasticsearch/ml/calendar/delete.go:30-51 (verified)
  • internal/elasticsearch/ml/calendar_event/delete.go:31-55 (25 lines, adds an ID-splitting step via the already-shared ml.SplitCalendarResourcePath)
  • internal/elasticsearch/ml/calendar_job/delete.go:31-59 (29 lines, same shape as calendar_event)
  • internal/elasticsearch/ml/filter/read.go:30-64
  • internal/elasticsearch/ml/calendar/read.go:30-60

  Note: internal/elasticsearch/ml/anomalydetectionjob/delete.go has a superficially similar shape but is legitimately more complex (close-job + wait-for-closed + retry-with-force) — it is a candidate consumer of a shared "404-as-success" helper for its final delete step, but is not itself evidence of straight duplication and should not be force-fit into this refactor.

• Code Sample (internal/elasticsearch/ml/filter/delete.go:30-53):

  func deleteFilter(ctx context.Context, client *clients.ElasticsearchScopedClient, resourceID string, _ TFModel) fwdiags.Diagnostics {
      var diags fwdiags.Diagnostics
      filterID := resourceID
      if filterID == "" {
          diags.AddError("Invalid resource ID", "filter_id cannot be empty")
          return diags
      }
      tflog.Debug(ctx, fmt.Sprintf("Deleting ML filter: %s", filterID))
      typedClient := client.GetESClient()
      _, err := typedClient.Ml.DeleteFilter(filterID).Do(ctx)
      if err != nil {
          if elasticsearch.IsNotFoundElasticsearchError(err) {
              tflog.Debug(ctx, fmt.Sprintf("ML filter already absent: %s", filterID))
              return diags
          }
          diags.AddError("Failed to delete ML filter", fmt.Sprintf("Unable to delete ML filter: %s — %s", filterID, err.Error()))
          return diags
      }
      tflog.Debug(ctx, fmt.Sprintf("Successfully deleted ML filter: %s", filterID))
      return diags
  }

  Compare internal/elasticsearch/ml/calendar/delete.go:30-51 — the same function with calendar/Calendar substituted for filter/Filter:

  func deleteCalendar(ctx context.Context, client *clients.ElasticsearchScopedClient, resourceID string, _ TFModel) fwdiags.Diagnostics {
      var diags fwdiags.Diagnostics
      calendarID := resourceID
      tflog.Debug(ctx, fmt.Sprintf("Deleting ML calendar: %s", calendarID))
      typedClient := client.GetESClient()
      _, err := typedClient.Ml.DeleteCalendar(calendarID).Do(ctx)
      if err != nil {
          if elasticsearch.IsNotFoundElasticsearchError(err) {
              tflog.Debug(ctx, fmt.Sprintf("ML calendar %s already deleted", calendarID))
              return diags
          }
          diags.AddError("Failed to delete ML calendar", fmt.Sprintf("Unable to delete ML calendar: %s — %s", calendarID, err.Error()))
          return diags
      }
      tflog.Debug(ctx, fmt.Sprintf("Successfully deleted ML calendar: %s", calendarID))
      return diags
  }

Impact Analysis

• Maintainability: Any future change to how 404s are handled, or to error-message conventions, must be applied to 4-6 files by hand.
• Bug Risk: The filter variant validates for an empty ID while calendar does not — a real (small) inconsistency introduced by copy-paste drift, which is exactly the kind of divergence that a shared helper would prevent.
• Code Bloat: ~20-29 lines per file × 4-6 files that could collapse to a handful of lines each plus the type-specific API call.

Refactoring Recommendations

1. Extract shared "404-is-success" delete/read helpers into internal/elasticsearch/ml
   • Alongside the existing shared ml.SplitCalendarResourcePath, add:

     func DeleteWithNotFoundAsSuccess(ctx context.Context, kindLabel, id string, do func() error) fwdiags.Diagnostics
     func ReadWithNotFoundAsAbsent[T any](ctx context.Context, kindLabel, id string, do func() (T, error)) (T, bool, fwdiags.Diagnostics)

   • Each package's delete<X>/read<X> becomes a thin wrapper: extract/validate the ID, call the helper with a closure around the typed client call, map the result into the package's model.
   • Estimated effort: small-medium (2-4 hours) — mechanical per package, but requires generics care for read given differing return model types.
   • Benefits: ~100+ lines removed, consistent 404/error handling and log wording, fixes the filter-vs-calendar empty-ID-validation inconsistency uncovered above.

Implementation Checklist

• Review duplication findings
• Add shared delete/read-with-404-handling helpers in internal/elasticsearch/ml
• Migrate filter, calendar, calendar_event, calendar_job delete.go/read.go to use them
• Resolve the empty-ID-validation inconsistency between filter and calendar during migration
• Update tests
• Verify no functionality broken (make build, relevant unit/acceptance tests)

Analysis Metadata

• Analyzed Files: internal/elasticsearch/ml/{filter,calendar,calendar_event,calendar_job}/{delete,read}.go
• Detection Method: Semantic code analysis (background research agent, spot-verified by re-reading filter/delete.go and calendar/delete.go in full)
• Commit: repository HEAD at analysis time (10af2a6)

Generated by Duplicate Code Detector · sonnet50 · 199.7 AIC · ⌖ 17.8 AIC · ⊞ 8K · ◷