Fix concurrency limiter and add sync service tests - #8
Closed
AlecKriebel wants to merge 2 commits into
Closed
Conversation
This commit implements the scheduled sync functionality for CalendarSync: Database Schema: - Add syncConfigs table to store sync configuration (calendar, destination, schedule) - Add syncHistory table to track sync run results - Add oauthCredentials table to store Google OAuth tokens - Add enums for sync status, frequency, destination type, and run status Worker Cron Implementation: - Cron runs every 15 minutes to process due syncs - Queries syncConfigs where next_sync_at <= now and status is active - Processes syncs in parallel with concurrency limit (5 at a time) - Limits to 50 syncs per cron run to prevent timeout Google Calendar Integration: - Full sync on first run (fetch events from now onwards) - Incremental sync using syncToken for subsequent runs - Automatic token refresh when expired - Handles 410 Gone response (expired syncToken) with automatic full sync Destination Adapters: - Google Sheets: Creates/updates rows in spreadsheet with event data - Notion: Creates/updates pages in database with event properties - Both support incremental updates (track existing events by ID) Error Handling: - Each sync processes independently (one failure doesn't stop others) - Failed syncs are logged and marked with error status - Next sync is still scheduled even on failure (will retry) - Sync history records all run details including errors Testing: - Unit tests for Google Calendar client - Unit tests for event transformation - Integration tests for cron handler with mocked sync service Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fixed bug in processWithConcurrency function where promises were incorrectly identified as pending/resolved (used Set instead of array with self-removal pattern) - Added comprehensive test suite for SyncService covering: - getDueSyncs() query functionality - processSync() success path with event push - Event deletion handling - OAuth credentials validation - Destination push error handling - Calendar API error handling - Next sync scheduling - Fixed type error in google-calendar test (fetch mock) Task: #41189058 - Implement Cloudflare Worker cron for scheduled syncs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Author
Review: Cannot Merge - Architecture ConflictThis PR cannot be merged due to fundamental conflicts with the evolved dev branch. Since this PR was created, dev has merged multiple PRs (#3-#6) that implemented the sync functionality with a different architecture: Current Dev Architecture:
This PR Architecture:
Merge Conflicts:
Recommendation: Note: The concurrency limiter bug fix in this PR (commit 1705d68) is well-designed but doesn't apply to dev, which uses sequential processing instead of parallel processing. Code Quality: The implementation itself is high quality with 25 passing tests and good architecture. However, it's been superseded by the evolution of the dev branch. |
Contributor
Author
|
Closing this PR as the functionality already exists on dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
processWithConcurrencyfunction in the cron handler where the logic for detecting completed promises was incorrect (Promise.resolve always wins the race, causing all promises to be marked as "pending")SyncServicewith 8 new testsChanges
packages/worker/src/cron/index.ts- Fixed concurrency limiter implementationpackages/worker/src/__tests__/sync-service.test.ts- New test file for SyncServicepackages/worker/src/__tests__/google-calendar.test.ts- Fixed type casting for fetch mockTest plan
Task Reference
Task ID: #41189058 - Implement Cloudflare Worker cron for scheduled syncs
🤖 Generated with Claude Code