v2.4.1: 实现获取收藏夹内容，优化类和函数的命名 #164
Conversation
WalkthroughThe recent updates transition from a debug-focused logging system to a more general custom logging approach. This shift involves renaming functions and variables from 'debug' to 'log', refining the API client functionality, and enhancing the plugin system. The changes streamline the codebase, improve documentation, and introduce new features like favorite folder handling and more flexible request methods. Changes
Poem
TipsChat with CodeRabbit Bot (
|
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 11
Configuration used: CodeRabbit UI
Files selected for processing (17)
- README.md (1 hunks)
- assets/docs/sources/TODO.md (1 hunks)
- assets/docs/sources/index.md (1 hunks)
- assets/docs/sources/tutorial/4_module_custom.md (1 hunks)
- assets/docs/sources/tutorial/8_pick_domain.md (1 hunks)
- src/jmcomic/init.py (1 hunks)
- src/jmcomic/cl.py (1 hunks)
- src/jmcomic/jm_client_impl.py (18 hunks)
- src/jmcomic/jm_client_interface.py (11 hunks)
- src/jmcomic/jm_config.py (14 hunks)
- src/jmcomic/jm_downloader.py (4 hunks)
- src/jmcomic/jm_entity.py (6 hunks)
- src/jmcomic/jm_option.py (8 hunks)
- src/jmcomic/jm_plugin.py (13 hunks)
- src/jmcomic/jm_toolkit.py (7 hunks)
- tests/test_jmcomic/init.py (4 hunks)
- tests/test_jmcomic/test_jm_client.py (2 hunks)
Files skipped from review due to trivial changes (6)
- assets/docs/sources/TODO.md
- assets/docs/sources/index.md
- assets/docs/sources/tutorial/8_pick_domain.md
- src/jmcomic/init.py
- tests/test_jmcomic/init.py
- tests/test_jmcomic/test_jm_client.py
Additional comments: 66
README.md (1)
- 56-60: The documentation has been updated to reflect the renaming of logging-related terminology and the addition of new plugins. It's important to ensure that all references to the old "debug" terminology are updated across the documentation, tutorials, and docstrings within the codebase to maintain consistency.
src/jmcomic/cl.py (1)
- 77-84: The change from
jm_debugtojm_logis consistent with the pull request summary, which indicates a shift from "debug" to "log" terminology. However, it's important to ensure that thejm_logfunction is correctly implemented and that it supports the same functionality as the previousjm_debug. Ifjm_loghas different behavior or requires different parameters, additional changes may be needed to ensure the logging works as intended.src/jmcomic/jm_toolkit.py (5)
- 273-293: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [252-287]
The
JmPageToolclass has been updated with new patterns and methods for parsing HTML to search and favorite pages. Ensure that the new patterns match the expected HTML structure and that the new methods are being called correctly from other parts of the application.
- 303-310: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [290-320]
The
parse_html_to_search_pagemethod has been updated to return aJmSearchPageobject with a total count. Ensure that the total count is being correctly extracted from the HTML and that all consumers of this method are updated to handle the new return type.
323-340: The new
parse_html_to_favorite_pagemethod has been added to parse HTML into aJmFavoritePageobject. Verify that the method correctly extracts the total count and content from the HTML and that the folder list is intentionally left empty (as indicated by the comment on line 337).365-428: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [343-425]
The
parse_api_to_search_pageandparse_api_to_favorite_pagemethods have been updated to adapt API responses to page objects. Ensure that theadapt_contentmethod correctly transforms the API response content into the expected format for theJmSearchPageandJmFavoritePageconstructors.
- 659-669: The
get_nummethod inJmImageToolclass calculates the number of segments for image scrambling based on the album ID and filename. Ensure that the logic for determining the number of segments is correct and that thehashlib.md5usage does not introduce any security concerns, as MD5 is not considered secure for cryptographic purposes.src/jmcomic/jm_downloader.py (4)
4-44: The refactoring from
jm_debugtojm_logis consistent and correctly applied across theDownloadCallbackclass methods. The logging statements are descriptive and seem to be placed appropriately to log the before and after states of album, photo, and image downloads. This should provide clear runtime diagnostics for these operations.108-114: The
execute_by_conditionmethod has been updated to acceptDetailEntityinstead ofDownloadIterObjs. This change aligns with the new type replacement and should be checked for compatibility with the rest of the codebase where this method is used. Ensure that all instances ofDetailEntityare compatible with the expected operations within this method.138-150: The
filter_iter_objsmethod has been updated to accept a singledetailparameter of typeDetailEntity. The method's documentation suggests that it can be overridden for custom filtering logic, which is a flexible design choice. However, ensure that the default implementation (which returns the detail unmodified) is the intended behavior for all cases where this method is not overridden.195-200: The
__exit__method is correctly implemented to log exceptions that may occur during the context manager's scope. This is a good use of Python's context manager protocol to handle cleanup or logging after the execution of a block of code. The log message includes the class name and the exception details, which should aid in debugging.src/jmcomic/jm_entity.py (7)
9-22: The addition of class methods
is_image,is_photo, andis_albumto theJmBaseEntityclass and its subclasses is a good practice for type checking and can be useful for polymorphism. This allows for easy checking of the entity type without the need forisinstancechecks.165-170: The
tagproperty in theJmImageDetailclass provides a formatted string that can be useful for logging and debugging. It's a good use of Python's f-string formatting.450-473: The
JmPageContentclass is well-structured with clear properties and methods. However, thepage_sizeproperty is marked asNotImplementedError, which means subclasses are expected to implement this. Ensure that all subclasses provide an implementation for this property to avoid runtime errors.491-495: The
iter_id_title_taggenerator method inJmPageContentclass is a good example of Python's generator capabilities, providing an efficient way to iterate over content. The use ofsetdefaultto ensuretag_listis a list even if it's not present inainfois a good defensive programming practice.514-517: The
page_sizeproperty in theJmSearchPageclass correctly retrieves the value fromJmMagicConstants. This centralizes the page size value, making it easier to manage and update.537-538: The
wrap_single_albumclass method inJmSearchPageclass is a good utility method for wrapping a single album into a search page format. This could be useful for uniform handling of album entities in contexts where a search page is expected.543-555: The
JmFavoritePageclass constructor correctly initializes the base class and sets thefolder_listattribute. Thepage_sizeproperty implementation is consistent with theJmSearchPageclass, using a constant fromJmMagicConstants.src/jmcomic/jm_client_interface.py (9)
- 195-199: The
loginmethod has been simplified by removing additional parameters. Ensure that this change is reflected across all calls to this method and that no functionality is lost due to the removal of these parameters.Hunk 2
- 220-233: The
favorite_foldermethod has been added to theJmUserClientclass. This method should be implemented to fetch the contents of a user's favorite folder. Ensure that the implementation adheres to the API's expected behavior and error handling is in place for failed requests.Hunk 3
- 251-257: The
download_imagemethod in theJmImageClientclass has been updated. Ensure that the defaultscramble_idprovided aligns with the application's requirements and that the method's error handling is robust.Hunk 4
No changes to review in this hunk.
Hunk 5
- 322-329: The
search_sitemethod is a convenience wrapper around thesearchmethod. Ensure that the default values provided for the parameters match the expected behavior for site-wide searches.Hunk 6
- 333-340: The
search_workmethod is another convenience wrapper around thesearchmethod. Verify that themain_tagparameter is correctly set to represent the "work" category in the context of the application.Hunk 7
- 344-351: The
search_authormethod is added to facilitate searches by author. As with other search methods, ensure that themain_tagparameter is correctly set to represent the "author" category.Hunk 8
- 355-362: The
search_tagmethod is introduced for tag-based searches. Verify that themain_tagparameter is correctly set to represent the "tag" category.Hunk 9
- 366-375: The
search_actormethod is added for actor-based searches. Verify that themain_tagparameter is correctly set to represent the "actor" category.Hunk 10
- 366-454: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [445-483]
The
search_genmethod has been modified to return a generator. This change allows for more efficient iteration over search results, especially when dealing with large datasets. Ensure that the implementation correctly handles state between yields and that the generator can be safely used in the context of the application.src/jmcomic/jm_plugin.py (9)
34-41: The method
loghas been refactored to use the newjm_logfunction. Ensure that thejm_logfunction is defined and imported correctly in the context where this class is used. Also, verify that theplugin_keyis set for all subclasses ofJmOptionPluginsince it's being used to construct the logging topic.71-79: The
JmLoginPluginclass now updates theAPP_COOKIESdirectly after login. Ensure that this change is thread-safe and consistent with the rest of the application, especially ifAPP_COOKIESis accessed from multiple threads.210-221: The
FindUpdateDownloaderclass is a specializedJmDownloaderthat filters out already downloaded chapters. Ensure that thefind_updatefunction correctly identifies new chapters to download. Also, verify that thedownload_albumfunction is correctly updated to work with this new downloader class.318-324: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [318-332]
The
do_zipmethod in theZipPluginclass zips the specified directory. Ensure that thebackup_dir_to_zipfunction is defined and imported correctly. Also, consider handling exceptions that may occur during the zipping process to prevent partial zip files in case of errors.
394-400: The
ClientProxyPluginclass replaces thenew_jm_clientmethod with a proxy client for certain clients. Ensure that thewhitelistis correctly applied and that the proxy client is properly instantiated with the necessary arguments.415-421: The
ImageSuffixFilterPluginclass filters out images with disallowed suffixes. Ensure that thedecide_download_cachemethod is correctly overridden and that the original method is called when the suffix is allowed. Also, verify that settingimage.is_exists = Trueis the correct way to skip downloading an image.446-449: The
SendQQEmailPluginclass sends an email using the provided configuration. Ensure that the email sending functionality is tested and works as expected. Also, consider adding error handling for the email sending process to manage failures gracefully.455-467: The
LogTopicFilterPluginclass filters logging based on a whitelist of topics. Ensure that the new logging function correctly filters out topics not in the whitelist and that the original logging function is called otherwise.510-519: The
AutoSetBrowserCookiesPluginclass attempts to set browser cookies automatically. Ensure that theget_browser_cookiesfunction handles different browsers correctly and that the error handling is sufficient for different types of failures. Also, verify that only the accepted cookies are updated in theoption.</blockquote></details> <details><summary>src/jmcomic/jm_client_impl.py (10)</summary><blockquote> * 79-93: The logging statement within the `request_with_retry` method should be reviewed for potential information leakage. Logging the URL and parameters can expose sensitive information, especially if `login` is part of the URL. Consider masking sensitive information before logging. * 106-116: The `before_retry` method logs the exception `e` directly. It's important to ensure that this does not lead to logging sensitive information. Additionally, the `jm_log` function should be reviewed to ensure it handles exceptions properly and does not cause any side effects when logging. * 176-180: The `fallback` method raises an exception using a custom `ExceptionTool.raises` method. Ensure that this method is implemented correctly and that it logs the error and raises an exception that is appropriately handled by the caller. * 190-196: The `decode` method is used to decode URLs when logging. Ensure that this method is only called in a logging context and that the decoded URLs are not used in any context where they might be treated as trusted input, as URL decoding can potentially introduce security vulnerabilities if not handled correctly. * 268-279: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [268-302] The `login` method has been updated to return a response object. Ensure that all callers of this method are updated to handle the new return type. Additionally, the method now checks for the presence of the 'AVS' cookie and returns early if it's missing after a login attempt. This behavior should be clearly documented, and callers should be aware of this potential early return to handle it correctly. * 304-322: The new `favorite_folder` method has been added to the `JmApiClient` class. Ensure that this method is tested thoroughly, especially with different `page`, `order_by`, `folder_id`, and `username` parameters to ensure it behaves as expected. * 476-482: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [476-510] The `search` method in the `JmApiClient` class has been updated. Ensure that the changes to the method's logic, especially the handling of the `redirect_aid` in the response data, are correct and that the method returns the expected `JmSearchPage` object. * 529-535: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [529-588] The `get_scramble_id` method has been added to cache and fetch scramble IDs. Ensure that the caching logic is correct and that the cache is properly invalidated when necessary to prevent stale data issues. Also, verify that the fallback scramble ID (`JmMagicConstants.SCRAMBLE_220980`) is a sensible default and that the logging statement in case of a missing scramble ID does not leak sensitive information. * 652-656: The `setting` method has been added to the `JmApiClient` class. Ensure that this method is properly handling the response from the `/setting` API endpoint and that the response data is being used appropriately. * 661-736: The `login` method in the `JmApiClient` class has been updated to handle API login and set cookies. Ensure that the method correctly handles the response from the `/login` API endpoint, sets the cookies appropriately, and that the `require_resp_success` method is correctly validating the response. </blockquote></details> <details><summary>src/jmcomic/jm_option.py (7)</summary><blockquote> * 289-298: The `construct` method has been updated to handle the new `log` configuration parameter. The method checks if logging is disabled and calls `disable_jm_log()` accordingly. This is a good approach to handle feature toggles. However, ensure that the `disable_jm_log` function is implemented and works as expected. * 323-329: The `deconstruct` method has been updated to reflect the change from `debug` to `log` in the returned dictionary. This is consistent with the changes made throughout the codebase to standardize logging terminology. * 372-386: The `new_jm_client` method has been updated to handle the `impl` parameter more flexibly. It now supports passing a class type directly and retrieves the `client_key` from it. This is a good improvement for flexibility and usability. Ensure that all client implementations have a `client_key` attribute and that it is correctly used throughout the codebase. * 536-542: The `invoke_plugin` method has been updated to use `jm_log` for logging instead of `jm_debug`. This change is consistent with the overall logging refactor. Ensure that the `jm_log` function is implemented and that the logging levels and formats are correctly configured. * 563-573: The `handle_plugin_valid_exception` method provides different behaviors based on the `mode` configuration. It supports 'ignore', 'log', and 'raise' modes, which is a flexible way to handle validation exceptions from plugins. Ensure that the `jm_log` function is correctly capturing these logs and that the `mode` configuration is being set and used correctly. * 580-589: The `handle_plugin_unexpected_error` and `handle_plugin_exception` methods log the exception and then re-raise it. This is consistent with the logging and error handling strategy seen in other parts of the code. Ensure that the `jm_log` function is implemented and that the logging levels and formats are correctly configured. * 613-619: The `fix_kwargs` method is converting non-string keys to strings for plugin kwargs, which is a good practice for ensuring consistency in data types. The logging of the type conversion is helpful for debugging. Ensure that the `jm_log` function is implemented and that the logging levels and formats are correctly configured. </blockquote></details> <details><summary>src/jmcomic/jm_config.py (13)</summary><blockquote> * 3-8: The function `default_jm_logging` has been renamed to reflect the shift from "debug" to "log". This change is consistent with the pull request's goal of standardizing logging terminology. Ensure that all references to the old function name are updated across the codebase. * 80-86: The comment about cookies being used only on the mobile side and not being verified for content could potentially be a security concern if the cookies contain sensitive information. Verify that the cookies are handled securely and consider adding validation or encryption if necessary. * 130-142: The configuration for logging has been updated to use the new `log_executor` and related settings. It's important to ensure that the logging behavior is consistent with the previous version unless intentional changes have been made. Also, verify that the default values for logging are set appropriately and that they can be easily configured if needed. * 211-215: The method `get_html_url` has been updated to use the new logging method. Ensure that the logging level and format are appropriate and that sensitive information is not being logged. * 232-236: The method `get_html_domain_all` has been updated to use the new logging method. As with the previous hunk, ensure that the logging level and format are appropriate and that sensitive information is not being logged. * 245-249: The method `get_cookies` has been updated to use the new logging method. Again, ensure that the logging level and format are appropriate and that sensitive information, such as cookies, is not being logged in an insecure manner. * 266-271: Headers configuration for web requests has been updated. Ensure that these headers are appropriate for the requests being made and that they do not expose any sensitive information or cause any security concerns. * 280-286: The method `new_api_headers` generates headers for mobile API requests, including a token. Ensure that the token generation is secure and that the `key_ts` and `APP_SECRET` are not exposed or logged inappropriately. * 302-311: The `jm_log` method and the `disable_jm_log` method have been updated to reflect the new logging configuration. Ensure that the logic for enabling and disabling logging works as expected and that it is consistent across the codebase. * 340-346: The default proxy configuration has been updated to use the system proxy by default. Ensure that this default behavior is documented and that it can be overridden if necessary. Also, verify that the system proxy is used securely and does not expose the application to man-in-the-middle attacks. * 365-372: The plugin configuration has been updated. Ensure that the behavior for handling plugin exceptions is clear and that the default 'log' action is appropriate for the application's needs. Also, verify that the plugin system allows for easy configuration and extension. * 381-389: The `option_default_dict` method has been updated to reflect the new logging configuration and to provide default values for various options. Ensure that these defaults are appropriate and that they can be easily overridden by the user if necessary. * 429-433: The registration methods for plugins and clients have been updated. Ensure that the registration process is robust and that it properly handles cases where a plugin or client is not found. Also, verify that the `jm_log` and `disable_jm_log` methods are accessible and function correctly after being assigned to module-level variables. </blockquote></details></blockquote></details> </details>
| ## 自定义log | ||
|
|
||
| ```python | ||
| def custom_jm_debug(): | ||
| def custom_jm_log(): | ||
| """ | ||
| 该函数演示自定义debug | ||
| 该函数演示自定义log | ||
| """ | ||
|
|
||
| # jmcomic模块在运行过程中会使用 jm_debug() 这个函数进行打印信息 | ||
| # jm_debug() 这个函数 最后会调用 JmModuleConfig.debug_executor 函数 | ||
| # 你可以写一个自己的函数，替换 JmModuleConfig.debug_executor，实现自定义debug | ||
| # jmcomic模块在运行过程中会使用 jm_log() 这个函数进行打印信息 | ||
| # jm_log() 这个函数 最后会调用 JmModuleConfig.log_executor 函数 | ||
| # 你可以写一个自己的函数，替换 JmModuleConfig.log_executor，实现自定义log | ||
|
|
||
| # 1. 自定义debug函数 | ||
| def my_debug(topic: str, msg: str): | ||
| # 1. 自定义log函数 | ||
| def my_log(topic: str, msg: str): | ||
| """ | ||
| 这个debug函数的参数列表必须包含两个参数，topic和msg | ||
| @param topic: debug主题，例如 'album.before', 'req.error', 'plugin.error' | ||
| @param msg: 具体debug的信息 | ||
| 这个log函数的参数列表必须包含两个参数，topic和msg | ||
| @param topic: log主题，例如 'album.before', 'req.error', 'plugin.error' | ||
| @param msg: 具体log的信息 | ||
| """ | ||
| pass | ||
|
|
||
| # 2. 让my_debug生效 | ||
| JmModuleConfig.debug_executor = my_debug | ||
| # 2. 让my_log生效 | ||
| JmModuleConfig.log_executor = my_log |
There was a problem hiding this comment.
The code snippet demonstrates how to customize the logging function within the jmcomic module. The renaming from custom_jm_debug to custom_jm_log and my_debug to my_log is consistent with the overall shift from "debug" to "log" terminology. The comments and docstrings have been updated accordingly to reflect this change. This is a good practice as it maintains consistency and clarity in the codebase.
However, the custom log function my_log provided in the example is empty and does not contain any implementation. It is important to either provide a sample implementation or a comment indicating that users should add their own logging logic inside the function.
def my_log(topic: str, msg: str):
"""
This is a custom log function. You should implement your logging logic here.
For example, you could print the log messages to the console or write them to a file.
@param topic: log topic, such as 'album.before', 'req.error', 'plugin.error'
@param msg: the actual log message
"""
# Example implementation:
# print(f"{topic}: {msg}")
pass| @classmethod | ||
| def analyse_jm_search_html(cls, html: str) -> JmSearchPage: | ||
| return JmSearchTool.parse_html_to_page(html) | ||
| return JmPageTool.parse_html_to_search_page(html) | ||
|
|
||
| @classmethod | ||
| def reflect_new_instance(cls, html: str, cls_field_prefix: str, clazz: type): |
There was a problem hiding this comment.
The reflect_new_instance method is complex and does a lot of work. Consider breaking it down into smaller, more focused methods to improve readability and maintainability. For example, the pattern matching and field extraction logic could be separated from the instance creation logic.
| def __iter__(self) -> Generator[JmImageDetail, None, None]: | ||
| return super().__iter__() | ||
|
|
||
| @classmethod | ||
| def is_photo(cls): | ||
| return True |
There was a problem hiding this comment.
The implementation of the __iter__ method in JmPhotoDetail class is correct, but it's redundant as it simply calls the superclass's __iter__ method. If the superclass's method is not being overridden to provide additional functionality, this method could be removed to reduce unnecessary code.
- def __iter__(self) -> Generator[JmImageDetail, None, None]:
- return super().__iter__()Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| def __iter__(self) -> Generator[JmImageDetail, None, None]: | |
| return super().__iter__() | |
| @classmethod | |
| def is_photo(cls): | |
| return True | |
| @classmethod | |
| def is_photo(cls): | |
| return True |
| def __iter__(self) -> Generator[JmPhotoDetail, None, None]: | ||
| return super().__iter__() | ||
|
|
||
| @classmethod | ||
| def is_album(cls): | ||
| return True |
There was a problem hiding this comment.
Similar to the previous comment, the __iter__ method in JmAlbumDetail class is redundant if it's only calling the superclass's __iter__ method without adding any new behavior.
- def __iter__(self) -> Generator[JmPhotoDetail, None, None]:
- return super().__iter__()Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| def __iter__(self) -> Generator[JmPhotoDetail, None, None]: | |
| return super().__iter__() | |
| @classmethod | |
| def is_album(cls): | |
| return True | |
| @classmethod | |
| def is_album(cls): | |
| return True |
| ret = self.apply_rule_solver(album, photo, solver) | ||
| except BaseException as e: | ||
| # noinspection PyUnboundLocalVariable | ||
| jm_debug('dir_rule', f'路径规则"{solver[2]}"的解析出错: {e}, album={album}, photo={photo}') | ||
| jm_log('dir_rule', f'路径规则"{solver[2]}"的解析出错: {e}, album={album}, photo={photo}') | ||
| raise e |
There was a problem hiding this comment.
The exception handling in deside_image_save_dir method logs the error and then re-raises it. This is a good practice as it preserves the stack trace while also providing a way to log the error for debugging purposes. However, the method name deside_image_save_dir seems to be a typo and should be corrected to decide_image_save_dir.
- def deside_image_save_dir(self,
+ def decide_image_save_dir(self,Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ret = self.apply_rule_solver(album, photo, solver) | |
| except BaseException as e: | |
| # noinspection PyUnboundLocalVariable | |
| jm_debug('dir_rule', f'路径规则"{solver[2]}"的解析出错: {e}, album={album}, photo={photo}') | |
| jm_log('dir_rule', f'路径规则"{solver[2]}"的解析出错: {e}, album={album}, photo={photo}') | |
| raise e | |
| ret = self.apply_rule_solver(album, photo, solver) | |
| except BaseException as e: | |
| # noinspection PyUnboundLocalVariable | |
| jm_log('dir_rule', f'路径规则"{solver[2]}"的解析出错: {e}, album={album}, photo={photo}') | |
| raise e |
| class JmMagicConstants: | ||
| ORDER_BY_LATEST = 'mr' | ||
| ORDER_BY_VIEW = 'mv' | ||
| ORDER_BY_PICTURE = 'mp' | ||
| ORDER_BY_LIKE = 'tf' | ||
|
|
||
| TIME_TODAY = 't' | ||
| TIME_WEEK = 'w' | ||
| TIME_MONTH = 'm' | ||
| TIME_ALL = 'a' | ||
|
|
||
| # 分页大小 | ||
| PAGE_SIZE_SEARCH = 80 | ||
| PAGE_SIZE_FAVORITE = 20 | ||
|
|
||
| SCRAMBLE_220980 = 220980 | ||
| SCRAMBLE_268850 = 268850 | ||
| SCRAMBLE_421926 = 421926 # 2023-02-08后改了图片切割算法 | ||
|
|
||
| # 移动端API密钥 | ||
| APP_SECRET = '18comicAPPContent' | ||
|
|
There was a problem hiding this comment.
The addition of the JmMagicConstants class is a good practice for managing magic numbers and strings throughout the code. It centralizes the constants, making them easier to manage and update. Ensure that all instances where these constants should be used are updated to reference this class.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 10
Configuration used: CodeRabbit UI
Files selected for processing (7)
- src/jmcomic/cl.py (1 hunks)
- src/jmcomic/jm_client_impl.py (18 hunks)
- src/jmcomic/jm_client_interface.py (10 hunks)
- src/jmcomic/jm_downloader.py (4 hunks)
- src/jmcomic/jm_option.py (10 hunks)
- src/jmcomic/jm_plugin.py (13 hunks)
- tests/test_jmcomic/test_jm_custom.py (1 hunks)
Files skipped from review due to trivial changes (2)
- src/jmcomic/cl.py
- tests/test_jmcomic/test_jm_custom.py
Additional comments: 43
src/jmcomic/jm_downloader.py (3)
7-44: The logging refactor from
jm_debugtojm_logis consistent and correctly implemented across the callback methods. This aligns with the pull request summary indicating a shift to a more general logging system.108-114: The method
execute_by_conditionhas been updated to useDetailEntityinstead ofDownloadIterObjs. This change should be verified across the codebase to ensure that all references and usages have been updated accordingly.138-150: The method
filter_iter_objsis designed to be overridden for custom filtering logic. The comment provides useful examples of how to use this method. It's important to ensure that subclasses or other parts of the codebase that override this method are updated to match the new parameter namedetail.src/jmcomic/jm_client_interface.py (10)
194-198: The
loginmethod signature has been simplified by removing unnecessary parameters. This change will require updates in all places whereloginis called to ensure compatibility.216-232: The addition of the
favorite_foldermethod is consistent with the pull request summary, which mentions the ability to retrieve contents of a favorite folder. Ensure that the implementation of this method is provided elsewhere in the codebase.306-311: The
searchmethod's signature has been updated to remove several constants related to ordering and time. This simplification could affect existing calls tosearchthat relied on these constants. Ensure that all calls tosearchare updated accordingly.321-327: The
search_sitemethod is a wrapper around thesearchmethod with default parameters. This is a good use of method overloading to provide convenience methods for common search types.332-338: Similarly, the
search_workmethod provides a specialized search for album works. This is another example of method overloading for convenience.343-349: The
search_authormethod is added for searching by album author. This addition is consistent with the pull request summary, which mentions updates to the client and interface.354-360: The
search_tagmethod is added for searching by album tags. This addition is consistent with the pull request summary, which mentions updates to the client and interface.365-374: The
search_actormethod is added for searching by album actors. This addition is consistent with the pull request summary, which mentions updates to the client and interface.427-442: The
favorite_folder_genmethod is a generator that yields pages of favorite folder contents. This is a new feature mentioned in the pull request summary. Ensure that thedo_page_itermethod called within this generator is implemented and tested.365-453: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [444-482]
The
search_genmethod is a generator that yields search result pages. This method allows for dynamic search parameter updates via thesendmethod, which is a powerful feature for interactive searching. Ensure that thedo_page_itermethod called within this generator is implemented and tested.src/jmcomic/jm_option.py (10)
79-85: The method name has been correctly updated from
deside_image_save_dirtodecide_image_save_dir. This change aligns with the pull request summary and corrects the previous typo.89-93: The exception handling within
decide_image_save_dirmethod is appropriate as it logs the error and re-raises it. This preserves the stack trace and provides logging for debugging purposes. The log message includes thealbumandphotodetails, which is helpful for diagnosing issues.205-211: The
decide_image_save_dirmethod in theJmOptionclass correctly delegates to theDirRuleinstance's method of the same name. This maintains consistency in how directory paths are determined across the application.289-298: The
constructmethod has been updated to handle thelogconfiguration. The method now correctly pops thelogkey from the dictionary and disables logging if the value isFalse. This change is consistent with the pull request summary, which mentions a shift from a debug-focused approach to a more general logging system.323-329: The
deconstructmethod has been updated to include thelogconfiguration in the returned dictionary. This change is consistent with the updates made in theconstructmethod and reflects the new logging system.372-381: The
new_jm_clientmethod has been modified to handle theimplparameter more flexibly. It now supports passing a class type directly and retrieves theclient_keyfrom it. This change improves the method's usability by allowing different types of client implementations to be specified more conveniently.536-542: The
invoke_pluginmethod correctly constructs a plugin instance and logs the invocation. This is consistent with the new logging system and provides traceability for plugin usage within the application.563-571: The
handle_plugin_valid_exceptionmethod provides different handling strategies for plugin validation exceptions based on themodeconfiguration. This allows for flexible error handling, which can be configured to ignore, log, or raise the exception. This is a robust design that can accommodate different operational requirements.580-589: The
handle_plugin_unexpected_errorandhandle_plugin_exceptionmethods log the exception and then re-raise it. This ensures that unexpected errors are not silently swallowed and that they are properly logged for debugging purposes.613-618: The
fix_kwargsmethod includes logic to convert non-string keys to strings and logs the conversion. This ensures that plugin keyword arguments are in the expected format and provides logging for any type conversions that occur, which is useful for debugging.src/jmcomic/jm_plugin.py (8)
34-41: The method
loghas been refactored to usejm_loginstead of a debug equivalent. This change aligns with the overall shift towards a more general logging system. Ensure that all references to the old debug method are updated to use this newlogmethod.71-79: The
JmLoginPluginclass'sinvokemethod has been updated to log a message upon successful login instead of using a debug statement. This change is consistent with the new logging approach. Ensure that the login method's return value is handled correctly where this plugin is used.210-221: The
FindUpdateDownloaderclass'sfilter_iter_objsmethod has been updated to accept a singledetailparameter instead ofiter_objs. This change simplifies the method signature and potentially improves the clarity of the method's purpose. Ensure that all usages of this method are updated accordingly.318-324: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [318-332]
The
do_zipmethod in theZipPluginclass now logs messages instead of debugging them. This change is consistent with the new logging approach. Ensure that the log messages provide enough context for the operation being performed.
394-400: The
ClientProxyPluginclass now logs messages instead of debugging them when a client is proxied. This change is consistent with the new logging approach. Ensure that the proxying mechanism is tested thoroughly to prevent any unintended side effects.415-422: The
ImageSuffixFilterPluginclass'sinvokemethod has been updated to log messages instead of debugging them when an image is skipped due to its suffix not being in the allowed set. This change is consistent with the new logging approach. Ensure that the filtering logic is correct and that it does not inadvertently skip images that should be processed.446-467: The
SendQQEmailPluginandLogTopicFilterPluginclasses now log messages instead of debugging them. This change is consistent with the new logging approach. Ensure that the email sending functionality is tested to confirm that it works as expected with the new logging system.510-519: The
AutoSetBrowserCookiesPluginclass now logs messages instead of debugging them when browser cookies are successfully retrieved or if there is a failure. This change is consistent with the new logging approach. Ensure that the error handling is robust and that the plugin gracefully handles cases where cookies cannot be retrieved.src/jmcomic/jm_client_impl.py (12)
106-110: The
log_topicmethod is defined but does not seem to be used anywhere within the provided code. If it's not used elsewhere in the codebase, consider removing it to avoid dead code.176-180: The
fallbackmethod logs a message and then raises an exception usingExceptionTool.raises. Ensure thatExceptionTool.raisesis a valid method and is imported correctly, as it's not defined within the provided code.190-196: The
decodemethod is used to decode URLs, but it's only applied if the URL contains '/search/'. If the intention is to always decode URLs when logging, this condition might be too restrictive. Consider whether this logic is correct or if it should be applied more generally.268-279: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [268-302]
The
loginmethod has been refactored to return a response object and handle cookies. Ensure that the status code check (if resp.status_code != 301:) is correct for the application's logic. Typically, a successful login might redirect with a 302 status code, not 301. Also, the handling of cookies should be verified to ensure that it aligns with the application's authentication flow.
304-322: The
favorite_foldermethod has been added to retrieve favorite folders. Ensure that theget_username_or_raisemethod is implemented correctly and that it can indeed raise an exception if the username is not found. The methodget_username_or_raiseis called but not defined within the provided code, so it should be verified in the rest of the codebase.333-334: The
get_jm_htmlmethod is a wrapper around thegetmethod that adds additional checks. Ensure that therequire_200parameter is being used correctly and that the methodcheck_special_http_codeis defined and imported correctly, as it's not present within the provided code.385-401: The
album_commentmethod is used to post comments on albums. Ensure that the logging statements are correct and that the response is handled properly. The methodJmAcRespis used to wrap the response, but it's not defined within the provided code. Verify that it's defined elsewhere and used correctly.406-432: The
require_resp_success_else_raisemethod checks for specific error URLs after a redirect. Ensure that the logic for detecting errors based on the URL is correct and that theExceptionToolmethods used for raising exceptions are defined and imported correctly.661-736: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [476-727]
The
JmApiClientclass has been updated with new API endpoints and methods for interacting with the API. Ensure that the new methods and properties likeAPI_SEARCH,API_ALBUM,API_CHAPTER,API_SCRAMBLE, andAPI_FAVORITEare correctly implemented and that the URLs correspond to the correct API endpoints.
652-656: The
settingmethod is supposed to fetch settings from an API endpoint. Verify that the endpoint/settingis correct and that the methodreq_apiis properly implemented to handle the API request.661-736: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [658-698]
The
loginmethod in theJmApiClientclass has been updated to handle API login. Ensure that the API endpoint/loginis correct and that the methodreq_apiis properly implemented. Also, verify that the response handling and cookie management are correct for the application's authentication flow.
- 700-715: The
favorite_foldermethod in theJmApiClientclass has been added to retrieve favorite folders from the API. Verify that the API endpoint used is correct and that the methodJmPageTool.parse_api_to_favorite_pageis properly implemented to parse the API response.
| jm_debug('image.after', | ||
| f'图片下载完成: {image.tag}, [{image.img_url}] → [{img_save_path}]') | ||
| jm_log('image.after', | ||
| f'图片下载完成: {image.tag}, [{image.img_url}] → [{img_save_path}]') | ||
|
|
||
|
|
||
| class JmDownloader(DownloadCallback): |
There was a problem hiding this comment.
The class JmDownloader inherits from DownloadCallback, which is a good use of inheritance to extend functionality. The docstring could be more descriptive about the responsibilities of JmDownloader.
class JmDownloader(DownloadCallback):
"""
JmDownloader extends DownloadCallback to provide a comprehensive downloading
mechanism, including scheduling logic and interaction with JmcomicClient instances.
It also maintains a record of all downloaded images for plugin data provision.
"""|
|
||
| def __exit__(self, exc_type, exc_val, exc_tb): | ||
| if exc_type is not None: | ||
| jm_debug('dler.exception', | ||
| f'{self.__class__.__name__} Exit with exception: {exc_type, exc_val}' | ||
| ) | ||
| jm_log('dler.exception', | ||
| f'{self.__class__.__name__} Exit with exception: {exc_type, exc_val}' | ||
| ) |
There was a problem hiding this comment.
The __exit__ method is correctly logging exceptions that may occur during the context manager's execution. This is a good practice for debugging and error tracking. However, it would be beneficial to also handle the cleanup or rollback actions if necessary, depending on the context of use.
def __exit__(self, exc_type, exc_val, exc_tb):
if exc_type is not None:
jm_log('dler.exception',
f'{self.__class__.__name__} Exit with exception: {exc_type, exc_val}')
# Perform necessary cleanup or rollback actions hereGeneral Observations:
- The code is well-structured and follows Python's naming conventions.
- The logging statements are descriptive and should provide good insight during debugging and monitoring.
- The use of type hints throughout the code improves readability and helps with type checking.
- The use of
noinspectioncomments suggests that the code might be using an IDE-specific feature to suppress warnings. It's important to ensure that these suppressions are valid and do not hide potential issues. - The pull request summary mentions updates to the README, TODO, and tutorial documents, but these are not included in the provided diff. It's important to verify that all documentation is consistent with the code changes.
| :param decode_image: 要保存的是解密后的图还是原图 | ||
| """ | ||
| if scramble_id is None: | ||
| scramble_id = JmModuleConfig.SCRAMBLE_220980 | ||
| scramble_id = JmMagicConstants.SCRAMBLE_220980 | ||
|
|
||
| # 请求图片 | ||
| resp = self.get_jm_image(img_url) |
There was a problem hiding this comment.
The default value for scramble_id is set to a constant JmMagicConstants.SCRAMBLE_220980. This seems like a magic number and should be documented to explain its significance or replaced with a more descriptive constant name.
|
|
||
| raise PluginValidationException(self, msg) | ||
|
|
||
| def warning_lib_not_install(self, lib='psutil'): | ||
| def warning_lib_not_install(self, lib: str): | ||
| msg = (f'插件`{self.plugin_key}`依赖库: {lib}，请先安装{lib}再使用。' | ||
| f'安装命令: [pip install {lib}]') | ||
| import warnings |
There was a problem hiding this comment.
The warning_lib_not_install method now explicitly specifies the type of its lib parameter as a string, which is a good practice for type clarity. However, consider adding a mechanism to prevent the plugin's execution if the required library is not installed, as the current implementation only issues a warning without stopping the execution.
| if len(warning_msg_list) != 0: | ||
| warning_msg_list.insert(0, '硬件占用告警，占用过高可能导致系统卡死！') | ||
| warning_msg_list.append('') | ||
| self.debug('\n'.join(warning_msg_list), topic='warning') | ||
| self.log('\n'.join(warning_msg_list), topic='warning') |
There was a problem hiding this comment.
The monitor_resource_usage method in the UsageLogPlugin class logs a warning if resource usage exceeds certain thresholds. Ensure that the logging does not become too verbose and affect the performance itself.
| # f"发送的字节数: {network_bytes_sent}", | ||
| # f"接收的字节数: {network_bytes_received}", | ||
| ]) | ||
| self.debug(msg, topic='log') | ||
| self.log(msg, topic='log') | ||
|
|
||
| if enable_warning is True: | ||
| # 警告 |
There was a problem hiding this comment.
The commented-out code for network usage should be removed if it's not intended to be used, to keep the codebase clean and maintainable.
| continue | ||
|
|
||
| os.remove(f) | ||
| self.debug(f'删除原文件: {f}', 'remove') | ||
| self.log(f'删除原文件: {f}', 'remove') | ||
|
|
||
| for d in sorted(dir_list, reverse=True): | ||
| # check exist | ||
| if file_exists(d): | ||
| os.rmdir(d) | ||
| self.debug(f'删除文件夹: {d}', 'remove') | ||
| self.log(f'删除文件夹: {d}', 'remove') |
There was a problem hiding this comment.
The delete_all_files_and_empty_dir method in the ZipPlugin class deletes files and directories after zipping. Ensure that this deletion is safe and does not accidentally delete unintended files. It might be beneficial to add a dry-run or confirmation step before deletion, especially for irreversible operations like file deletion.
| # 抽象基类，实现了域名管理，发请求，重试机制，log，缓存等功能 | ||
| class AbstractJmClient( | ||
| JmcomicClient, | ||
| PostmanProxy, |
There was a problem hiding this comment.
The class AbstractJmClient is missing a closing parenthesis for the class definition. This will cause a syntax error. The closing parenthesis should be added after PostmanProxy,.
- PostmanProxy,
+ PostmanProxy):Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # 抽象基类，实现了域名管理，发请求，重试机制，log，缓存等功能 | |
| class AbstractJmClient( | |
| JmcomicClient, | |
| PostmanProxy, | |
| # 抽象基类，实现了域名管理，发请求，重试机制，log，缓存等功能 | |
| class AbstractJmClient( | |
| JmcomicClient, | |
| PostmanProxy): |
| api_path=url, | ||
| domain=self.domain_list[domain_index], | ||
| ) | ||
| jm_debug(self.debug_topic_request(), self.decode(url)) | ||
| jm_log(self.log_topic(), self.decode(url)) | ||
| else: | ||
| # 图片url | ||
| pass | ||
|
|
||
| if domain_index != 0 or retry_count != 0: | ||
| jm_debug(f'req.retry', | ||
| ', '.join([ | ||
| f'次数: [{retry_count}/{self.retry_times}]', | ||
| f'域名: [{domain_index} of {self.domain_list}]', | ||
| f'路径: [{url}]', | ||
| f'参数: [{kwargs if "login" not in url else "#login_form#"}]' | ||
| ]) | ||
| ) | ||
| jm_log(f'req.retry', | ||
| ', '.join([ | ||
| f'次数: [{retry_count}/{self.retry_times}]', | ||
| f'域名: [{domain_index} of {self.domain_list}]', | ||
| f'路径: [{url}]', | ||
| f'参数: [{kwargs if "login" not in url else "#login_form#"}]' | ||
| ]) | ||
| ) |
There was a problem hiding this comment.
The jm_log function is called with a string concatenation inside the function call. It's better to use formatted strings for readability and performance.
- jm_log(f'req.retry',
- ', '.join([
- f'次数: [{retry_count}/{self.retry_times}]',
- f'域名: [{domain_index} of {self.domain_list}]',
- f'路径: [{url}]',
- f'参数: [{kwargs if "login" not in url else "#login_form#"}]'
- ])
- )
+ jm_log('req.retry', f'次数: [{retry_count}/{self.retry_times}], '
+ f'域名: [{domain_index} of {self.domain_list}], '
+ f'路径: [{url}], '
+ f'参数: [{"#login_form#" if "login" in url else kwargs}]')Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| api_path=url, | |
| domain=self.domain_list[domain_index], | |
| ) | |
| jm_debug(self.debug_topic_request(), self.decode(url)) | |
| jm_log(self.log_topic(), self.decode(url)) | |
| else: | |
| # 图片url | |
| pass | |
| if domain_index != 0 or retry_count != 0: | |
| jm_debug(f'req.retry', | |
| ', '.join([ | |
| f'次数: [{retry_count}/{self.retry_times}]', | |
| f'域名: [{domain_index} of {self.domain_list}]', | |
| f'路径: [{url}]', | |
| f'参数: [{kwargs if "login" not in url else "#login_form#"}]' | |
| ]) | |
| ) | |
| jm_log(f'req.retry', | |
| ', '.join([ | |
| f'次数: [{retry_count}/{self.retry_times}]', | |
| f'域名: [{domain_index} of {self.domain_list}]', | |
| f'路径: [{url}]', | |
| f'参数: [{kwargs if "login" not in url else "#login_form#"}]' | |
| ]) | |
| ) | |
| api_path=url, | |
| domain=self.domain_list[domain_index], | |
| ) | |
| jm_log(self.log_topic(), self.decode(url)) | |
| else: | |
| # 图片url | |
| pass | |
| if domain_index != 0 or retry_count != 0: | |
| jm_log('req.retry', f'次数: [{retry_count}/{self.retry_times}], ' | |
| f'域名: [{domain_index} of {self.domain_list}], ' | |
| f'路径: [{url}], ' | |
| f'参数: [{"#login_form#" if "login" in url else kwargs}]') |
| return self.client_key | ||
|
|
||
| # noinspection PyMethodMayBeStatic, PyUnusedLocal | ||
| def before_retry(self, e, kwargs, retry_count, url): | ||
| jm_debug('req.error', str(e)) | ||
| jm_log('req.error', str(e)) |
There was a problem hiding this comment.
The before_retry method logs the exception but does not re-raise it or handle it in any other way. This could lead to silent failures where exceptions are logged but not properly handled, potentially causing the application to continue in an erroneous state.
Summary by CodeRabbit
New Features
Enhancements
Documentation
Bug Fixes
Refactor
Tests