[BACKEND] Remove unnecessary async from R2Service.GeneratePresignedUploadUrl

[Andreawingardh]

Remove Unnecessary async Keyword from R2Service.GeneratePresignedUploadUrl

Issue Type

• Technical Debt
• Code Quality
• Bug
• Feature

Priority

🟡 Low - Works correctly but not optimal

---

Description

The GeneratePresignedUploadUrl method in R2Service is marked as async but doesn't use await anywhere, causing compiler warning CS1998. This creates unnecessary overhead from the async state machine when the method is actually synchronous.

Current signature:

public async Task<PresignedUrlResponseDto> GeneratePresignedUploadUrl(string fileName, int expiryMinutes)

Why it's async: The method signature requires Task<T>, but the implementation only calls _s3Client.GetPreSignedURL() which is synchronous.

Why GetPreSignedURL is synchronous: Generating a presigned URL doesn't make a network call to AWS/Cloudflare - it's just cryptographic signing and URL construction done locally. The actual upload happens when the client uses the URL.

---

Current Behavior

// In R2Service.cs
public async Task<PresignedUrlResponseDto> GeneratePresignedUploadUrl(string fileName, int expiryMinutes)
{
    var request = new GetPreSignedUrlRequest
    {
        BucketName = _bucketName,
        Key = fileName,
        Verb = HttpVerb.PUT,
        Expires = DateTime.UtcNow.AddMinutes(expiryMinutes)
    };
    
    var uploadUrl = _s3Client.GetPreSignedURL(request); // ⚠️ Synchronous method
    return new PresignedUrlResponseDto
    {
        UploadUrl = uploadUrl,
        PublicUrl = GetPublicUrl(fileName)
    };
}

Compiler Warning:

warning CS1998: This async method lacks 'await' operators and will run synchronously.

---

Expected Behavior

The method should be synchronous and return a completed Task:

public Task<PresignedUrlResponseDto> GeneratePresignedUploadUrl(string fileName, int expiryMinutes)
{
    var request = new GetPreSignedUrlRequest
    {
        BucketName = _bucketName,
        Key = fileName,
        Verb = HttpVerb.PUT,
        Expires = DateTime.UtcNow.AddMinutes(expiryMinutes)
    };
    
    var uploadUrl = _s3Client.GetPreSignedURL(request);
    return Task.FromResult(new PresignedUrlResponseDto
    {
        UploadUrl = uploadUrl,
        PublicUrl = GetPublicUrl(fileName)
    });
}

---

Implementation Steps

1. Update R2Service.cs (5 min)

• Remove async keyword from GeneratePresignedUploadUrl
• Wrap return value in Task.FromResult()
• Verify IR2Service interface matches

2. Update Interface (if needed) (2 min)

• Check IR2Service.cs - signature should remain Task<PresignedUrlResponseDto> (no change needed)

3. Update All Call Sites (15-30 min)

Find all places that call this method and remove await:

Before:

var result = await _r2Service.GeneratePresignedUploadUrl(fileName, 15);

After:

var result = _r2Service.GeneratePresignedUploadUrl(fileName, 15);

Search for: GeneratePresignedUploadUrl in the codebase
Likely locations:

• DesignController
• Any service that handles screenshot uploads

4. Testing (15 min)

• Run all existing tests
• Manual test: Generate presigned URLs in DesignController
• Verify screenshot upload flow still works
• Check that no deadlocks occur (should be fine since we're using Task.FromResult)

---

Files to Modify

Backend

• backend/Atelje/Services/R2Service.cs (method implementation)
• backend/Atelje/Services/IR2Service.cs (verify interface signature)
• Any controllers/services that call this method:
  • backend/Atelje/Controllers/DesignController.cs (likely candidate)
  • Search codebase for GeneratePresignedUploadUrl to find all call sites

---

Impact Assessment

Risk Level: 🟢 Low

• The method behavior doesn't change
• Existing await calls will still work (awaiting a completed Task is valid)
• No breaking changes to API contracts

Performance Impact: Minimal

• Removes unnecessary async state machine overhead
• Presigned URL generation is already fast (milliseconds)
• Real performance gain is negligible but code is more honest

Testing Requirements:

• Existing integration tests should pass
• No new test cases needed
• Just verify screenshot upload flow works

---

Alternative Approaches

Option A: Remove async, wrap in Task.FromResult (recommended)

Pros:

• Honest about synchronous nature
• Removes compiler warning
• No unnecessary async state machine overhead
• Follows .NET best practices

Cons:

• Requires updating call sites to remove await
• Moderate testing effort

Option B: Make fully synchronous (returns PresignedUrlResponseDto directly)

Pros:

• Most honest implementation
• No Task overhead at all
• Clearest code

Cons:

• Breaking change to interface
• If other methods in IR2Service are async, creates inconsistent interface
• Callers in async methods would need await Task.FromResult() or similar workarounds

---

Estimated Time

• Total: 30-45 minutes
• Complexity: Low
• Risk: Low

---

Success Criteria

• No compiler warning CS1998
• All tests pass
• Screenshot upload functionality works
• No deadlocks or threading issues
• Code review approved

---

Notes

• This issue was identified during final project review
• Not critical - code works correctly as-is
• Good learning opportunity about async/await patterns
• Consider this after project submission and when you have time for non-critical improvements

---

References

• Microsoft Docs: Async/Await Best Practices
• Why GetPreSignedURL is synchronous
• Project file: backend/Atelje/Services/R2Service.cs line 29

---

Labels

technical-debt code-quality low-priority async-await r2-service