🐛 Fix Consecutive Annotations Load Bug#927
🐛 Fix Consecutive Annotations Load Bug#927shaneahmed merged 1 commit intoTissueImageAnalytics:developfrom
Conversation
aacic
force-pushed
the
fix-consecutive-annotations-bug
branch
from
0233f93 to
f240903
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #927 +/- ##
========================================
Coverage 99.70% 99.70%
========================================
Files 71 71
Lines 8847 8847
Branches 1154 1154
========================================
Hits 8821 8821
Misses 23 23
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shaneahmed
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a bug where annotation overlays for slides were mismatched by incorporating session-specific temporary database files.
- Update temporary database file naming using session_id to ensure overlays correspond to the correct session.
- Adjust commit logic to correctly identify session-specific temporary files.
| np.array(model_mpp) / np.array(self.slide_mpps[session_id]), | ||
| ) | ||
| tmp_path = Path(tempfile.gettempdir()) / "temp.db" | ||
| tmp_path = Path(tempfile.gettempdir()) / f"temp_{session_id}.db" |
There was a problem hiding this comment.
Ensure that session_id is sanitized or validated before using it in the file name to prevent potential file path injection vulnerabilities.
| tmp_path = Path(tempfile.gettempdir()) / f"temp_{session_id}.db" | |
| sanitized_session_id = re.sub(r'[^a-zA-Z0-9_]', '_', session_id) | |
| tmp_path = Path(tempfile.gettempdir()) / f"temp_{sanitized_session_id}.db" |
There was a problem hiding this comment.
The tiatoolbox code generates the session ID, so this is not a concern.
| else: | ||
| # make a temporary db for the new annotations | ||
| tmp_path = Path(tempfile.gettempdir()) / "temp.db" | ||
| tmp_path = Path(tempfile.gettempdir()) / f"temp_{session_id}.db" |
There was a problem hiding this comment.
[nitpick] Consider refactoring the logic for creating session-specific temporary file names into a helper function to avoid code duplication and potential inconsistencies in future modifications.
There was a problem hiding this comment.
The tiatoolbox code generates the session ID, so this is not a concern.
measty
left a comment
measty
left a comment
There was a problem hiding this comment.
Thanks for adding this bugfix, the PR looks fine to me
This PR fixes a bug that occurs when multiple slides are loaded into the TileServer and multiple annotation overlays are subsequently uploaded in sequence.
Steps to reproduce:
Load Slide 1 into the TileServer.
Load Slide 2 into the TileServer.
Upload the annotation overlay (GeoJSON) for Slide 1 via the /tileserver/overlay endpoint.
Upload the annotation overlay (GeoJSON) for Slide 2 via the /tileserver/overlay endpoint.
Attempt to load the overlay for Slide 1 via the /tileserver/layer/overlay/default/zoomify/ endpoint.
Expected behavior:
The annotation overlay for Slide 1 is displayed.
Actual behavior:
The annotation overlay for Slide 2 is displayed instead.