git.iko.soy/parseable-android
on 4ea48de help sign in
parseable-android/IMPROVEMENTS.md
176 lines 11 kB history raw
1# Parseable Android — Improvements
3Summary of all improvements made to the codebase, organized by category.
5---
7## Security
9### SQL Injection Prevention
10- **LogViewerViewModel**: All user-supplied column names are escaped with `escapeIdentifier()` (double-quote escaping) and filter values use single-quote escaping (`'` → `''`) to prevent SQL injection in generated WHERE clauses.
11- **Custom SQL Queries**: Only `SELECT` statements are allowed; `DELETE`, `DROP`, etc. are rejected. Queries without a `LIMIT` clause automatically get `LIMIT 5000` appended to prevent OOM from unbounded result sets.
12- **ParseableRepository**: Stream names in generated SQL are escaped with `escapeIdentifier()`.
14### Credential Storage
15- **SettingsRepository**: Passwords are stored in `EncryptedSharedPreferences` (AES-256-GCM) backed by Android Keystore, separate from other preferences in DataStore.
16- **Corruption Recovery**: If `EncryptedSharedPreferences` becomes corrupted (common after backup/restore), the file is deleted and recreated automatically with a Timber log.
17- **Mutex Protection**: A `configMutex` guards concurrent reads/writes across DataStore and EncryptedSharedPreferences.
19### Network Security
20- **network_security_config.xml**: Cleartext HTTP is explicitly allowed (required for self-hosted Parseable instances without TLS) with a proper XML config rather than a blanket manifest flag.
21- **Private IP Detection**: LoginScreen warns users when enabling HTTP for non-private IP addresses (checks `localhost`, `127.*`, `10.*`, `192.168.*`, `172.16-31.*`).
22- **Clipboard Safety**: Log export catches `SecurityException` when accessing the clipboard on Android 13+ where background clipboard access may be restricted.
24### Input Validation
25- **URL Validation**: Server URLs are validated with `Patterns.WEB_URL`, trimmed of whitespace, and checked for blank hosts before connection attempts.
26- **Filter Operators**: Only operators from an `ALLOWED_OPERATORS` allowlist are accepted for filter clauses.
28---
30## Concurrency & Reliability
32### CancellationException Handling
33- **All ViewModels** (`SettingsViewModel`, `StreamsViewModel`, `StreamInfoViewModel`, `LogViewerViewModel`): `async/await` blocks are wrapped in `try-catch` with explicit `CancellationException` handling. On cancellation (e.g., user navigates away), `isLoading` is reset to `false` before re-throwing, preventing the UI from getting stuck in a permanent loading state.
35### Structured Concurrency
36- **LogViewerViewModel**: Schema loading uses a dedicated `schemaJob` that is cancelled on re-initialization or `onCleared()`. Search debounce uses a separate `searchJob`. Streaming uses `streamingJob` with a generation counter to prevent stale poll results.
37- **StreamsViewModel**: Stats loading uses a `statsJob` that is cancelled on refresh. A `Semaphore(8)` limits concurrent stats requests to 8 at a time.
38- **StreamInfoViewModel**: A `loadJob` is cancelled when `load()` is called again, preventing duplicate parallel loads.
40### Thread Safety
41- **LogViewerViewModel**: `streamingGeneration`, `lastSeenTimestamp`, and `consecutiveStreamingErrors` are all `@Volatile` to ensure visibility across coroutines.
42- **ParseableApiClient**: The `config` field is `@Volatile` and `buildRequest()` captures a local snapshot to avoid reading a partially-updated config during concurrent requests.
44### Resource Cleanup
45- **ParseableApiClient**: HTTP responses use `.use { }` blocks to ensure the response body is always closed, even on exceptions. A `shutdown()` method evicts the connection pool and shuts down the dispatcher.
46- **ViewModels**: All `onCleared()` implementations cancel active jobs (`schemaJob`, `searchJob`, `streamingJob`).
47- **Streaming**: Consecutive errors during streaming trigger an exponential backoff (1s → 2s → 4s) and auto-stop after 10 consecutive failures with an error message.
49### Logout Race Condition
50- **StreamsViewModel**: `logout()` calls `clearConfig()` and `clearCredentials()` sequentially to prevent auth errors from triggering a second logout navigation.
51- **ParseableRepository**: Auth errors (401) are detected via `checkAuth()` which clears credentials and sends to `authErrors` channel (conflated to prevent duplicate navigations).
53---
55## Error Handling & UX
57### Error Messages
58- **ParseableApiClient**: Specific exception types produce targeted messages:
59 - `SocketTimeoutException` → "Connection timed out"
60 - `UnknownHostException` → "Unable to resolve host"
61 - `ConnectException` → "Connection refused. Is the server running?"
62 - `SSLException` → "SSL error: ... Check server certificate."
63 - Generic `IOException` → the exception message or "Network error"
64- **SettingsScreen**: Error card is now visible even during a reload (removed the `!isLoading` gate that hid errors during pull-to-refresh).
66### Empty States
67- **LogViewerScreen**: Context-aware empty messages:
68 - During streaming: "Waiting for new logs…"
69 - With search + filters: "No logs match \"query\" with the active filters"
70 - With search only: "No logs match \"query\""
71 - With filters only: "No logs match the active filters"
72 - Default: "No logs found for the selected time range"
73- **Result Cap Warning**: When results hit the 5,000 limit, shows "Showing max 5,000 results. Refine your query to see more." in primary color instead of misleading "End of results".
75### Streaming Error Persistence
76- **LogViewerViewModel**: `stopStreaming()` now preserves the streaming error in state (only clears `isStreaming`). A separate `dismissStreamingError()` method lets users explicitly dismiss the error banner. Errors are also cleared on `refresh()`.
78### Schema Failure Visibility
79- **StreamInfoViewModel**: A `schemaFailed` boolean flag tracks whether the schema request specifically failed (separate from the combined error string).
80- **StreamInfoScreen**: Shows a "Schema unavailable" card when schema loading fails, instead of silently hiding the entire section.
82### Alert Deletion Safety
83- **AlertsScreen**: Alerts without an `id` field have their delete button disabled at the card level, preventing them from reaching the delete confirmation dialog.
85---
87## Accessibility
89### Touch Targets
90- All interactive elements meet the 48dp minimum touch target size:
91 - **StreamsScreen**: Stats retry `TextButton` uses `defaultMinSize(minHeight = 48.dp)` instead of a fixed 24dp height.
92 - **LogViewerScreen**: Filter chips, time range buttons, and all `IconButton` elements meet the 48dp minimum.
93 - **LoginScreen**: Both login buttons use `defaultMinSize(minHeight = 50.dp)` instead of fixed `.height(50.dp)`, allowing growth for landscape or accessibility text scaling.
95### Content Descriptions
96- **StreamsScreen**: The decorative `ChevronRight` icon uses `contentDescription = null` since the parent `Card` already handles the click action (avoids duplicate screen reader announcements).
97- All other icons have meaningful `contentDescription` values that describe the action, not the icon.
99### Keyboard / IME
100- **LogViewerScreen**: Pressing the keyboard Search button (`ImeAction.Search`) now triggers `viewModel.refresh()` in addition to hiding the keyboard.
101- **Filter Bottom Sheet**: The filter value field has `ImeAction.Done` with a `keyboardActions` handler that applies the filter directly from the keyboard.
102- **LoginScreen**: IME actions chain correctly: URL → Next → Username → Next → Password → Done (triggers login).
104### Configuration Changes
105- **LoginScreen**: `passwordVisible` uses `rememberSaveable` instead of `remember`, so the "show password" toggle survives device rotation.
107---
109## Performance
111### List Operations
112- **LogViewerViewModel**: `removeFilter()` uses `filterIndexed` instead of `toMutableList().apply { removeAt() }`, avoiding temporary mutable list allocations.
113- **LazyColumn Keys**: All `LazyColumn` lists use deterministic, stable keys (e.g., `alert.id ?: alert.name ?: "alert_$index"`) to prevent unnecessary recompositions.
115### Streaming Backpressure
116- **LogViewerViewModel**: Streaming caps at `STREAMING_MAX_LOGS = 1000` entries, dropping oldest when the cap is reached. A generation counter prevents stale poll results from being applied.
118### Caching
119- **ParseableRepository**: TTL-based caching with `ConcurrentHashMap`:
120 - Streams: 30s
121 - Schema: 2min
122 - Stats: 30s
123 - About: 5min
124- Force-refresh bypasses the cache when explicitly requested (e.g., pull-to-refresh).
126### R8 Full Mode
127- **build.gradle.kts**: `isMinifyEnabled = true` with R8 full mode enabled for release builds.
128- **proguard-rules.pro**: Comprehensive keep rules for kotlinx.serialization, Room entities, ViewModel state classes, OkHttp, EncryptedSharedPreferences, Google Tink, and Protobuf.
130---
132## Testing
134### Unit Test Coverage
135Every ViewModel has a dedicated test class:
137| Test Class | Tests | Covers |
138|---|---|---|
139| `LogViewerViewModelTest` | 17 | Initialize, search, filters, time ranges, streaming, custom SQL LIMIT, SQL escaping, IS NULL, error states |
140| `StreamsViewModelTest` | 8 | Refresh, error handling, stats loading, favorites CRUD, exception resilience |
141| `SettingsViewModelTest` | 8 | Config loading, about info, user parsing, error consolidation, exception resilience, refresh |
142| `StreamInfoViewModelTest` | 9 | Parallel loading, partial errors, schemaFailed flag, delete, cancel, idempotency |
143| `AlertsViewModelTest` | 7 | Refresh, error, delete flow, cancel delete |
144| `LoginViewModelTest` | 9+ | Validation, URL normalization, login flow, saved credentials |
145| `ParseableRepositoryTest` | 9+ | Caching, TTL, auth error detection, cache invalidation |
147### CI
148- **ci.yml**: Runs `testDebugUnitTest` and `lintDebug` on PRs and pushes to main/master.
149- **build-release.yml**: Builds both debug and release APKs on `v*` tags and creates a draft GitHub release.
151---
153## Architecture & Code Quality
155### Error Handler
156- **ErrorHandler**: A `CompositionLocal`-based error handler (`LocalErrorHandler`) provides a centralized `SnackbarHostState` for showing error messages from any screen. Used for session-expired notifications on the login screen.
158### Navigation
159- **NavGraph**: All authenticated routes (`streams`, `log_viewer`, `stream_info`, `alerts`, `settings`) have auth guards that redirect to login if the API client isn't configured.
160- **URL Encoding**: Stream names are URL-encoded/decoded in navigation routes to handle special characters.
161- **Deep Links**: Supported for `parseable://streams`, `parseable://stream/{streamName}`, and `parseable://alerts`.
163### Favorites (Room)
164- **FavoriteStream**: Room entity with a DAO for persisting favorite streams locally.
165- **ParseableDatabase**: Room database with `fallbackToDestructiveMigration()` and schema export directory.
166- **StreamsScreen**: Favorite streams are sorted to the top of the list. Toggle via star icon with snackbar feedback.
168### Dependency Management
169- **libs.versions.toml**: All dependency versions are centralized in a Gradle version catalog.
170- **Timber**: Debug logging via `Timber.DebugTree()` planted only in debug builds. Used throughout the API client and repository layer for error logging.
172---
174## Files Changed
17641 files modified across the codebase, with approximately **2,981 lines added** and **664 lines removed**.