Refactor lane module into smaller, focused components#3001
Refactor lane module into smaller, focused components#3001jonathangreen merged 3 commits intomainfrom
Conversation
Break up the large lane.py file (~3200 lines) into smaller, more maintainable modules: - Create feed/facets/ package with base.py, constants.py, database.py, feed.py, and search.py for facet-related classes - Create feed/worklist/ package with base.py, database.py, hierarchy.py, specific.py, and top_level.py for WorkList classes - Consolidate Pagination and SortKeyPagination in search/pagination.py - Slim lane.py to ~730 lines containing only Lane, LaneGenre, and lanes_customlists SQLAlchemy models Move corresponding tests to mirror the new source structure. Update imports throughout the codebase to use the new module locations.
Move sqlalchemy model imports to TYPE_CHECKING blocks and use local imports at runtime to avoid circular dependency when lane.py is loaded by sqlalchemy/model/__init__.py.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3001 +/- ##
==========================================
+ Coverage 92.97% 92.99% +0.01%
==========================================
Files 459 468 +9
Lines 43276 43370 +94
Branches 6034 6034
==========================================
+ Hits 40238 40330 +92
- Misses 1966 1967 +1
- Partials 1072 1073 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tdilauro
left a comment
tdilauro
left a comment
There was a problem hiding this comment.
🔀 🚀
I noticed a few inconsistencies unrelated to the refactor along the way, so noted them here for the future. No need to fix for this PR.
| """ | ||
| return filter | ||
|
|
||
| def modify_database_query(cls, _db, qu): |
There was a problem hiding this comment.
Instance method, cls param mismatch. But no use, so could be @staticmethod or function.
| @classmethod | ||
| def top_level_for_library(self, _db, library, collection_ids=None): |
There was a problem hiding this comment.
@classmethod, self param mismatch. But no use, so could be @staticmethod or function.
| """Create a filter clause that only books that are on one of the | ||
| CustomLists allowed by Lane configuration. | ||
|
|
||
| :return: A 3-tuple (query, clauses). |
There was a problem hiding this comment.
Minor - "2-tuple" of (query, clauses), rather than "3-tuple"
2 participants
Description
Break up the large
lane.pyfile (~3200 lines) into smaller, more maintainable modules. This PR contains only code movement and import updates - no logic changes were made.New Structure
I'm not 100% happy with the new structure, and the distinction / location of all the files. But I think its an improvement over what was there before, and now that the files are broken out its easier to move them around if we have a better idea for organization in the future.
feed/facets/package:constants.py-FacetConstants,FacetConfig(moved fromcore/facets.py)base.py-BaseFacets,FacetsWithEntryPointfeed.py-Facets,DefaultSortOrderFacets,FeaturedFacetssearch.py-SearchFacetsdatabase.py-DatabaseBackedFacetsFacets primarily control how feeds are filtered, sorted, and presented. They inherit from
FacetConstantsand are consumed by feed generation code. Placing them infeed/keeps feed-related concepts together. We avoidedcore/since we're trying to move away from that catch-all package.feed/worklist/package:base.py-WorkListdatabase.py-DatabaseBackedWorkListhierarchy.py-HierarchyWorkListtop_level.py-TopLevelWorkListspecific.py-SpecificWorkListWorkLists are used to generate OPDS feeds - they define collections of works that appear in feeds. The search filter creation (
Filter.from_worklist()) is an implementation detail of how feeds are populated.search/pagination.py:Pagination(moved fromlane.py) with existingSortKeyPaginationSortKeyPaginationalready existed here and inherits fromPagination. Consolidating them in one location makes the inheritance relationship clear.sqlalchemy/model/lane.py:Lane,LaneGenre, andlanes_customlistsMotivation and Context
This refactoring was done while investigating potential search improvements for PP-3472. The large
lane.pyfile made it difficult to understand the relationships between facets, worklists, pagination, and the Lane model. Breaking it into focused modules improves maintainability and makes the codebase easier to navigate.How Has This Been Tested?
Checklist