Skip to content

refactor: reduce complexity of execute_impl in upload_pipeline_artifact.rs#1625

Merged
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-upload-pipeline-artifact-9f0d3e01d5c9fce3
Jul 22, 2026
Merged

refactor: reduce complexity of execute_impl in upload_pipeline_artifact.rs#1625
jamesadevine merged 1 commit into
mainfrom
refactor/reduce-complexity-upload-pipeline-artifact-9f0d3e01d5c9fce3

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

What was complex

execute_impl in src/safe_outputs/upload_pipeline_artifact.rs was a single 383-line function with 33 branch points spanning six distinct phases:

  1. Resolve effective build ID
  2. Validate config allowlists and build final artifact name
  3. Resolve and integrity-check the staged file
  4. Extract required ADO context fields
  5. Upload file to the build container (HTTP step 1)
  6. Associate the artifact record with the target build (HTTP step 2)

Each phase had multiple early-return guard clauses nested inside the single function, making it hard to understand the overall flow and difficult to test individual phases in isolation.

What changed

Six focused helper functions were extracted:

Helper Responsibility
resolve_effective_build_id Resolve build ID from request param or BUILD_BUILDID
resolve_final_artifact_name All allowlist/name-prefix/extension checks → final name or ExecutionResult::failure
resolve_staged_file Canonicalize, bounds-check, size/dir guards → StagedFileInfo or failure
resolve_ado_context Extract org_url/project/project_id/token/container_id into ArtifactAdoContext
upload_file_to_container HTTP PUT to the build container (Step 1)
associate_artifact_with_build HTTP POST to associate the artifact record (Step 2)

The remaining execute_impl is now 85 lines with a clear sequential narrative: resolve IDs → validate names → validate staged file → dry-run check → verify hash → resolve ADO context → dedupe check → upload → associate → record dedupe key.

Two data-carrying structs (ArtifactAdoContext, StagedFileInfo) were introduced to avoid passing many individual parameters between the helpers.

Verification

  • All tests pass (cargo test)
  • cargo clippy --all-targets --all-features is clean
  • Behaviour is unchanged: same validation messages, same HTTP flow, same dedupe logic

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • spsprodeus21.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · 102.4 AIC · ⌖ 14 AIC · ⊞ 7.2K ·

…ct.rs

Extract six focused helpers from the 383-line execute_impl:
- resolve_effective_build_id: resolve build ID from request or BUILD_BUILDID
- resolve_final_artifact_name: all allowlist/name-prefix/extension checks
- resolve_staged_file: canonicalize, bounds-check, size/dir checks
- resolve_ado_context: extract org_url/project/project_id/token/container_id
- upload_file_to_container: Step 1 HTTP PUT to the build container
- associate_artifact_with_build: Step 2 HTTP POST to associate artifact record

The remaining execute_impl is now 85 lines with a clear sequential narrative:
resolve IDs → validate names → validate staged file → dry-run → verify hash
→ resolve context → dedupe check → upload → associate → record dedupe key.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine
jamesadevine marked this pull request as ready for review July 22, 2026 20:54
@jamesadevine
jamesadevine merged commit 375a202 into main Jul 22, 2026
2 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant