Skip to content
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

ensure sessions are deleted when empty #22

Merged
merged 2 commits into from
Oct 2, 2023
Merged

ensure sessions are deleted when empty #22

merged 2 commits into from
Oct 2, 2023

Conversation

maxcountryman
Copy link
Owner

When a session is empty it should be removed entirely.

Previously this was not the case as the modified flag cannot be set when the underlying session data is empty. This makes sense because an empty session should not be updated but instead deleted.

However, this also meant that removing the last key of a previously stored session would not reflect this update in the store--the session would not be saved to the store or deleted.

To address this, we now look for an empty session after resolving the request. When found, we delete the session from the store and return the response.

Fixes #21.

When a session is empty it should be removed entirely.

Previously this was not the case as the modified flag cannot be set when
the underlying session data is empty. This makes sense because an empty
session should not be updated but instead deleted.

However, this also meant that removing the last key of a previously
stored session would not reflect this update in the store--the session
would not be saved to the store or deleted.

To address this, we now look for an empty session after resolving the
request. When found, we delete the session from the store and return the
response.

Fixes #21.
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #22 (aaad8da) into main (5a1f701) will increase coverage by 1.47%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   77.96%   79.43%   +1.47%     
==========================================
  Files          13       13              
  Lines         422      428       +6     
==========================================
+ Hits          329      340      +11     
+ Misses         93       88       -5     
Files Coverage Δ
src/service.rs 73.23% <100.00%> (+2.47%) ⬆️
src/session.rs 73.73% <100.00%> (+5.05%) ⬆️

@maxcountryman maxcountryman merged commit b415ca8 into main Oct 2, 2023
13 checks passed
@maxcountryman maxcountryman deleted the fix-21 branch October 2, 2023 19:05
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.

Clearing a session's data does not get persisted to the store
1 participant