Skip to content

refactor: reduce complexity of execute_impl in upload_build_attachment.rs - #2094

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-upload-build-attachment-execute-impl-2-f7c86782edc19d33
Draft

refactor: reduce complexity of execute_impl in upload_build_attachment.rs#2094
github-actions[bot] wants to merge 1 commit into
mainfrom
refactor/reduce-complexity-upload-build-attachment-execute-impl-2-f7c86782edc19d33

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

What was complex

UploadBuildAttachmentResult::execute_impl() in src/safe_outputs/upload_build_attachment.rs was flagged by Clippy's too_many_lines lint at 267/100 lines. It combined build-ID reconciliation, artifact-name-prefix/validation, allow-list checks, extension checks, attachment-type resolution, staged-file path/integrity validation, SHA-256 verification, ADO API coordinate resolution, and the HTTP PUT + response handling all inline in one function.

What changed

Split the function into 9 focused, independently-testable helpers (following the existing Result<T, ExecutionResult> early-return pattern already used elsewhere in this module, e.g. update_github_issue.rs):

  • resolve_effective_build_id — reconciles agent-supplied build_id with the current run
  • resolve_final_artifact_name — applies name-prefix, re-validates charset, checks the artifact-name allow-list
  • validate_file_extension — checks the file extension against allowed-extensions
  • resolve_attachment_type — resolves and validates the {type} URL segment
  • resolve_staged_file — canonicalizes/validates the staged file path, size integrity, and size limit
  • read_and_verify_staged_bytes — reads file bytes and verifies the SHA-256 hash
  • resolve_timeline_coords (+ new TimelineAttachmentCoords struct) — resolves ADO org/project/token/plan/timeline/record IDs
  • upload_timeline_attachment — builds the URL, performs the PUT, and translates the HTTP response

execute_impl() is now a short orchestration function (~50 lines) that delegates to these helpers.

Verification

  • No public API signatures changed; no observable behaviour changed.
  • All existing comments preserved.
  • cargo test — full suite: 3322 passed, 0 failed (including all 39 tests in upload_build_attachment::tests).
  • cargo clippy --all-targets --all-features — clean.
  • cargo clippy -- -W clippy::too_many_lines — no longer flags this file (previously 267/100).

Before/after complexity

Before After
execute_impl line count (Clippy too_many_lines) 267/100 below threshold

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 · auto · 102.8 AIC · ⌖ 13.2 AIC · ⊞ 11.4K ·

…t.rs

Split the 267/100 too_many_lines execute_impl() into 9 focused helper
functions: resolve_effective_build_id, resolve_final_artifact_name,
validate_file_extension, resolve_attachment_type, resolve_staged_file,
read_and_verify_staged_bytes, resolve_timeline_coords, and
upload_timeline_attachment (plus a TimelineAttachmentCoords struct to
group the ADO API coordinates). execute_impl() is now a short
orchestration function that delegates to these helpers via early
returns on Result<T, ExecutionResult>.

No behavior change. Full suite (3322 tests) + clippy pass. clippy
--all-targets --all-features -W clippy::too_many_lines no longer
flags this file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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.

0 participants