refactor(storage): centralize default_db_path in utils#70
Closed
syf2211 wants to merge 3 commits into
Closed
Conversation
Extract the duplicated _default_db_path helper from graph_schema.py and graph_model.py into utils.default_db_path, and route PersistentRegistry path construction through the same helper. Fixes Wolfvin#40
Use os.path.abspath before joining the .codelens DB path so relative or traversal workspace inputs cannot escape the intended directory. Addresses SonarCloud security rating on PR Wolfvin#70.
Use pytest tmp_path instead of /tmp/codelens-workspace to satisfy SonarCloud rule python:S5443 (publicly writable directories). Addresses SonarCloud security rating on PR Wolfvin#70.
Owner
Review - Closing as superseded by #71Thanks for the PR! Solid implementation, but PR #71 addresses the same issue (#40) with slightly better coverage, so closing this to avoid duplicate merge conflicts. Why #71 wins
What this PR did better
These are minor enough that #71 can adopt them in a follow-up if needed. VerdictClosing. Your contribution is appreciated - please feel free to pick up another open issue at https://github.com/Wolfvin/CodeLens/issues. Reviewed by BOS (orchestrate-workers skill) - diff read in full before decision. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract the duplicated
_default_db_path()helper into a singleutils.default_db_path()function and routePersistentRegistrythrough the same helper.Motivation
_default_db_path()was copy-pasted identically ingraph_schema.pyandgraph_model.py, andpersistent_registry.pyinlined the same path logic. Any future change (e.g.CODELENS_DB_PATHsupport) would need to be applied in multiple places, creating silent drift risk.Fixes #40
Changes
DB_FILENAMEanddefault_db_path()toscripts/utils.py_default_db_path()fromgraph_schema.pyandgraph_model.pydefault_db_pathas_default_db_pathfromgraph_model.pyfor backward compatibility with existing importsdefault_db_path()inPersistentRegistry.__init__anddb_exists()TestDefaultDbPathregression test asserting all three modules resolve the same pathTests
pytest tests/test_graph_model.py::TestDefaultDbPath -v— PASSpytest tests/test_graph_model.py -q— 21 passedNotes
resolve_types.pystill has a local copy; left out of scope to keep this PR minimal. Happy to follow up.<workspace>/.codelens/codelens.db.