-
Notifications
You must be signed in to change notification settings - Fork 474
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
Move Session Storage to shopify_app
#1055
Changes from all commits
aaa813b
924bd3e
e4e1754
1d989aa
4658620
40c4eb1
3391c5f
b27123c
99290fa
6b3018c
269f322
3b6ff74
82b3a15
3b33a32
1450927
3956654
8b4eb07
d13ace5
b6816eb
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 | ||||
---|---|---|---|---|---|---|
|
@@ -17,12 +17,13 @@ class << self | |||||
).returns(T.nilable(Auth::Session)) | ||||||
end | ||||||
def load_current_session(auth_header: nil, cookies: nil, is_online: false) | ||||||
validate_session_storage_for_deprecated_utils | ||||||
return load_private_session if Context.private? | ||||||
|
||||||
session_id = current_session_id(auth_header, cookies, is_online) | ||||||
return nil unless session_id | ||||||
|
||||||
Context.session_storage.load_session(session_id) | ||||||
T.must(Context.session_storage).load_session(session_id) | ||||||
end | ||||||
|
||||||
sig do | ||||||
|
@@ -33,10 +34,12 @@ def load_current_session(auth_header: nil, cookies: nil, is_online: false) | |||||
).returns(T::Boolean) | ||||||
end | ||||||
def delete_current_session(auth_header: nil, cookies: nil, is_online: false) | ||||||
validate_session_storage_for_deprecated_utils | ||||||
|
||||||
session_id = current_session_id(auth_header, cookies, is_online) | ||||||
return false unless session_id | ||||||
|
||||||
Context.session_storage.delete_session(session_id) | ||||||
T.must(Context.session_storage).delete_session(session_id) | ||||||
end | ||||||
|
||||||
sig do | ||||||
|
@@ -46,8 +49,10 @@ def delete_current_session(auth_header: nil, cookies: nil, is_online: false) | |||||
).returns(T.nilable(Auth::Session)) | ||||||
end | ||||||
def load_offline_session(shop:, include_expired: false) | ||||||
validate_session_storage_for_deprecated_utils | ||||||
|
||||||
session_id = offline_session_id(shop) | ||||||
session = Context.session_storage.load_session(session_id) | ||||||
session = T.must(Context.session_storage).load_session(session_id) | ||||||
return nil if session && !include_expired && session.expires && T.must(session.expires) < Time.now | ||||||
|
||||||
session | ||||||
|
@@ -59,23 +64,10 @@ def load_offline_session(shop:, include_expired: false) | |||||
).returns(T::Boolean) | ||||||
end | ||||||
def delete_offline_session(shop:) | ||||||
session_id = offline_session_id(shop) | ||||||
Context.session_storage.delete_session(session_id) | ||||||
end | ||||||
|
||||||
private | ||||||
|
||||||
sig { returns(Auth::Session) } | ||||||
def load_private_session | ||||||
unless Context.private_shop | ||||||
raise Errors::SessionNotFoundError, "Could not load private shop, Context.private_shop is nil." | ||||||
end | ||||||
validate_session_storage_for_deprecated_utils | ||||||
|
||||||
Auth::Session.new( | ||||||
shop: T.must(Context.private_shop), | ||||||
access_token: Context.api_secret_key, | ||||||
scope: Context.scope.to_a, | ||||||
) | ||||||
session_id = offline_session_id(shop) | ||||||
T.must(Context.session_storage).delete_session(session_id) | ||||||
end | ||||||
|
||||||
sig do | ||||||
|
@@ -129,6 +121,29 @@ def offline_session_id(shop) | |||||
def cookie_session_id(cookies) | ||||||
cookies[Auth::Oauth::SessionCookie::SESSION_COOKIE_NAME] | ||||||
end | ||||||
|
||||||
private | ||||||
|
||||||
sig { void } | ||||||
def validate_session_storage_for_deprecated_utils | ||||||
unless Context.session_storage | ||||||
raise ShopifyAPI::Errors::SessionStorageError, | ||||||
"session_storage is required in ShopifyAPI::Context when using deprecated Session utility methods." | ||||||
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.
Suggested change
If these session utility methods are deprecated, what's the recommended replacement? Presumably something in 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. this particular error will be super hard to reach. This error was put in place mainly because the sorbet one wasn't pretty. I think the upgrade logs we have in the |
||||||
end | ||||||
end | ||||||
|
||||||
sig { returns(Auth::Session) } | ||||||
def load_private_session | ||||||
unless Context.private_shop | ||||||
raise Errors::SessionNotFoundError, "Could not load private shop, Context.private_shop is nil." | ||||||
end | ||||||
|
||||||
Auth::Session.new( | ||||||
shop: T.must(Context.private_shop), | ||||||
access_token: Context.api_secret_key, | ||||||
scope: Context.scope.to_a, | ||||||
) | ||||||
end | ||||||
end | ||||||
end | ||||||
end | ||||||
|
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.
It's probably an edge case, but should we be retaining the previous behaviour of raising if
store_session
returns nil?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.
The new normal is for
Context.session_storage
to be nil as it is deprecated. It's my hope that this will be nil :)