-
-
Notifications
You must be signed in to change notification settings - Fork 143
fix: decoder and encoder default instances #711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -34,6 +34,7 @@ extension SessionStorage { | |||||
let migrations: [StorageMigration] = [ | ||||||
.sessionNewKey(clientID: clientID), | ||||||
.storeSessionDirectly(clientID: clientID), | ||||||
.useDefaultEncoder(clientID: clientID), | ||||||
] | ||||||
|
||||||
var key: String { | ||||||
|
@@ -46,14 +47,16 @@ extension SessionStorage { | |||||
do { | ||||||
try migration.run() | ||||||
} catch { | ||||||
logger?.error("Storage migration failed: \(error.localizedDescription)") | ||||||
logger?.error( | ||||||
"Storage migration '\(migration.name)' failed: \(error.localizedDescription)" | ||||||
) | ||||||
} | ||||||
} | ||||||
|
||||||
do { | ||||||
let storedData = try storage.retrieve(key: key) | ||||||
return try storedData.flatMap { | ||||||
try AuthClient.Configuration.jsonDecoder.decode(Session.self, from: $0) | ||||||
try JSONDecoder().decode(Session.self, from: $0) | ||||||
} | ||||||
} catch { | ||||||
logger?.error("Failed to retrieve session: \(error.localizedDescription)") | ||||||
|
@@ -64,7 +67,7 @@ extension SessionStorage { | |||||
do { | ||||||
try storage.store( | ||||||
key: key, | ||||||
value: AuthClient.Configuration.jsonEncoder.encode(session) | ||||||
value: JSONEncoder().encode(session) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using JSONEncoder.supabase() here instead of a new JSONEncoder(), to maintain consistent encoding behavior in session storage.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the comment related to |
||||||
) | ||||||
} catch { | ||||||
logger?.error("Failed to store session: \(error.localizedDescription)") | ||||||
|
@@ -82,14 +85,15 @@ extension SessionStorage { | |||||
} | ||||||
|
||||||
struct StorageMigration { | ||||||
var name: String | ||||||
var run: @Sendable () throws -> Void | ||||||
} | ||||||
|
||||||
extension StorageMigration { | ||||||
/// Migrate stored session from `supabase.session` key to the custom provided storage key | ||||||
/// or the default `supabase.auth.token` key. | ||||||
static func sessionNewKey(clientID: AuthClientID) -> StorageMigration { | ||||||
StorageMigration { | ||||||
StorageMigration(name: "sessionNewKey") { | ||||||
let storage = Dependencies[clientID].configuration.localStorage | ||||||
let newKey = SessionStorage.key(clientID) | ||||||
|
||||||
|
@@ -117,16 +121,38 @@ extension StorageMigration { | |||||
var expirationDate: Date | ||||||
} | ||||||
|
||||||
return StorageMigration { | ||||||
return StorageMigration(name: "storeSessionDirectly") { | ||||||
let storage = Dependencies[clientID].configuration.localStorage | ||||||
let key = SessionStorage.key(clientID) | ||||||
|
||||||
if let data = try? storage.retrieve(key: key), | ||||||
let storedSession = try? AuthClient.Configuration.jsonDecoder.decode(StoredSession.self, from: data) | ||||||
let storedSession = try? AuthClient.Configuration.jsonDecoder.decode( | ||||||
StoredSession.self, | ||||||
from: data | ||||||
) | ||||||
{ | ||||||
let session = try AuthClient.Configuration.jsonEncoder.encode(storedSession.session) | ||||||
try storage.store(key: key, value: session) | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
static func useDefaultEncoder(clientID: AuthClientID) -> StorageMigration { | ||||||
StorageMigration(name: "useDefaultEncoder") { | ||||||
let storage = Dependencies[clientID].configuration.localStorage | ||||||
let key = SessionStorage.key(clientID) | ||||||
|
||||||
let storedData = try? storage.retrieve(key: key) | ||||||
let sessionUsingOldDecoder = storedData.flatMap { | ||||||
try? AuthClient.Configuration.jsonDecoder.decode(Session.self, from: $0) | ||||||
} | ||||||
|
||||||
if let sessionUsingOldDecoder { | ||||||
try storage.store( | ||||||
key: key, | ||||||
value: JSONEncoder().encode(sessionUsingOldDecoder) | ||||||
) | ||||||
} | ||||||
} | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using JSONDecoder.supabase() here instead of instantiating a new JSONDecoder(), to ensure consistent decoding behavior across the codebase.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is the decoder used for decoding the stored session, it is safer to use a new decoder instance, unrelated to the other ones used in codebase, since any change to decoder configuration may break the session decoding.