Skip to content

Conversation

rohilsurana
Copy link
Member

@rohilsurana rohilsurana commented Oct 3, 2025

Resource APIs Migration to Connect RPC

This PR migrates all Resource management APIs from gRPC to Connect RPC framework, following the established patterns from previous API migrations.

Migration Overview

Converting Resource APIs from traditional gRPC handlers to buf Connect RPC handlers with proper error handling, request/response wrapping, and comprehensive test coverage.

APIs Migration Progress

✅ Completed APIs (6/6) - COMPLETE

API Status Commit Implementation Tests
ListResources ✅ Complete 2cc8b7c internal/api/v1beta1connect/resource.go:18-41 3 test cases
ListProjectResources ✅ Complete ebc4f77 internal/api/v1beta1connect/resource.go:43-64 4 test cases
CreateProjectResource ✅ Complete 232c72d internal/api/v1beta1connect/resource.go:66-125 7 test cases
GetProjectResource ✅ Complete 8ed4fba internal/api/v1beta1connect/resource.go:127-148 6 test cases
UpdateProjectResource ✅ Complete 9727cb7 internal/api/v1beta1connect/resource.go:150-210 9 test cases
DeleteProjectResource ✅ Complete b5369f6 internal/api/v1beta1connect/resource.go:212-240 8 test cases

🎉 All Resource APIs have been successfully migrated to Connect RPC!

Technical Changes

API Implementation Updates

  • Updated function signatures to use Connect RPC types:
    • Request: *connect.Request[frontierv1beta1.RequestType]
    • Response: (*connect.Response[frontierv1beta1.ResponseType], error)
  • Changed request field access from request.GetField() to request.Msg.GetField()
  • Updated response wrapping with connect.NewResponse()
  • Converted error handling to Connect error codes

Test Migration

  • Migrated test suites to use Connect RPC patterns
  • Enhanced test coverage with comprehensive error and success scenarios
  • Updated mock service expectations to match Connect RPC patterns

Error Handling

  • Standardized error responses using Connect error codes:
    • Internal errors: connect.NewError(connect.CodeInternal, ErrInternalServerError)
    • Bad requests: connect.NewError(connect.CodeInvalidArgument, ErrBadRequest)
    • Conflicts: connect.NewError(connect.CodeAlreadyExists, ErrConflictRequest)
    • Authentication: connect.NewError(connect.CodeUnauthenticated, ErrUnauthenticated)
    • Not found: connect.NewError(connect.CodeNotFound, ErrResourceNotFound)
  • Maintained consistent error messages across the API
  • Added specific ErrResourceNotFound constant for better error specificity

Key Implementation Details

ListResources

  • Basic Listing: Lists resources with optional namespace and project filtering
  • Namespace Parsing: Uses schema.ParseNamespaceAliasIfRequired() for namespace handling
  • Filter Application: Builds resource.Filter with namespace and project ID
  • Resource Transformation: Converts each resource using transformResourceToPB()
  • Error Handling: Maps service errors and transformation errors to InternalServerError
  • Test Coverage: 3 comprehensive test cases (service error, transformation error, success)

DeleteProjectResource

  • Validation Pattern: Fetches resource first using Get() to validate existence and permissions
  • Error Mapping: Comprehensive error handling for multiple service layer errors:
    • resource.ErrNotExist, resource.ErrInvalidUUID, resource.ErrInvalidID → NotFound
    • Other errors → InternalServerError
  • Project Validation: Fetches parent project details for audit logging
  • Business Logic: Calls resourceService.Delete() with namespace and resource ID
  • Audit Logging: Preserves ResourceDeletedEvent audit trail with target info
  • Response: Returns empty Connect response on successful deletion
  • Test Coverage: 8 comprehensive test cases covering all error scenarios and success paths

UpdateProjectResource

  • Request Validation: Checks for nil request body
  • Metadata Processing: Converts protobuf Struct to metadata.Metadata
  • Project Validation: Fetches parent project details for audit logging
  • Principal Parsing: Supports both direct user ID and namespace:resourceID format
  • Comprehensive Error Mapping: Maps all service layer errors to appropriate Connect codes:
    • resource.ErrNotExist, resource.ErrInvalidUUID, resource.ErrInvalidID → NotFound
    • resource.ErrInvalidDetail, resource.ErrInvalidURN → BadRequest
    • resource.ErrConflict → AlreadyExists (conflict)
  • Audit Logging: Preserves ResourceUpdatedEvent audit trail
  • Test Coverage: 9 comprehensive test cases covering all error scenarios and success paths

