增加登录用户上下文接口，无需硬编码写死，格式化部分代码 #198
Conversation
# Conflicts: # base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java
WalkthroughThis pull request implements dynamic user context management by introducing the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Service as PageServiceImpl
participant Context as LoginUserContext
Client->>Service: createPage(pageData)
Service->>Context: getLoginUserId()
Context-->>Service: "1"
Service->>Client: Return created page object
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
base/src/main/java/com/tinyengine/it/common/utils/TestUtil.java (1)
16-35: Consider alternatives to reflection for testingWhile the utility helps with testing private fields, using reflection to modify private fields is generally considered a testing anti-pattern as it breaks encapsulation.
Consider alternatives:
- Add package-private getters/setters for testing
- Restructure the code to use dependency injection
- Use testing-specific subclasses that expose necessary functionality
If reflection is necessary, consider adding comments explaining why it's needed.
/** * 测试工工具类 + * + * Note: This utility should only be used in test code when absolutely necessary, + * as modifying private fields breaks encapsulation and can lead to brittle tests. + * Consider redesigning the code to avoid the need for this utility. */base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java (1)
48-50: Consider updating updateFill method.While the insertFill method was updated to use the LoginUserContext, the updateFill method still doesn't set the lastUpdatedBy field dynamically.
Consider updating the updateFill method to also use the LoginUserContext:
@Override public void updateFill(MetaObject metaObject) { this.setFieldValByName("lastUpdatedTime", LocalDateTime.now(), metaObject); + this.setFieldValByName("lastUpdatedBy", loginUserContext.getLoginUserId(), metaObject); }base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java (1)
34-40: Updated test method to support LoginUserContext.The test now:
- Properly declares exceptions from reflection
- Injects a mock implementation of LoginUserContext
- Adjusts verification expectations (changed from 7 to 6 calls)
Consider adding a comment explaining why the verification count changed from 7 to 6 calls, as this could help future maintainers understand the impact of the context changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/src/main/java/com/tinyengine/it/config/context/DefaultLoginUserContext.java(1 hunks)base/src/main/java/com/tinyengine/it/common/base/PageQueryVo.java(2 hunks)base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java(1 hunks)base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java(2 hunks)base/src/main/java/com/tinyengine/it/common/utils/TestUtil.java(1 hunks)base/src/main/java/com/tinyengine/it/controller/PageController.java(3 hunks)base/src/main/java/com/tinyengine/it/mapper/BlockGroupBlockMapper.java(1 hunks)base/src/main/java/com/tinyengine/it/mapper/PageHistoryMapper.java(2 hunks)base/src/main/java/com/tinyengine/it/model/dto/PageHistoryVo.java(1 hunks)base/src/main/java/com/tinyengine/it/model/dto/PublishedPageVo.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/PageHistoryService.java(1 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java(3 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java(4 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java(10 hunks)base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java(1 hunks)base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java(3 hunks)base/src/test/java/com/tinyengine/it/service/app/impl/PageServiceImplTest.java(4 hunks)base/src/test/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImplTest.java(6 hunks)base/src/test/java/com/tinyengine/it/service/material/impl/BlockServiceImplTest.java(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java (1)
base/src/main/java/com/tinyengine/it/common/utils/TestUtil.java (1)
TestUtil(19-35)
🔇 Additional comments (58)
base/src/main/java/com/tinyengine/it/service/app/PageHistoryService.java (1)
4-10: Improved documentation formatting.The changes in this file are focused on standardizing the copyright declaration format, which enhances readability and maintains consistency across the codebase.
base/src/main/java/com/tinyengine/it/model/dto/PageHistoryVo.java (1)
4-10: Documentation format standardized.The updates to the license header follow the project's standardization efforts, removing HTML paragraph tags and adding proper spacing for improved readability.
base/src/main/java/com/tinyengine/it/common/base/PageQueryVo.java (2)
4-10: Enhanced documentation formatting.The copyright header has been standardized by removing HTML tags and improving spacing.
27-27: Improved code readability.Adding a blank line after constant declaration follows good Java coding practices by visually separating constants from other class elements.
base/src/main/java/com/tinyengine/it/mapper/BlockGroupBlockMapper.java (1)
4-10: Standardized documentation format.The changes maintain consistency with other files by removing HTML tags and improving the copyright header spacing.
base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (2)
4-10: Documentation format improvements.The copyright header has been standardized to match the formatting in other files.
105-105: Fixed page publishing status.Setting
isPublishedto true when creating page history records ensures that new pages are properly marked as published, addressing the issue mentioned in the PR objectives about page publishing status.base/src/test/java/com/tinyengine/it/service/material/impl/BlockServiceImplTest.java (3)
25-26: Added LoginUserContext import for user context managementThe import of
LoginUserContextaligns with the PR objective of providing a centralized way to manage user context information, replacing hardcoded values with context-driven values.
72-74: LGTM: Added LoginUserContext mockAppropriate addition of a mock for the
LoginUserContextthat will be injected into the service being tested. This supports better testing practices by properly isolating the service from its dependencies.
78-79: Setup for LoginUserContext mockGood setup of the LoginUserContext mock to return a consistent user ID value for test cases. This ensures all tests will run with the same user context.
base/src/main/java/com/tinyengine/it/model/dto/PublishedPageVo.java (2)
4-10: LGTM: Improved documentation formatThe changes improve the format and readability of the copyright and license information with consistent line spacing.
28-29: LGTM: Added line spacingAdded appropriate line spacing after field declaration, improving code readability.
base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java (8)
4-6: LGTM: Improved comment formatting with<p>tagsThe addition of
<p>tags in JavaDoc comments enhances readability and follows standard JavaDoc formatting practices.
16-18: LGTM: Added necessary LoginUserContext importAdding the LoginUserContext import is necessary for the new dynamic user context implementation.
129-131: Implementation of dynamic user contextAdding the LoginUserContext dependency through @Autowired enables dynamic retrieval of user information, eliminating hardcoded user IDs.
224-224: Replaced hardcoded user ID with dynamic user contextGood change - replaces the static user ID with a dynamic method call to retrieve the current user's ID, improving flexibility and security.
290-291: Enhanced user ID retrieval with contextThe comment clarification and dynamic user ID retrieval improve code maintainability and readability.
536-543: LGTM: Improved documentation formattingThe improved formatting in method documentation enhances readability.
564-567: LGTM: Better JavaDoc formattingThe improved JavaDoc formatting for method parameters enhances documentation quality.
579-582: LGTM: Enhanced method documentationThe improved JavaDoc comments for the getSubPage method make the parameters and return values clearer.
base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (4)
16-17: LGTM: Added LoginUserContext importAdding the necessary import for LoginUserContext to support dynamic user information.
55-56: Implementation of dynamic user contextInjecting the LoginUserContext dependency enables dynamic user information retrieval, following the same pattern used in other service implementations.
76-76: Replaced hardcoded user ID with dynamic contextGood change - uses loginUserContext.getLoginUserId() instead of a hardcoded value, improving maintainability and adaptability.
191-193: Enhanced user context for queriesReplaced hardcoded user IDs with dynamic user context retrieval for both group creator and block creator IDs, maintaining consistency with other service implementations.
base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java (2)
171-183: Improved lambda expression formattingThe lambda expression has been reformatted with explicit braces and multi-line structure for better readability and consistent styling.
388-401: Enhanced lambda expression readabilityThe nested lambda expressions have been reformatted with a consistent multi-line structure and explicit braces, improving code readability.
base/src/main/java/com/tinyengine/it/common/context/LoginUserContext.java (1)
1-48: Well-designed user context interfaceThe LoginUserContext interface provides a clean abstraction for accessing user-related information. The comprehensive JavaDoc comments clearly explain the purpose of each method.
Key benefits:
- Eliminates hardcoded user IDs across the codebase
- Enables different implementations for production and testing
- Follows the dependency injection pattern for better testability
- Well-documented with clear method purposes
This is a solid foundation for dynamic user context management.
base/src/main/java/com/tinyengine/it/common/handler/MyMetaObjectHandler.java (4)
16-16: Appropriate import of LoginUserContext.The import statement correctly includes the LoginUserContext class which is going to be used as a dependency.
21-21: Necessary import for Autowired annotation.The Spring Autowired annotation is correctly imported to support dependency injection of the LoginUserContext.
34-36: Good implementation of dependency injection.The LoginUserContext is properly autowired, following Spring best practices for dependency injection, which allows for more flexible and testable code.
41-44: Excellent replacement of hardcoded values with context values.The implementation now correctly retrieves user-specific information (user ID, tenant ID, renter ID) from the LoginUserContext instead of using hardcoded values. This makes the handler more flexible and aligns with the PR objective of eliminating hardcoded values.
base/src/test/java/com/tinyengine/it/common/handler/MockUserContext.java (5)
1-10: Well-formed copyright header.The copyright header is properly formatted with paragraph tags and follows project conventions.
12-15: Appropriate package and import declarations.The class is placed in the correct package and imports the necessary LoginUserContext interface.
16-19: Adequate class documentation.The class has a basic documentation comment explaining its purpose as a mock user context.
20-33: Good implementation of core user context methods.The mock implementation provides fixed return values for the tenant ID, user ID, and renter ID methods, which is appropriate for testing purposes.
35-43: Proper implementation of platform-related methods.The mock correctly implements methods for app ID and platform ID, returning fixed integer values suitable for testing.
base/src/test/java/com/tinyengine/it/service/app/impl/PageServiceImplTest.java (5)
24-24: Appropriate import of LoginUserContext.The import statement correctly includes the LoginUserContext interface.
93-95: Proper mock declaration for LoginUserContext.The LoginUserContext is correctly declared as a mock using the @mock annotation, which follows good testing practices.
157-157: Good mock setup for LoginUserContext in testCreatePage.The loginUserContext.getLoginUserId() method is properly stubbed to return "1", which simulates a logged-in user for the test.
197-197: Consistent mock setup for LoginUserContext in testCreateFolder.The loginUserContext.getLoginUserId() method is correctly stubbed with the same value as in testCreatePage, maintaining consistency across test cases.
234-236:Details
❌ Incorrect review comment
Consider adding loginUserContext mock in testUpdatePage.
The testUpdatePage method doesn't set up a mock response for loginUserContext, which might be needed if the underlying implementation uses it.
🏁 Script executed:
#!/bin/bash # Check if PageServiceImpl.updatePage method uses loginUserContext rg -A 5 "updatePage.*loginUserContext" --glob "base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java"Length of output: 122
Clarification on
loginUserContextMock intestUpdatePage
After further verification, there was no evidence that theupdatePagemethod in base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java referencesloginUserContext. Consequently, setting up a mock response forloginUserContextin the testUpdatePage method does not appear necessary at this point. If the implementation changes in the future to depend onloginUserContext, please revisit this decision.Likely an incorrect or invalid review comment.
base/src/test/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImplTest.java (6)
4-6: Consistent copyright format.The copyright header has been updated to use paragraph tags, maintaining consistency with other files in the project.
18-18: Appropriate import of LoginUserContext.The import statement correctly includes the LoginUserContext interface needed for the mock.
50-51: Proper mock declaration for LoginUserContext.The LoginUserContext is correctly declared as a mock using the @mock annotation, following the same pattern as other dependencies in this test class.
70-71: Good mock setup in testFindBlockGroupById method.The test properly stubs both the blockGroupMapper and loginUserContext methods, ensuring that the test will run correctly with the new context dependency.
111-111: Consistent mock setup in testCreateBlockGroup.The loginUserContext.getLoginUserId() method is correctly stubbed to return "1", consistent with other test methods.
126-126: Appropriate mock setup in testGetBlockGroupByIdsOrAppId.The loginUserContext is properly mocked for this test method, ensuring it will work correctly with the new dependency.
base/src/main/java/com/tinyengine/it/mapper/PageHistoryMapper.java (2)
4-10: License format standardization.The formatting changes to the license comment follow the standardization effort mentioned in the PR objectives.
89-94: Code style improvements.The additional whitespace and indentation adjustments improve readability and consistency in the method signature and parameter descriptions.
base/src/main/java/com/tinyengine/it/controller/PageController.java (3)
4-6: License format standardization.The HTML paragraph tags added to the license comment improve formatting consistency.
73-74: Improved code indentation.The indentation adjustments in the annotations enhance code readability and maintain consistent formatting style.
209-211:Details
❓ Verification inconclusive
Return type enhancement for better API response.
The method now returns the complete
PageHistoryobject instead of just an integer, providing more useful information to API consumers.Please verify that all clients consuming this API are updated to handle the new response type. Run:
🏁 Script executed:
#!/bin/bash # Search for code that consumes the pageDeploy endpoint response rg -A 3 -B 3 "pageDeploy.*Result<Integer>" --type=javaLength of output: 56
API Client Compatibility Verification for
pageDeploy
The updated method now returns a completePageHistoryobject viaResult<PageHistory>, which offers a richer response than the previous integer-based return. Our automated search (using the command targetingpageDeploy.*Result<Integer>) did not uncover any references expecting an integer response. Nevertheless, this absence of evidence doesn’t guarantee that all consumers—especially external clients or those with indirect usage patterns—have been updated accordingly.
- Action Required:
- Manually verify that every client or service consuming the
pageDeployendpoint is now prepared to handle the completePageHistoryobject rather than an integer.base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java (3)
23-23: Added LoginUserContext import.This import supports the dynamic user context functionality being implemented.
97-98: Added LoginUserContext dependency injection.Injecting the LoginUserContext enables dynamic retrieval of user information throughout the service.
126-126: Replaced hardcoded user/platform IDs with context values.This change implements the core objective of the PR by replacing static default values with dynamic user context, improving maintainability and flexibility.
The code now dynamically retrieves:
- User ID via
loginUserContext.getLoginUserId()- Platform ID via
loginUserContext.getPlatformId()This eliminates hardcoded values and uses the actual logged-in user's context.
Also applies to: 174-174, 191-191, 238-238, 457-457, 645-645, 657-657, 702-702
base/src/test/java/com/tinyengine/it/common/handler/MyMetaObjectHandlerTest.java (3)
4-6: License format standardization.The HTML paragraph tags added to the license comment improve formatting consistency.
19-20: Added TestUtil import for reflection support.This import enables setting private fields in the test, which is necessary for injecting the mock context.
43-49: Updated second test method to support LoginUserContext.This change correctly modifies the second test to also inject the MockUserContext implementation.
app/src/main/java/com/tinyengine/it/config/context/DefaultLoginUserContext.java
Show resolved
Hide resolved
* fix published page history query method * 统一版权注释，增加登录用户的上下文接口，解决硬编码问题
2 participants
PR Type
Background and solution
Summary by CodeRabbit
New Features
Refactor