Backend refactor #5
Merged
Conversation
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
Because it was redundant.
Added 5 new tests and improved functionality a bit.
It seems cleaner this way.
- Added a func comment - Removed unused arguments in imap package - Fixed grammar in ROADMAP
Was duplicated 5x
- Handle multiple whitespace chars - Cover edge-cases around whitespaces with tests
These types had no names before, but they were used at a few places, nicer this way.
Extracted it to a WriteJSONResponse helper
- Add worker pool (1-3 connections) and listener connections per user - Implement thread safety with per-connection mutexes - Add connection lifecycle: idle timeout, health checks, cleanup - Fix race conditions with double-check locking - Update docs
Naming is hard.
- Add concurrent access and edge case tests for pool - Add tests for fetch.go and processIncrementalMessage - Fix unchecked error in FetchFullMessage - Export GetListenerConnection with ListenerClient interface - Fix Close() deadlock by using TryLock() for graceful shutdown - Remove all FIXME-TEST comments
- Use errors.Is() with sentinel error for invalid search queries - Extract duplicated pagination limit logic to helpers.go
- Refactor the API to expose release explicitly and remove the artificial minimum hold time of 5 sec. → more performant pool with the same number of workers - Share a single *imap.Pool between FoldersHandler and Service. → less workers needed - Make maxWorkers configurable. → Their number can be high in tests, conservative in prod - Clean up dbPool vs. imapPool naming.
For more safety to get worker slots released.
- Add VMAIL_ENV=test to avoid "Warning: .env file not found, using environment variables" logs. - Search: Treat client cancellations as non-errors to avoid "SearchHandler: Failed to search: failed to get IMAP client: failed to get user settings: failed to get user settings: context canceled" logs. - Require THREAD support from prod IMAP servers, so we don't need this error message: "THREAD command not supported, falling back to SEARCH: THREAD command returned error: Unknown command".
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.
Too many refactors to count. Now I feel confident about the back end.