Skip to content

[semantic-refactor] Extract shared poll-until-state helper (4 near-duplicate wrappers around asyncutils.WaitForStateTransition)Β #4636

Description

@github-actions

πŸ”§ Semantic Refactor: Poll-until-state wrappers duplicated across 4 packages

Analysis of repository: elastic/terraform-provider-elasticstack

Summary

Four independent implementations of the same "fetch status β†’ build a state-checker closure β†’ call asyncutils.WaitForStateTransition β†’ translate the resulting error into diag.Diagnostics" pattern exist in different packages. Each reinvents the same fetch/closure/error-translation boilerplate around the shared low-level poller asyncutils.WaitForStateTransition (internal/asyncutils/state_waiter.go:57). A single generic helper in asyncutils would remove ~150 lines of repeated plumbing and give future resources (any Elasticsearch/Kibana entity with an async "reach desired state" step) a one-line way to opt in.

Concrete Evidence

Opportunity: extract a generic PollUntilState-style helper

  • Severity: Medium

  • Type: extraction-opportunity / generics-candidate

  • Locations:

    • internal/elasticsearch/ml/datafeed/state_utils.go:41-92 β€” GetDatafeedState fetches stats and reduces to a State string; WaitForDatafeedState builds a stateChecker closure that calls GetDatafeedState, compares to desiredState, treats a fixed "terminal states" set as a hard failure, and calls asyncutils.WaitForStateTransition.
    • internal/elasticsearch/ml/jobstate/state_utils.go:33-66 β€” getJobState fetches ML job stats and reduces to a *string; waitForJobState builds an equivalent stateChecker closure and calls asyncutils.WaitForStateTransition.
    • internal/clients/elasticsearch/ml_anomaly_job.go:77-99 β€” WaitForMLJobClosed fetches job stats via GetMLJobStats, builds an isJobClosed closure, does an "immediate check before entering the poll loop" optimization (the same optimization is re-implemented independently in the entity-store version below), then calls asyncutils.WaitForStateTransition.
    • internal/kibana/security_entity_store/helpers.go:369-510 β€” getEntityStoreStatus fetches and JSON-decodes status; waitForUninstall/waitForStarted each build their own entityStoreStatusFunc-based state-checker (makeUninstallStateChecker line 409, makeStartedStateChecker line 472), each repeat the "immediate check first" optimization, and each has their own bespoke error-to-diagnostics translator (uninstallWaitDiagsFromError line 422, startedWaitDiagsFromError line 492).
  • Code Sample (the repeated shape, from jobstate/state_utils.go:50-66, structurally identical to the other three sites):

    func waitForJobState(ctx context.Context, client *clients.ElasticsearchScopedClient, data MLJobStateData, jobID, desiredState string) diag.Diagnostics {
        stateChecker := func(ctx context.Context) (bool, error) {
            currentState, diags := getJobState(ctx, client, data, jobID)
            if diags.HasError() {
                return false, diagutil.FwDiagsAsError(diags)
            }
            if currentState == nil {
                return false, errJobNotFound
            }
            return *currentState == desiredState, nil
        }
        err := asyncutils.WaitForStateTransition(ctx, "ml_job", jobID, stateChecker)
        return diagutil.FrameworkDiagFromError(err)
    }

Impact Analysis

  • Maintainability: Bug fixes to the polling contract (e.g. the "check immediately before the first poll interval" optimization present in WaitForMLJobClosed and waitForStarted but absent from WaitForDatafeedState/waitForJobState) have to be applied independently in up to 4 places, and already have drifted β€” two of the four sites lack the immediate-check optimization the other two have.
  • Organization: New resources that need "wait for async state" logic have no obvious single place to look; a developer adding a fifth resource is likely to write a fifth copy instead of discovering and reusing an existing one.
  • Duplication Risk: The diag.Diagnostics ↔ error boundary crossed in every implementation (diagutil.FwDiagsAsError in, diagutil.FrameworkDiagFromError or a bespoke translator out) is exactly the kind of boilerplate that silently diverges β€” e.g. startedWaitDiagsFromError downgrades a timeout to a warning while the other three sites always return a hard error.

Refactoring Recommendations

  1. Add a generic polling helper to internal/asyncutils

    • Target: internal/asyncutils/state_waiter.go (or a new poll_until.go in the same package)
    • Action: introduce something like func PollUntilState[T any](ctx context.Context, resourceType, resourceID string, fetch func(context.Context) (*T, diag.Diagnostics), isDesired func(T) bool, opts ...Option) (*T, diag.Diagnostics) that performs the fetch, the "check immediately" optimization, the StateChecker closure, the call to WaitForStateTransition, and the diagnostics translation in one place, leaving only the resource-specific fetch/isDesired callbacks to each caller.
    • Estimated effort: ~1 day (helper + migrating 4 call sites + unit tests already present for each site should keep passing with minimal changes).
    • Benefits: removes ~150 lines of duplicated closures/translators, gives the "immediate check" optimization to all 4 call sites uniformly, and centralizes the diagnostics-translation policy (with an option to opt into the "downgrade timeout to warning" behavior startedWaitDiagsFromError uses).
  2. Migrate the 4 existing call sites onto the new helper incrementally, starting with jobstate/datafeed (smallest, easiest to verify against existing unit tests) before tackling the larger security_entity_store/helpers.go.

Implementation Checklist

  • Review refactoring findings
  • Prioritize refactoring tasks
  • Create refactoring plan
  • Implement changes
  • Update tests as needed
  • Verify no functionality broken

Analysis Metadata

  • Analyzed Files: internal/elasticsearch/ml/datafeed/state_utils.go, internal/elasticsearch/ml/jobstate/state_utils.go, internal/clients/elasticsearch/ml_anomaly_job.go, internal/kibana/security_entity_store/helpers.go, internal/asyncutils/state_waiter.go
  • Total Functions Cataloged: 13 (across the 4 duplicate sites)
  • Function Clusters Identified: 1 (poll-until-state wrappers)
  • Outliers Found: 0
  • Duplicates Detected: 4 near-duplicate implementations of the same pattern
  • Detection Method: manual read of all asyncutils.WaitForStateTransition call sites (11 total found via search; 4 confirmed structurally identical) + line-by-line comparison
  • Analysis Date: 2026-08-21

Generated by Semantic Function Refactor Β· sonnet50 Β· 326 AIC Β· βŒ– 9.16 AIC Β· ⊞ 10.2K Β· β—·

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions