Skip to content

refactor: dry - extract file ID resolution helper in Drive package #9

@aliwatters

Description

@aliwatters

Summary

Every Drive testable function repeats a 6-line block to resolve the service, extract file_id, validate it's non-empty, and call ExtractGoogleResourceID. This block appears 8 times. Additionally, TestableDriveDownload is 99 lines with an inline readLimited closure and complex MIME type branching that should be decomposed.

Location

  • File: internal/drive/drive_tools_testable.go
  • Repeated block: L57-66, L78-87, L334-343, L378-387, L419-428, L448-457, L475-484, L522-531 (8 instances)
  • Long function: TestableDriveDownload L77-175 (99 lines)
  • Inline closure: readLimited L102-112
  • Google Workspace MIME mapping: L118-151 (inline magic strings)

Category

Type: DRY Violation / Long Function

Severity: High

Evidence

Repeated 6-line block (8x):

srv, errResult, ok := ResolveDriveServiceOrError(ctx, request, deps)
if !ok { return errResult, nil }
fileID := common.ParseStringArg(request.Params.Arguments, "file_id", "")
if fileID == "" { return mcp.NewToolResultError("file_id parameter is required"), nil }
fileID = common.ExtractGoogleResourceID(fileID)

Magic MIME strings (inline):

"application/vnd.google-apps.document"
"application/vnd.google-apps.spreadsheet"
"application/vnd.google-apps.presentation"
"application/vnd.google-apps.folder"

Suggested Refactoring

  1. Extract resolveFileID(ctx, request, deps) (DriveService, string, *mcp.CallToolResult, bool) helper
  2. Extract readLimited as a package-level or common/ function
  3. Extract Google Workspace MIME-to-export mapping as a constant map
  4. Split TestableDriveDownload into separate export and download paths

Effort Estimate

  • Size: Medium (1-4 hours)
  • Risk: Low (internal refactoring only)
  • Tests Required: Yes (existing tests must pass)

Acceptance Criteria

  • resolveFileID helper eliminates 8 repeated blocks (~40 lines saved)
  • readLimited extracted to reusable function
  • MIME type mapping extracted to named constant
  • TestableDriveDownload decomposed to <50 lines
  • All existing tests pass

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions