Skip to content

Refactor SecureStorage to support observation and async streams #5

Merged
stalkermv merged 4 commits into
mainfrom
storage-updates
Oct 13, 2025
Merged

Refactor SecureStorage to support observation and async streams #5
stalkermv merged 4 commits into
mainfrom
storage-updates

Conversation

@stalkermv
Copy link
Copy Markdown
Owner

Introduces StorageObservableValue and StorageObservationsRegistrar for observable storage values.
Refactors SecureStorage and SecureStorageService to use async streams for value observation, removing legacy subscription logic.

Introduces StorageObservableValue and StorageObservationsRegistrar for observable storage values. Refactors SecureStorage and SecureStorageService to use async streams for value observation, removing legacy subscription logic. Adds DiskNonSecureStorage for preview/debug use, moves storage implementations to Service subfolder, and defines SecureStorageError.
Enhanced Optional.unwrapped to include file and line info in errors and made OptionalUnwrapError more informative. Deprecated wrappedValue in favor of unwrapped(). Added Codable conformance to HexColorContainer.
@stalkermv stalkermv requested a review from Copilot October 13, 2025 19:18
@stalkermv stalkermv self-assigned this Oct 13, 2025
Copy link
Copy Markdown

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 refactors the SecureStorage system to support async streams and observable values for storage monitoring. It introduces a new observation system that replaces the legacy subscription-based approach with async stream patterns.

Key changes:

  • Introduces StorageObservableValue and StorageObservationsRegistrar for managing observable storage values
  • Refactors SecureStorageService protocol to use async streams instead of manual subscriptions
  • Removes legacy subscription logic from storage implementations

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Sources/SwiftStorage/SecureStorage/Service/SecureStorageService.swift Moved and updated protocol with async stream support
Sources/SwiftStorage/SecureStorage/Service/KeychainSecureStorage.swift Removed subscription logic and cleaned up implementation
Sources/SwiftStorage/SecureStorage/Service/InMemorySecureStorage.swift Removed subscription-related code
Sources/SwiftStorage/SecureStorage/Service/DiskNonSecureStorage.swift Added new debug-only storage implementation with async stream support
Sources/SwiftStorage/SecureStorage/SecureStorageService.swift Removed old protocol definition
Sources/SwiftStorage/SecureStorage/SecureStorageError.swift Extracted error types to separate file
Sources/SwiftStorage/SecureStorage/SecureStorage.swift Refactored to use new observable value system
Sources/SwiftStorage/SecureStorage/Observation/StorageObservationsRegistrar.swift Added registrar for managing storage observations
Sources/SwiftStorage/SecureStorage/Observation/StorageObservableValue.swift Added observable value wrapper for storage
Sources/SwiftHelpers/HexColorContainer.swift Added Codable conformance
Sources/FoundationExtensions/OptionalProtocol.swift Updated error handling and deprecated property
Sources/FoundationExtensions/Optional+Unwrapped.swift Enhanced unwrapped method with file/line tracking

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

Comment thread Sources/SwiftStorage/SecureStorage/Service/SecureStorageService.swift Outdated
Comment thread Sources/SwiftStorage/SecureStorage/Service/DiskNonSecureStorage.swift Outdated
Comment thread Sources/SwiftStorage/SecureStorage/Observation/StorageObservableValue.swift Outdated
Comment thread Sources/FoundationExtensions/OptionalProtocol.swift
cursor[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Comment on lines 47 to 49
}

keychain.addOrUpdate(key: key, data: data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Async observation never emits after external writes

The new async stream returned by SecureStorageService.observe only yields the current value once and then drops the continuation, and KeychainSecureStorage.set simply writes to the keychain without ever notifying observers. Any SecureStorage wrappers that subscribed earlier will therefore never receive updates when other code calls KeychainSecureStorage.set for the same key, regressing the previous subscription-based behaviour and leaving UIs stale. Retain the continuation and yield when a value is written (or otherwise tie set into the stream) so that observers are kept in sync.

Useful? React with 👍 / 👎.

Introduces observation streams to SecureStorage services, allowing clients to observe value changes for specific keys. Updates protocols and implementations for in-memory, keychain, and disk storage to support AsyncThrowingStream-based observation. Adds error handling for missing document directories and new tests for observation functionality.
Expanded the README with detailed module descriptions, usage examples, and installation instructions for SwiftHelpers. Fixed a variable assignment in SecureStorage.swift to correctly assign the provided store to the storage property.
.sink { _ in
if let value: T = try? self.value(type: T.self, forKey: key) {
continuation.yield(value)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Error Handling in Storage Observers

When a storage change notification is received, the observe methods use try? to fetch the updated value. If decoding fails, the error is silently ignored, preventing observers from receiving the new value and potentially causing UI inconsistencies.

Additional Locations (2)

Fix in Cursor Fix in Web

@stalkermv stalkermv merged commit 66b34d5 into main Oct 13, 2025
2 checks passed
@stalkermv stalkermv deleted the storage-updates branch October 13, 2025 20:07
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.

2 participants