GetProjectResource

  • Simple Resource Retrieval: Fetches resource by ID using resourceService.Get()
  • Error Mapping: Maps multiple service layer errors to Connect NotFound:
    • resource.ErrNotExist → Not Found
    • resource.ErrInvalidUUID → Not Found
    • resource.ErrInvalidID → Not Found
  • Response Transformation: Uses transformResourceToPB() for consistent response format
  • Test Coverage: 6 comprehensive test cases covering all error scenarios and success path

CreateProjectResource

  • Validation: Checks for nil request body
  • Metadata Processing: Converts protobuf Struct to metadata.Metadata
  • Project Validation: Fetches parent project details
  • Principal Parsing: Supports both direct user ID and namespace:resourceID format
  • Error Mapping: Maps service layer errors to appropriate Connect codes
  • Audit Logging: Preserves ResourceCreatedEvent audit trail
  • Business Logic: Maintains all original gRPC functionality

Verification

Each migrated API goes through:

  • ✅ Build verification (make build)
  • ✅ Test execution (make test)
  • ✅ Lint checking (make lint)
  • ✅ Functionality validation

Final test coverage: 54.2% for v1beta1connect package (37 total test cases)

Files Modified

Implementation Files

  • internal/api/v1beta1connect/resource.go - Connect RPC resource handlers
  • internal/api/v1beta1connect/resource_test.go - Connect RPC resource tests

Supporting Files

  • Error constants defined in internal/api/v1beta1connect/errors.go - Added ErrResourceNotFound
  • Schema constants in internal/api/v1beta1connect/metaschema.go

Migration Methodology

Following established systematic approach:

  1. Analysis - Study original gRPC implementation
  2. API Migration - Convert handler to Connect RPC format
  3. Test Migration - Migrate and enhance test coverage
  4. Verification - Build, test, and lint validation
  5. Integration - Commit and push changes

Summary

Migration Complete: All 6 Resource APIs successfully migrated to Connect RPC
Test Coverage: 37 comprehensive test cases covering all scenarios
Error Handling: Consistent Connect RPC error patterns
Audit Logging: Preserved for create/update/delete operations
Business Logic: All original functionality maintained
Quality: Passes build, test, and lint verification

This completes the Resource APIs migration milestone. All Resource management endpoints now use the modern Connect RPC framework with improved type safety, better error handling, and comprehensive test coverage.

Copy link

vercel bot commented Oct 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
frontier Ready Ready Preview Comment Oct 6, 2025 8:09am

@coveralls
Copy link

coveralls commented Oct 3, 2025

Pull Request Test Coverage Report for Build 18274242710

Details

  • 143 of 157 (91.08%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 36.111%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/v1beta1connect/resource.go 143 157 91.08%
Totals Coverage Status
Change from base Build 18274118895: 0.2%
Covered Lines: 13791
Relevant Lines: 38191

💛 - Coveralls

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates all 6 Resource management APIs from traditional gRPC to the modern Connect RPC framework, following established migration patterns from previous API conversions. The migration includes comprehensive test coverage with 37 test cases and maintains all original functionality including error handling, audit logging, and business logic.

  • Converts all Resource API handlers to use Connect RPC request/response patterns with proper error mapping
  • Migrates comprehensive test suites with enhanced coverage for all error and success scenarios
  • Adds specific ErrResourceNotFound error constant for better error specificity

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/api/v1beta1connect/resource.go Implements 5 new Connect RPC handlers (ListProjectResources, CreateProjectResource, GetProjectResource, UpdateProjectResource, DeleteProjectResource)
internal/api/v1beta1connect/resource_test.go Adds comprehensive test suites for all new Connect RPC handlers with 31 new test cases
internal/api/v1beta1connect/errors.go Adds ErrResourceNotFound constant for consistent error handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rohilsurana rohilsurana force-pushed the feature/migrate-frontier-rpc-resource branch from 758c569 to 3e6cd09 Compare October 6, 2025 08:07
@whoAbhishekSah whoAbhishekSah merged commit b7b576b into main Oct 6, 2025
7 checks passed
@whoAbhishekSah whoAbhishekSah deleted the feature/migrate-frontier-rpc-resource branch October 6, 2025 08:18
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.

3 participants