添加了管理员全局搜索Value值的功能 #5182
添加了管理员全局搜索Value值的功能 #5182
Conversation
dosubot
bot
added
the
size:XL
WalkthroughThis update enhances the Apollo platform by introducing new search functionality for configuration items, enabling users to perform fuzzy searches based on keys and values across applications, environments, clusters, and namespaces. The changes encompass new API endpoints, documentation updates, and system parameter configurations to improve the usability and manageability of configuration resources. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GlobalSearchValueController
participant GlobalSearchValueService
participant AdminServiceAPI
User->>GlobalSearchValueController: Search by key & value
GlobalSearchValueController->>GlobalSearchValueService: Call search method
GlobalSearchValueService->>AdminServiceAPI: Request item info
AdminServiceAPI-->>GlobalSearchValueService: Return ItemInfoDTO[]
GlobalSearchValueService-->>GlobalSearchValueController: Return item data
GlobalSearchValueController-->>User: Display search results
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 (
|
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range, codebase verification and nitpick comments (6)
apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js (1)
18-35: Address code style issues: Missing semicolon and ES6 compatibility.Ensure to add missing semicolons and consider using ES6 features for better readability and maintainability.
appService.service('GlobalSearchValueService', ['$resource', '$q', 'AppUtil', function ($resource, $q, AppUtil) { let global_search_resource = $resource('', {}, { get_item_Info_by_key_and_Value: { method: 'GET', isArray: true, url: AppUtil.prefixPath() + '/global-search/item-info/by-key-or-value?key=:key&value=:value', } }); return { findItemInfoByKeyAndValue: function (key, value) { let d = $q.defer(); global_search_resource.get_item_Info_by_key_and_Value({key: key, value: value}, function (result) { d.resolve(result); }, function (error) { d.reject(error); }); return d.promise; } }; }]);apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (2)
5-11: Consider adding field-level comments.Adding comments to describe each field can improve the readability and maintainability of the code.
13-14: Constructor can be omitted if not used.If the default constructor is not used, consider omitting it to reduce unnecessary code.
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (2)
5-12: Consider adding field-level comments.Adding comments to describe each field can improve the readability and maintainability of the code.
14-15: Constructor can be omitted if not used.If the default constructor is not used, consider omitting it to reduce unnecessary code.
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (1)
205-210: Ensure input validation and error handling.The method
getItemInfoBySearchlacks input validation and error handling. Ensure that thekeyandvalueparameters are properly validated and handle potential errors from theitemService.
ItemController.java:@GetMapping("/items-search/key-and-value") public List<ItemInfoDTO> getItemInfoBySearch(@RequestParam(value = "key", required = false, defaultValue = "") String key, @RequestParam(value = "value", required = false, defaultValue = "") String value) { List<ItemInfoDTO> ItemInfoDTOs = itemService.getItemInfoBySearch(key, value); return ItemInfoDTOs; }
ItemService.java:public List<ItemInfoDTO> getItemInfoBySearch(String key, String value) { List<ItemInfoDTO> itemInfoDTOs = new ArrayList<>(); List<Object[]> infos; if (key.isEmpty() && value.isEmpty()) { infos = itemRepository.findAllItems(); } else if (key.isEmpty() && !value.isEmpty()) { // Additional logic } else if (!key.isEmpty() && value.isEmpty()) { // Additional logic } else { // Additional logic } // More logic return itemInfoDTOs; }Analysis chain
Ensure input validation and error handling.
The method
getItemInfoBySearchlacks input validation and error handling. Ensure that thekeyandvalueparameters are properly validated and handle potential errors from theitemService.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify input validation and error handling for `getItemInfoBySearch`. # Test: Search for validations and error handling in the method. rg --type java -A 5 'public List<ItemInfoDTO> getItemInfoBySearch'Length of output: 1853
Script:
#!/bin/bash # Description: Verify input validation and error handling in the entire `ItemService` class. # Test: Search for input validation and error handling in `ItemService`. rg --type java -A 10 'public class ItemService'Length of output: 4036
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
apollo-portal/src/main/resources/static/img/nodata.pngis excluded by!**/*.png
Files selected for processing (16)
- apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (2 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (2 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ReleaseRepository.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (4 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1 hunks)
- apollo-portal/src/main/resources/static/global_search_value.html (1 hunks)
- apollo-portal/src/main/resources/static/i18n/en.json (1 hunks)
- apollo-portal/src/main/resources/static/i18n/zh-CN.json (1 hunks)
- apollo-portal/src/main/resources/static/scripts/app.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js (1 hunks)
- apollo-portal/src/main/resources/static/views/common/nav.html (1 hunks)
Files skipped from review due to trivial changes (1)
- apollo-portal/src/main/resources/static/views/common/nav.html
Additional context used
Biome
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
[error] 181-181: Shouldn't redeclare 'position'. Consider to delete it or rename it.
'position' is defined here:
(lint/suspicious/noRedeclare)
Additional comments not posted (67)
apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (2)
16-25: Constructor looks good.The constructor initializes all fields correctly.
27-33: Getter and setter methods look good.All getter and setter methods are correctly implemented.
Also applies to: 35-41, 43-49, 51-57, 59-65, 67-73, 75-81
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ReleaseRepository.java (1)
50-51: New methodfindAlllooks good.The new method
findAllis correctly added to the interface.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (2)
17-27: Constructor looks good.The constructor initializes all fields correctly.
29-35: Getter and setter methods look good.All getter and setter methods are correctly implemented.
Also applies to: 37-43, 45-51, 53-59, 61-67, 69-75, 77-83
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (5)
26-26: Import statement approved.The import statement for
org.springframework.data.repository.query.Paramis necessary for the new query methods.
47-63: Verify the usage ofRIGHT JOIN.The
RIGHT JOINmight exclude some items where there is no matching namespace. Ensure that this is the intended behavior.
65-80: Verify the usage ofRIGHT JOIN.The
RIGHT JOINmight exclude some items where there is no matching namespace. Ensure that this is the intended behavior.
83-98: Verify the usage ofRIGHT JOIN.The
RIGHT JOINmight exclude some items where there is no matching namespace. Ensure that this is the intended behavior.
100-114: Verify the usage ofRIGHT JOIN.The
RIGHT JOINmight exclude some items where there is no matching namespace. Ensure that this is the intended behavior.apollo-portal/src/main/resources/static/scripts/app.js (1)
67-68: New module approved.The new AngularJS module
global_search_valueis correctly defined with necessary dependencies.Ensure that the dependencies are correctly used in the module.
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (3)
24-27: Import statements approved.The import statements for
ReleaseandReleaseRepositoryare necessary for the new methodgetItemInfoBySearch.
52-64: Constructor modification approved.The constructor is correctly modified to include
ReleaseRepository.
152-192: New method approved.The new method
getItemInfoBySearchis correctly implemented to retrieve item information based on key and value search criteria.Ensure that the release configurations are correctly checked.
apollo-portal/src/main/resources/static/global_search_value.html (4)
17-30: LGTM! HTML structure is correctly implemented.The HTML structure includes necessary meta tags, styles, and AngularJS directives.
49-63: LGTM! Form structure is correctly implemented.The form includes input fields for
keyandvaluewith AngularJS bindings and a submit button.
68-124: LGTM! Table structure is correctly implemented.The table displays search results with columns for various attributes and uses AngularJS bindings.
143-182: LGTM! Script inclusions are correctly implemented.The scripts include AngularJS, jQuery, Bootstrap, and custom scripts.
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (8)
20-35: LGTM! Controller initialization is correctly implemented.The function initializes the controller and sets up scope variables and functions.
39-42: LGTM! Functioninitis correctly implemented.The function initializes permissions.
44-49: LGTM! FunctioninitPermissionis correctly implemented.The function checks for root permissions.
51-204: LGTM! FunctiongetItemInfoByKeyAndValueis correctly implemented.The function retrieves item info based on key and value and handles various cases.
Tools
Biome
[error] 181-181: Shouldn't redeclare 'position'. Consider to delete it or rename it.
'position' is defined here:
(lint/suspicious/noRedeclare)
207-209: LGTM! FunctionjumpToTheEditingPageis correctly implemented.The function navigates to the editing page.
211-215: LGTM! FunctionhighlightKeywordis correctly implemented.The function highlights the keyword in the full text.
217-219: LGTM! FunctioncloseAllValueis correctly implemented.The function toggles the display of highlighted keywords.
221-223: LGTM! FunctionshowAllValueis correctly implemented.The function toggles the display of highlighted keywords.
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (1)
201-206: LGTM! But verify the endpoint usage in the codebase.The code changes are approved.
However, ensure that all usages of the endpoint
"items-search/key-and-value"are correctly handled.Verification successful
The endpoint usage is correctly handled in the codebase.
- The endpoint
items-search/key-and-valueis defined inItemControllerand returns a list ofItemInfoDTOobjects.- The
getPerEnvItemInfoBySearchmethod inAdminServiceAPIcorrectly makes a GET request to this endpoint and processes the response.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the endpoint "items-search/key-and-value". # Test: Search for the endpoint usage. Expect: Only occurances of the new endpoint. rg --type java -A 5 $'items-search/key-and-value'Length of output: 1923
apollo-portal/src/main/resources/static/i18n/zh-CN.json (20)
897-897: Translation approved.The translation for
"Global.Title": "Value的全局搜索"is accurate and contextually appropriate.
898-898: Translation approved.The translation for
"Global.App": "应用名称"is accurate and contextually appropriate.
899-899: Translation approved.The translation for
"Global.Env": "环境"is accurate and contextually appropriate.
900-900: Translation approved.The translation for
"Global.Cluster": "集群名"is accurate and contextually appropriate.
901-901: Translation approved.The translation for
"Global.NameSpace": "命名空间"is accurate and contextually appropriate.
902-902: Translation approved.The translation for
"Global.Status": "发布状态"is accurate and contextually appropriate.
903-903: Translation approved.The translation for
"Global.Key": "Key"is accurate and contextually appropriate.
904-904: Translation approved.The translation for
"Global.Value": "Value"is accurate and contextually appropriate.
905-905: Translation approved.The translation for
"Global.ValueSearch.Tips": "(模糊搜索,key可为配置项名称或content,value为配置项值)"is accurate and contextually appropriate.
906-906: Translation approved.The translation for
"Global.Operate": "操作"is accurate and contextually appropriate.
907-907: Translation approved.The translation for
"Global.Expand": "展开"is accurate and contextually appropriate.
908-908: Translation approved.The translation for
"Global.Abbreviate": "缩略"is accurate and contextually appropriate.
909-909: Translation approved.The translation for
"Global.JumpToEditPage": "跳转到编辑页面"is accurate and contextually appropriate.
910-910: Translation approved.The translation for
"Item.GlobalSearchByKey": "按照Key值检索"is accurate and contextually appropriate.
911-911: Translation approved.The translation for
"Item.GlobalSearchByValue": "按照Value值检索"is accurate and contextually appropriate.
912-912: Translation approved.The translation for
"Item.GlobalSearch": "查询"is accurate and contextually appropriate.
913-913: Translation approved.The translation for
"Item.UnPublished": "未发布"is accurate and contextually appropriate.
914-914: Translation approved.The translation for
"Item.Published": "已发布"is accurate and contextually appropriate.
915-915: Translation approved.The translation for
"Item.GlobalSearchSystemError": "系统出错,请重试或联系系统负责人"is accurate and contextually appropriate.
916-916: Translation approved.The translation for
"ApolloGlobalSearch.NoData": "暂无数据,请进行检索或者添加"is accurate and contextually appropriate.apollo-portal/src/main/resources/static/i18n/en.json (20)
897-897: LGTM!The key
"Global.Title"with the value"Global Search for Value"is clear and descriptive.
898-898: LGTM!The key
"Global.App"with the value"App Name"is clear and consistent with existing terminology.
899-899: LGTM!The key
"Global.Env"with the value"Env Name"is clear and consistent with existing terminology.
900-900: LGTM!The key
"Global.Cluster"with the value"Cluster Name"is clear and consistent with existing terminology.
901-901: LGTM!The key
"Global.NameSpace"with the value"NameSpace Name"is clear and consistent with existing terminology.
902-902: LGTM!The key
"Global.Status"with the value"Posting Status"is clear and consistent with existing terminology.
903-903: LGTM!The key
"Global.Key"with the value"Key"is clear and consistent with existing terminology.
904-904: LGTM!The key
"Global.Value"with the value"Value"is clear and consistent with existing terminology.
905-905: LGTM!The key
"Global.ValueSearch.Tips"with the value"(Fuzzy search, key can be the name or content of the configuration item, value is the value of the configuration item.)"is clear and descriptive.
906-906: LGTM!The key
"Global.Operate"with the value"Operate"is clear and consistent with existing terminology.
907-907: LGTM!The key
"Global.Expand"with the value"Expand"is clear and consistent with existing terminology.
908-908: LGTM!The key
"Global.Abbreviate"with the value"Abbreviate"is clear and consistent with existing terminology.
909-909: LGTM!The key
"Global.JumpToEditPage"with the value"Jump to edit page"is clear and consistent with existing terminology.
910-910: LGTM!The key
"Item.GlobalSearchByKey"with the value"Search by Key"is clear and consistent with existing terminology.
911-911: LGTM!The key
"Item.GlobalSearchByValue"with the value"Search by Value"is clear and consistent with existing terminology.
912-912: LGTM!The key
"Item.GlobalSearch"with the value"Search"is clear and consistent with existing terminology.
913-913: LGTM!The key
"Item.UnPublished"with the value"UnPublished"is clear and consistent with existing terminology.
914-914: LGTM!The key
"Item.Published"with the value"Published"is clear and consistent with existing terminology.
915-915: LGTM!The key
"Item.GlobalSearchSystemError"with the value"System error, please try again or contact the system administrator"is clear and descriptive.
916-916: LGTM!The key
"ApolloGlobalSearch.NoData"with the value"No data yet, please search or add"is clear and user-friendly.
...portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
Outdated
Show resolved
Hide resolved
apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js
Outdated
Show resolved
Hide resolved
.../src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
Outdated
Show resolved
Hide resolved
dosubot
bot
added
size:XXL
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (6)
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (2)
62-75: Refactor query for consistency.The query structure is similar to the previous method. Ensure consistency in formatting and consider extracting common query parts if applicable.
77-90: Potential SQL Injection Risk inItemRepositoryQueriesThe use of
%?1%in native SQL queries without proper input sanitization or parameterization can expose the application to SQL injection risks. Consider validating or sanitizing inputs before they are used in queries.
- Methods affected:
findItemsByKeyLikefindItemsByValueLikefindItemsByKeyAndValueLikeEnsure that inputs are properly handled to mitigate SQL injection vulnerabilities.
Analysis chain
Ensure correct use of SQL parameters.
Ensure that SQL injection is not a risk by validating inputs or using parameterized queries correctly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the use of parameterized queries in the repository. # Test: Check for the use of parameterized queries. Expect: All queries should use parameters. rg '@Query' --type java -A 5Length of output: 20007
Script:
#!/bin/bash # Description: Gather more context around the ItemRepository to verify parameter handling. # Test: Check the full content of ItemRepository.java for any annotations or configurations that ensure safe parameter handling. fd 'ItemRepository.java' | xargs catLength of output: 3747
apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java (1)
77-97: Enhance test assertions for clarity.Consider using descriptive assertion messages to improve test readability and ease of debugging.
Assert.assertEquals("Expected item info does not match for key and value search", itemInfoDTO.toString(), ExpectedItemInfoDTOSByKeyAndValue.get(0).toString()); Assert.assertEquals("Expected item info does not match for key search", itemInfoDTO.toString(), ExpectedItemInfoDTOSByKey.get(0).toString()); Assert.assertEquals("Expected item info does not match for value search", itemInfoDTO.toString(), ExpectedItemInfoDTOSByValue.get(0).toString());apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (1)
157-170: Add Edge Case Tests fortestSearchMethodThe
testSearchmethod currently does not cover edge cases such as empty keys and values. Consider adding tests to handle these scenarios to ensure comprehensive test coverage.
- File:
apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java- Method:
testSearchAnalysis chain
Ensure comprehensive test coverage for
testSearch.The
testSearchmethod validates the search functionality. Ensure that it covers edge cases, such as empty keys and values.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `testSearch` covers edge cases like empty keys and values. # Test: Search for test cases in `testSearch`. Expect: Coverage of edge cases. rg --type java -A 5 'public void testSearch'Length of output: 8036
Script:
#!/bin/bash # Search for any additional test coverage related to `testSearch` in the same test file. rg --type java -A 10 '@Test' apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.javaLength of output: 2493
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (1)
205-210: Lack of Validation and Error Handling ingetItemInfoBySearchThe
getItemInfoBySearchmethod does not include explicit validation for null inputs or error handling mechanisms. Consider adding these to improve robustness and prevent potential runtime errors.
- Ensure null checks for
keyandvalue.- Implement error handling for repository method calls.
Analysis chain
New endpoint
getItemInfoBySearchis well-structured.The new endpoint allows searching by key and value, enhancing the API's flexibility. Ensure proper validation and error handling.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `getItemInfoBySearch` has proper validation and error handling. # Test: Search for validation and error handling in `getItemInfoBySearch`. Expect: Proper handling. rg --type java -A 5 'public List<ItemInfoDTO> getItemInfoBySearch'Length of output: 1867
Script:
#!/bin/bash # Search for validation or error handling logic in the ItemService class related to getItemInfoBySearch method. rg --type java -A 10 'getItemInfoBySearch' apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.javaLength of output: 578
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (1)
154-202: Improve Exception Handling ingetItemInfoBySearchMethodThe
getItemInfoBySearchmethod currently usese.printStackTrace()for handling SQL exceptions, which is not suitable for production code. Consider implementing a proper logging mechanism and handling exceptions in a way that maintains application stability.
- Use a logging framework like SLF4J or Log4j to log exceptions.
- Consider re-throwing exceptions or handling them to prevent application crashes.
Analysis chain
New method
getItemInfoBySearchis comprehensive but ensure error handling.The method implements flexible search functionality. Ensure that exceptions, especially SQL exceptions, are handled properly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `getItemInfoBySearch` handles SQL exceptions properly. # Test: Search for exception handling in `getItemInfoBySearch`. Expect: Proper handling. rg --type java -A 5 'public List<ItemInfoDTO> getItemInfoBySearch'Length of output: 1867
Script:
#!/bin/bash # Description: Check for logging or other error handling mechanisms in `getItemInfoBySearch`. # Search for logging statements in the method. rg --type java -A 10 'public List<ItemInfoDTO> getItemInfoBySearch' | grep -i 'log'Length of output: 83
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
apollo-portal/src/main/resources/static/img/nodata.pngis excluded by!**/*.png
Files selected for processing (20)
- apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (2 hunks)
- apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (3 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ReleaseRepository.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (4 hunks)
- apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java (2 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1 hunks)
- apollo-portal/src/main/resources/static/global_search_value.html (1 hunks)
- apollo-portal/src/main/resources/static/i18n/en.json (1 hunks)
- apollo-portal/src/main/resources/static/i18n/zh-CN.json (1 hunks)
- apollo-portal/src/main/resources/static/scripts/app.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js (1 hunks)
- apollo-portal/src/main/resources/static/views/common/nav.html (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java (1 hunks)
Additional comments not posted (33)
apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js (1)
30-37: Add error handling for the HTTP request.The existing comment correctly identifies the need for improved error handling. Consider implementing it to enhance robustness.
Fix missing semicolon.
Ensure to add a semicolon at the end of the return statement for consistency.
Use of
letis appropriate.The use of
letis valid for ES6, which is supported in modern browsers.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1)
37-45: Add error handling for the external API call.The existing comment correctly identifies the need for error handling. Implement try-catch blocks to manage exceptions effectively.
Improve naming conventions for consistency and readability.
The method and variable names should follow Java naming conventions for better readability. Consider renaming them as suggested in the existing comments.
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ReleaseRepository.java (1)
50-50: Addition offindAll()method is appropriate.The
findAll()method is a standard addition to repository interfaces, enabling retrieval of all entities. This enhances the repository's functionality without affecting existing methods.apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (1)
89-99: Consider usingStringBuilderfortoStringmethod.Using
StringBuildercan improve performance, especially if the method is called frequently. This suggestion was made in a previous comment and is still valid.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (1)
99-110: Consider usingStringBuilderfortoStringmethod.Using
StringBuildercan improve performance, especially if the method is called frequently. This suggestion was made in a previous comment and is still valid.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java (1)
51-71: Well-structured tests.The unit tests for
GlobalSearchValueServiceare comprehensive and cover both expected and edge cases. The use of Mockito for mocking dependencies is appropriate, and the assertions are clear.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (2)
61-79: Add error handling for service calls.Consider adding error handling for the
globalSearchValueService.get_PerEnv_ItemInfo_BySearchcall to manage potential exceptions and ensure robustness.
48-51: Improve naming conventions for consistency and readability.Consider renaming the method and variables to follow Java naming conventions and improve readability.
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (1)
46-60: Consider optimizing the SQL query for performance.The use of
RIGHT JOINandLIKEwith wildcards can lead to performance issues on large datasets. Ensure that appropriate indexes are in place and consider alternatives if performance becomes a concern.apollo-portal/src/main/resources/static/scripts/app.js (1)
67-68: LGTM! Verify the module integration.The addition of the
global_search_valuemodule is consistent with the existing structure.Ensure that this module is correctly integrated with the rest of the application and used where necessary.
Verification successful
Module Integration Verified
The
global_search_valuemodule is correctly integrated and used in the application. It is defined inapp.js, and its controller,GlobalSearchValueController, is utilized inglobal_search_value.html. This confirms its active use within the application.
- JavaScript Usage:
GlobalSearchValueController.js- HTML Usage:
global_search_value.htmlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `global_search_value` module. # Test: Search for the module usage. Expect: Occurrences where the module is used or injected. rg --type js -A 5 $'global_search_value'Length of output: 1741
Script:
#!/bin/bash # Description: Check for usage of the GlobalSearchValueController to verify complete integration of the global_search_value module. # Test: Search for the usage of GlobalSearchValueController in HTML or JS files. rg --type js,html 'GlobalSearchValueController'Length of output: 79
Script:
#!/bin/bash # Description: Check for usage of the GlobalSearchValueController to verify complete integration of the global_search_value module. # Test: Search for the usage of GlobalSearchValueController in JavaScript files. rg --type js 'GlobalSearchValueController' # Test: Search for the usage of GlobalSearchValueController in HTML files. rg --type html 'GlobalSearchValueController'Length of output: 1022
apollo-portal/src/main/resources/static/views/common/nav.html (1)
69-69: LGTM! Verify the translation key usage.The addition of the global search link in the navigation menu is a useful enhancement.
Ensure that the translation key
'Global.Title'is correctly defined and used in the translation files.Verification successful
Translation Key Verified: The translation key
'Global.Title'is correctly defined in both English and Chinese translation files. No issues found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the translation key `'Global.Title'` is defined and used. # Test: Search for the translation key definition. Expect: Occurrences in translation files. rg --type json --type js --type html -A 5 $'"Global.Title"'Length of output: 1093
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (1)
1-127: Comprehensive test coverage! Verify service method usage.The test cases for
GlobalSearchValueControllerare thorough and cover various scenarios effectively.Ensure that the
get_PerEnv_ItemInfo_BySearchmethod inGlobalSearchValueServiceis implemented and used correctly.Verification successful
Service Method Verified:
get_PerEnv_ItemInfo_BySearchis Implemented and Used CorrectlyThe
get_PerEnv_ItemInfo_BySearchmethod is implemented in theGlobalSearchValueServiceclass and is used appropriately in both the main code and test cases.
- Implementation:
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java- Usage:
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.javaapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.javaScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `get_PerEnv_ItemInfo_BySearch` method in `GlobalSearchValueService`. # Test: Search for the method implementation and usage. Expect: Occurrences in the service class and its usage. ast-grep --lang java --pattern $'class GlobalSearchValueService { $$$ List<ItemInfo> get_PerEnv_ItemInfo_BySearch($_, $_, $_) { $$$ } $$$ }'Length of output: 155
Script:
#!/bin/bash # Description: Search for the `get_PerEnv_ItemInfo_BySearch` method across the entire codebase to verify its existence and usage. rg 'get_PerEnv_ItemInfo_BySearch'Length of output: 2637
apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (2)
24-25: ImportItemServiceis necessary.The import of
ItemServiceis necessary for the new test method and aligns with the changes.
51-52: AutowiredItemServiceis appropriate.The
ItemServiceis autowired correctly, allowing the test to utilize service-level functionality.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (2)
26-27: ImportReleaseRepositoryis necessary.The import of
ReleaseRepositoryis necessary for the new functionality.
54-66: Constructor modification is appropriate.The constructor now includes
ReleaseRepository, which is necessary for the new search functionality.apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (9)
47-50: LGTM!The
initfunction correctly initializes permissions.
52-57: LGTM!The
initPermissionfunction correctly handles root permission checks.
180-189: LGTM!The
handleErrorfunction appropriately handles error messages.
193-195: LGTM!The
convertPageSizeToIntfunction correctly handles page size changes.
197-205: LGTM!The
changePagefunction correctly updates the current page and displayed items.
208-241: LGTM!The
getPagesArrayfunction correctly calculates the pagination array.
243-246: LGTM!The
jumpToTheEditingPagefunction correctly constructs and opens the URL.
248-252: LGTM!The
highlightKeywordfunction correctly highlights the specified keyword.
254-256: LGTM!The
isShowAllValuefunction correctly toggles the display state.apollo-portal/src/main/resources/static/global_search_value.html (5)
20-30: LGTM!The head section correctly includes stylesheets and sets the page title.
32-34: LGTM!The body section correctly sets up the main container and controller.
48-65: LGTM!The form section correctly binds inputs and handles search submissions.
69-126: LGTM!The table section is well-structured and correctly displays search results.
174-212: LGTM!The scripts section correctly includes necessary dependencies.
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (1)
201-206: LGTM!The
getPerEnvItemInfoBySearchmethod correctly retrieves item information based on search criteria.apollo-portal/src/main/resources/static/i18n/zh-CN.json (1)
897-922: Translations look consistent and complete.The added translations for global search and pagination functionalities are consistent with the existing style and terminology used in the file.
apollo-portal/src/main/resources/static/i18n/en.json (1)
897-922: Translations look consistent and complete.The added translations for global search and pagination functionalities are consistent with the existing style and terminology used in the file.
.../src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (2 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (2 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (3 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1 hunks)
- apollo-portal/src/main/resources/static/global_search_value.html (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (1 hunks)
Additional comments not posted (29)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1)
47-49: LGTM!The method is correctly implemented.
The code changes are approved.
apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (4)
28-29: LGTM!The default constructor is correctly implemented.
The code changes are approved.
31-39: LGTM!The constructor is correctly implemented.
The code changes are approved.
41-49: LGTM!The constructor is correctly implemented.
The code changes are approved.
51-97: LGTM!The getters and setters are correctly implemented.
The code changes are approved.
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (3)
63-64: LGTM!The method is correctly implemented.
The code changes are approved.
66-67: LGTM!The method is correctly implemented.
The code changes are approved.
69-70: LGTM!The method is correctly implemented.
The code changes are approved.
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (9)
210-214: LGTM!The function is correctly implemented.
The code changes are approved.
205-208: LGTM!The function is correctly implemented.
The code changes are approved.
216-218: LGTM!The function is correctly implemented.
The code changes are approved.
192-199: LGTM!The function is correctly implemented.
The code changes are approved.
201-203: LGTM!The function is correctly implemented.
The code changes are approved.
51-54: LGTM!The function is correctly implemented.
The code changes are approved.
56-61: LGTM!The function is correctly implemented.
The code changes are approved.
136-138: LGTM!The function is correctly implemented.
The code changes are approved.
140-155: LGTM!The function is correctly implemented.
The code changes are approved.
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (5)
71-79: LGTM!The test case correctly verifies the behavior when both key and value are empty.
The code changes are approved.
81-90: LGTM!The test case correctly verifies the behavior when there are no active environments.
The code changes are approved.
92-109: LGTM!The test case correctly verifies the behavior when there are active environments but no matching items.
The code changes are approved.
111-145: LGTM!The test case correctly verifies the behavior when there are active environments and matching items, but the number of items exceeds the per-environment limit.
The code changes are approved.
147-179: LGTM!The test case correctly verifies the behavior when there are active environments and matching items within the per-environment limit.
The code changes are approved.
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (2)
55-67: LGTM!The constructor correctly initializes the new
releaseRepositoryfield.The code changes are approved.
183-191: LGTM!The method is well-implemented and does not require any changes.
The code changes are approved.
apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (2)
205-209: LGTM!The endpoint is well-implemented and correctly uses the
itemServiceto perform the operation.The code changes are approved.
211-215: LGTM!The endpoint is well-implemented and correctly uses the
itemServiceto perform the operation.The code changes are approved.
apollo-portal/src/main/resources/static/global_search_value.html (3)
1-18: LGTM!The license information and HTML declaration are correctly included.
The code changes are approved.
20-30: LGTM!The meta tag, favicon, stylesheets, and dynamic title are correctly included.
The code changes are approved.
172-213: LGTM!The necessary scripts are correctly included.
The code changes are approved.
apollo-portal/src/main/resources/static/scripts/services/GlobalSearchValueService.js
Show resolved
Hide resolved
...portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
Outdated
Show resolved
Hide resolved
apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/global_search_value.html
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java
Outdated
Show resolved
Hide resolved
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java
Outdated
Show resolved
Hide resolved
|
I have read the CLA Document and I hereby sign the CLA |
.../src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java (1)
89-90: Add a brief comment explaining the purpose of the method.Adding a comment will improve the readability and maintainability of the code.
+ /** + * Retrieves the maximum number of search results per environment. + * Defaults to 200 if the property is not set. + */ public int getPerEnvSearchMaxResults() { return getIntProperty("apollo.portal.search.perEnvMaxResults", 200); }apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (2)
55-67: Add a brief comment explaining the purpose of the new parameter.Adding a comment will improve the readability and maintainability of the code.
public ItemService( final ItemRepository itemRepository, final @Lazy NamespaceService namespaceService, final AuditService auditService, final BizConfig bizConfig, + // ReleaseRepository is used to access release-related data final ReleaseRepository releaseRepository) { this.itemRepository = itemRepository; this.namespaceService = namespaceService; this.auditService = auditService; this.bizConfig = bizConfig; this.releaseRepository = releaseRepository; }
181-181: Add a brief comment explaining the purpose of the method.Adding a comment will improve the readability and maintainability of the code.
+ /** + * Counts the number of items matching the search criteria. + */ public int countItemInfoNumBySearch(String key, String value) { if (key.isEmpty() && !value.isEmpty()) { return itemRepository.countByValueLike(value); } else if (value.isEmpty() && !key.isEmpty()) { return itemRepository.countByKeyLike(key); } else { return itemRepository.countByKeyAndValueLike(key, value); } }apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (1)
205-210: Add a brief comment explaining the purpose of the endpoint.Adding a comment will improve the readability and maintainability of the code.
+ /** + * Retrieves a paginated list of ItemInfoDTO objects based on optional query parameters for key and value. + */ @GetMapping("/items-search/key-and-value") public PageDTO<ItemInfoDTO> getItemInfoBySearch(@RequestParam(value = "key", required = false) String key, @RequestParam(value = "value", required = false) String value, Pageable limit) { return new PageDTO<>(itemService.getItemInfoBySearch(key, value, limit).getContent(), limit, itemService.getItemInfoBySearch(key, value, limit).getTotalElements()); }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java (2 hunks)
- apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (3 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java (2 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (3 hunks)
- apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java (2 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (2 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ItemRepository.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java
Additional comments not posted (17)
apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java (4)
1-16: LGTM!The license header is compliant with the Apache License, Version 2.0.
17-43: LGTM!The package declaration and imports are appropriate for the test class.
55-68: LGTM!The test method
testGet_PerEnv_ItemInfo_BySearch_withKeyAndValue_ReturnExpectedItemInfosis well-structured and verifies the expected behavior of the service method.
70-76: LGTM!The test method
testGet_PerEnv_ItemInfo_withKeyAndValue_BySearch_ReturnEmptyItemInfosis well-structured and verifies the expected behavior of the service method.apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java (5)
Line range hint
1-16: LGTM!The license header is compliant with the Apache License, Version 2.0.
Line range hint
17-31: LGTM!The package declaration and imports are appropriate for the test class.
Line range hint
33-52: LGTM!The test method
testSaveItemis well-structured and verifies the expected behavior of the service method.
Line range hint
53-78: LGTM!The test method
testUpdateItemis well-structured and verifies the expected behavior of the service method.
79-100: LGTM!The test method
testSearchItemis well-structured and verifies the expected behavior of the service method.apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (6)
Line range hint
1-16: LGTM!The license header is compliant with the Apache License, Version 2.0.
Line range hint
17-37: LGTM!The package declaration and imports are appropriate for the test class.
Line range hint
39-78: LGTM!The test method
testCreateis well-structured and verifies the expected behavior of the controller method.
Line range hint
79-115: LGTM!The test method
testUpdateis well-structured and verifies the expected behavior of the controller method.
Line range hint
116-157: LGTM!The test method
testDeleteis well-structured and verifies the expected behavior of the controller method.
159-178: LGTM!The test method
testSearchis well-structured and verifies the expected behavior of the controller method.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java (2)
187-188: LGTM!The addition of the
pageItemInfoDTOfield is appropriate for handling paginated responses.
204-211: Consider adding error handling for the REST call.The method is correctly implemented, but it lacks error handling for the REST call. This can be achieved by wrapping the REST call in a try-catch block and handling exceptions appropriately.
Apply this diff to add error handling:
public PageDTO<ItemInfoDTO> getPerEnvItemInfoBySearch(Env env, String key, String value, int page, int size){ - ResponseEntity<PageDTO<ItemInfoDTO>> - entity = - restTemplate.get(env, - "items-search/key-and-value?key={key}&value={value}&page={page}&size={size}", - pageItemInfoDTO, key, value, page, size); - return entity.getBody(); + try { + ResponseEntity<PageDTO<ItemInfoDTO>> + entity = + restTemplate.get(env, + "items-search/key-and-value?key={key}&value={value}&page={page}&size={size}", + pageItemInfoDTO, key, value, page, size); + return entity.getBody(); + } catch (Exception e) { + // Handle exception (e.g., log the error, return an empty page, etc.) + return new PageDTO<>(); + } }
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
.../src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/zh/portal/apollo-user-guide.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- docs/zh/portal/apollo-user-guide.md
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (4)
docs/zh/design/apollo-introduction.md (1)
73-74: Fix the formatting of the unordered list.Markdownlint reported some formatting issues in the unordered list:
- The list items are using 1 space for indentation instead of the recommended 2 spaces.
- Hard tabs are used for indentation instead of spaces.
Please apply this diff to fix the issues:
- * 通过对配置项的key与value进行的模糊检索，找到拥有对应值的配置项在哪个应用、环境、集群、命名空间中被使用 - * 通过高亮显示、分页与跳转配置等操作，便于让管理员以及SRE角色快速、便捷地找到与更改资源的配置值 + * 通过对配置项的key与value进行的模糊检索，找到拥有对应值的配置项在哪个应用、环境、集群、命名空间中被使用 + * 通过高亮显示、分页与跳转配置等操作，便于让管理员以及SRE角色快速、便捷地找到与更改资源的配置值Tools
Markdownlint
73-73: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
74-74: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
73-73: Column: 1
Hard tabs(MD010, no-hard-tabs)
74-74: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/zh/portal/apollo-user-guide.md (1)
132-132: Nitpick: Use consistent unordered list style.For better consistency and to adhere to the Markdownlint rules, please consider using asterisks (
*) instead of dashes (-) for the unordered list items.Also applies to: 136-136
Tools
Markdownlint
132-132: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
docs/en/portal/apollo-user-guide.md (1)
499-511: Improve clarity and consistency.Consider making the following changes:
- Replace "In order to" with "To" to avoid wordiness.
- Use asterisk (
*) instead of dash (-) for the unordered list style to maintain consistency.Apply this diff to implement the suggestions:
-Starting from version 2.4.0, apollo-portal adds the ability to globally search for configuration items by fuzzy retrieval of the key and value of a configuration item to find out which application, environment, cluster, or namespace the configuration item with the corresponding value is used in. In order to prevent memory overflow (OOM) problems when performing global view searches of configuration items, we introduce a system parameter `apollo.portal.search.perEnvMaxResults`, which is used to limit the number of maximum search results per environment configuration item in a single search. By default, this value is set to `200`, but administrators can adjust it to suit their actual needs. +Starting from version 2.4.0, apollo-portal adds the ability to globally search for configuration items by fuzzy retrieval of the key and value of a configuration item to find out which application, environment, cluster, or namespace the configuration item with the corresponding value is used in. To prevent memory overflow (OOM) problems when performing global view searches of configuration items, we introduce a system parameter `apollo.portal.search.perEnvMaxResults`, which is used to limit the number of maximum search results per environment configuration item in a single search. By default, this value is set to `200`, but administrators can adjust it to suit their actual needs. **Setting method：** -1. Log in to the Apollo Configuration Center interface with a super administrator account. -2. Just go to the `Administrator Tools - System Parameters` page and add or modify the `apollo.portal.search.perEnvMaxResults` configuration item. +* Log in to the Apollo Configuration Center interface with a super administrator account. +* Just go to the `Administrator Tools - System Parameters` page and add or modify the `apollo.portal.search.perEnvMaxResults` configuration item.Tools
LanguageTool
[style] ~501-~501: Consider a shorter alternative to avoid wordiness.
Context: ...ith the corresponding value is used in. In order to prevent memory overflow (OOM) problems ...(IN_ORDER_TO_PREMIUM)
README.md (1)
46-48: Nit: Consider using dashes for the unordered list style.Markdownlint hints indicate that the list items at lines 46-48 use asterisks instead of the expected dashes. Consider updating the list style for consistency with Markdown best practices:
-* **Unified management of the configurations of different environments and different clusters** +- **Unified management of the configurations of different environments and different clusters** -* **Configuration changes takes effect in real time (hot release)** +- **Configuration changes takes effect in real time (hot release)** -* **Global Search Configuration Items** +- **Global Search Configuration Items**This is a minor formatting suggestion and can be skipped if not deemed necessary.
Tools
Markdownlint
46-46: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
47-47: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
48-48: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
docs/en/images/Configuration query-Non properties.pngis excluded by!**/*.pngdocs/en/images/Configuration query-properties.pngis excluded by!**/*.pngdocs/zh/images/Configuration query-Non properties.pngis excluded by!**/*.pngdocs/zh/images/Configuration query-properties.pngis excluded by!**/*.png
Files selected for processing (9)
- README.md (1 hunks)
- docs/en/deployment/distributed-deployment-guide.md (2 hunks)
- docs/en/design/apollo-design.md (1 hunks)
- docs/en/portal/apollo-user-guide.md (2 hunks)
- docs/zh/README.md (1 hunks)
- docs/zh/deployment/distributed-deployment-guide.md (1 hunks)
- docs/zh/design/apollo-design.md (1 hunks)
- docs/zh/design/apollo-introduction.md (1 hunks)
- docs/zh/portal/apollo-user-guide.md (3 hunks)
Files skipped from review due to trivial changes (1)
- docs/en/design/apollo-design.md
Additional context used
Markdownlint
docs/zh/README.md
43-43: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
44-44: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
45-45: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
docs/zh/design/apollo-introduction.md
73-73: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
74-74: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
73-73: Column: 1
Hard tabs(MD010, no-hard-tabs)
74-74: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/zh/portal/apollo-user-guide.md
132-132: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
136-136: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
docs/en/portal/apollo-user-guide.md
142-142: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
146-146: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
README.md
46-46: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
47-47: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
48-48: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
LanguageTool
docs/en/portal/apollo-user-guide.md
[formatting] ~146-~146: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...aml, txt and other formats configuration, because the storage of content-value storage, s...(COMMA_BEFORE_BECAUSE)
[style] ~501-~501: Consider a shorter alternative to avoid wordiness.
Context: ...ith the corresponding value is used in. In order to prevent memory overflow (OOM) problems ...(IN_ORDER_TO_PREMIUM)
Additional comments not posted (14)
docs/zh/design/apollo-design.md (1)
139-139: LGTM!The updated description of the Admin Service provides a more comprehensive and accurate overview of its functionalities by explicitly mentioning the configuration retrieval interfaces.
docs/zh/design/apollo-introduction.md (1)
72-75: Content changes look good!The newly added section clearly describes the global search feature for configuration items and how it benefits the users. The content is relevant and fits well in the introduction document.
Tools
Markdownlint
73-73: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
74-74: Expected: 2; Actual: 1
Unordered list indentation(MD007, ul-indent)
73-73: Column: 1
Hard tabs(MD010, no-hard-tabs)
74-74: Column: 1
Hard tabs(MD010, no-hard-tabs)
docs/zh/portal/apollo-user-guide.md (5)
126-129: LGTM!The new section "1.7 配置查询（管理员权限）" provides useful information about the global search functionality for administrators.
130-138: LGTM!The description provides a clear explanation of the fuzzy search functionality. The examples cover different configuration formats.
Tools
Markdownlint
132-132: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
136-136: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
243-245: The comment from the previous review is still valid. Skipping further comments on this code segment.
465-468: LGTM!The instructions provide clear steps for project administrators to configure access keys.
472-484: LGTM!The new section "6.3 全局搜索配置项的系统参数设置" provides essential information about the global search functionality and the related system parameter
apollo.portal.search.perEnvMaxResults. The content is clear and includes important considerations for administrators.docs/en/portal/apollo-user-guide.md (1)
136-149: LGTM!The code changes are approved.
Tools
LanguageTool
[formatting] ~146-~146: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...aml, txt and other formats configuration, because the storage of content-value storage, s...(COMMA_BEFORE_BECAUSE)
Markdownlint
142-142: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
146-146: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
README.md (2)
46-48: LGTM!The new "Global Search Configuration Items" section provides a clear and concise description of the new search functionality and its benefits for administrators and SREs. The changes are approved.
Tools
Markdownlint
46-46: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
47-47: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
48-48: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
Line range hint
1-45: LGTM!The restructured sections enhance the clarity and readability of the documentation while maintaining the overall flow and emphasis on key features. The changes are approved.
Also applies to: 49-1000
Tools
LanguageTool
[uncategorized] ~52-~52: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... logs for easy tracking of problems * Client side configuration information monitoring ...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~53-~53: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ation information monitoring** * It's very easy to see which instances are using the co...(EN_WEAK_ADJECTIVE)
[uncategorized] ~58-~58: The official spelling of this programming framework is “Node.js”.
Context: ...re also available, e.g. Golang, Python, NodeJS, PHP, C, etc * Open platform API ...(NODE_JS)
[style] ~58-~58: In American English, abbreviations like “etc.” require a period.
Context: ...e, e.g. Golang, Python, NodeJS, PHP, C, etc * Open platform API * Apollo itse...(ETC_PERIOD)
[misspelling] ~60-~60: This word is normally spelled as one.
Context: ...erface, which supports features such as multi-environment, multi-data center configuration manage...(EN_COMPOUNDS_MULTI_ENVIRONMENT)
[style] ~67-~67: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...pendency is MySQL, so the deployment is very simple. Apollo can run as long as Java and MyS...(EN_WEAK_ADJECTIVE)
Markdownlint
37-37: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
38-38: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
39-39: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
40-40: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
41-41: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
42-42: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
43-43: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
44-44: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
45-45: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
46-46: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
47-47: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
48-48: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
49-49: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
50-50: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
51-51: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
52-52: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
53-53: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
54-54: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
55-55: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
56-56: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
57-57: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
58-58: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
59-59: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
60-60: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
61-61: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
62-62: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
63-63: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
64-64: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
65-65: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
66-66: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
67-67: Expected: dash; Actual: asterisk
Unordered list style(MD004, ul-style)
docs/zh/deployment/distributed-deployment-guide.md (1)
1395-1402: Changes look good!The new configuration item
apollo.portal.search.perEnvMaxResultsis clearly documented with a reasonable default value. The guidance on adjusting the value based on business needs and available system resources is helpful.docs/en/deployment/distributed-deployment-guide.md (3)
772-774: LGTM!The code changes are approved. The added tip provides helpful guidance for customizing service URIs in intranet deployments.
1450-1457: LGTM!The code changes are approved. The new configuration item
apollo.portal.search.perEnvMaxResultsis clearly documented with helpful details on usage, default value, performance considerations, and recommendations.
1452-1453: LGTM!The code changes are approved. The version information for the new configuration item is correctly specified.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Outdated
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/ReleaseRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
docs/en/images/Configuration query-Non properties.pngis excluded by!**/*.pngdocs/en/images/Configuration query-properties.pngis excluded by!**/*.pngdocs/zh/images/Configuration query-Non properties.pngis excluded by!**/*.pngdocs/zh/images/Configuration query-properties.pngis excluded by!**/*.png
Files selected for processing (16)
- CHANGES.md (1 hunks)
- apollo-adminservice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java (7 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (3 hunks)
- apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java (2 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java (1 hunks)
- apollo-portal/src/main/resources/static/global_search_value.html (1 hunks)
- apollo-portal/src/main/resources/static/i18n/en.json (1 hunks)
- apollo-portal/src/main/resources/static/i18n/zh-CN.json (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java (1 hunks)
- changes/changes-2.4.0.md (1 hunks)
- docs/en/portal/apollo-user-guide.md (2 hunks)
- docs/zh/portal/apollo-user-guide.md (3 hunks)
Files skipped from review due to trivial changes (2)
- apollo-portal/src/main/resources/static/i18n/en.json
- changes/changes-2.4.0.md
Files skipped from review as they are similar to previous changes (7)
- apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/ItemServiceTest.java
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/dto/ItemInfoDTO.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueController.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
- apollo-portal/src/main/resources/static/global_search_value.html
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchValueControllerTest.java
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueServiceTest.java
Additional context used
Markdownlint
docs/zh/portal/apollo-user-guide.md
132-132: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
136-136: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
docs/en/portal/apollo-user-guide.md
142-142: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
146-146: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
LanguageTool
docs/en/portal/apollo-user-guide.md
[formatting] ~146-~146: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...aml, txt and other formats configuration, because the storage of content-value storage, s...(COMMA_BEFORE_BECAUSE)
[style] ~501-~501: Consider a shorter alternative to avoid wordiness.
Context: ...ith the corresponding value is used in. In order to prevent memory overflow (OOM) problems ...(IN_ORDER_TO_PREMIUM)
Additional comments not posted (9)
CHANGES.md (2)
10-10: Documentation of new feature is clear and well-linked.The addition of the global search feature to the release notes is well-documented and includes a direct link to the pull request, which provides additional context. This is a good practice for transparency and traceability.
13-13: Formatting adjustment enhances accessibility.Updating the link to point directly to the closed issues and pull requests under the specific milestone improves navigation and user experience by providing a centralized view of all related discussions and resolutions.
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/vo/ItemInfo.java (1)
89-98: Consider usingStringBuilderfortoStringmethod.Using
StringBuildercan improve performance, especially if the method is called frequently. This suggestion was previously made and is still applicable.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java (1)
37-37: Addition ofReleaseRepositoryto constructorThe constructor of
ItemServicehas been updated to includeReleaseRepositoryas a new parameter. This change is significant as it allows the service to interact with release-related data, enhancing its functionality.The addition is well-integrated and follows proper dependency injection principles using Spring's
@Autowired(implicit in this case). Ensure that all uses of this service in the codebase are updated to pass the new parameter.docs/zh/portal/apollo-user-guide.md (2)
126-139: Review of new section "1.7 配置查询（管理员权限）"This new section provides detailed instructions on how administrators can use the global search functionality to query configuration items. The content is clear and well-structured, providing step-by-step guidance which aligns with the PR objectives to enhance search capabilities.
The addition of this section improves the documentation by explaining the new feature comprehensively. Ensure that the screenshots are up-to-date and accurately reflect the current UI.
Tools
Markdownlint
132-132: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
136-136: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
474-484: System Parameterization of Global Search Configuration ItemsThe new section "6.3 全局搜索配置项的系统参数设置" explains the introduction of a system parameter to limit search results, which is crucial for preventing performance issues. The explanation is clear and the instructions for setting the parameter are straightforward.
This section is a valuable addition to the documentation as it addresses potential performance concerns with the new global search feature. It's important to test these settings under load to ensure they meet the system's needs without degrading user experience.
docs/en/portal/apollo-user-guide.md (2)
136-149: Review of new section "1.7 Configuration queries (administrator privileges)"This new section provides detailed instructions on how administrators can use the global search functionality to query configuration items. The content is clear and well-structured, providing step-by-step guidance which aligns with the PR objectives to enhance search capabilities.
The addition of this section improves the documentation by explaining the new feature comprehensively. Ensure that the screenshots are up-to-date and accurately reflect the current UI.
Tools
Markdownlint
142-142: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
146-146: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
LanguageTool
[formatting] ~146-~146: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...aml, txt and other formats configuration, because the storage of content-value storage, s...(COMMA_BEFORE_BECAUSE)
499-511: System Parameterization of Global Search Configuration ItemsThe new section "6.3 System parameterization of global search configuration items" explains the introduction of a system parameter to limit search results, which is crucial for preventing performance issues. The explanation is clear and the instructions for setting the parameter are straightforward.
This section is a valuable addition to the documentation as it addresses potential performance concerns with the new global search feature. It's important to test these settings under load to ensure they meet the system's needs without degrading user experience.
Tools
LanguageTool
[style] ~501-~501: Consider a shorter alternative to avoid wordiness.
Context: ...ith the corresponding value is used in. In order to prevent memory overflow (OOM) problems ...(IN_ORDER_TO_PREMIUM)
apollo-portal/src/main/resources/static/i18n/zh-CN.json (1)
897-919: Review of Localization Key-Value Pairs for Global Search FunctionalityThe added key-value pairs are correctly formatted and seem appropriate for the intended functionality of global search within the Apollo admin tool. Here are some specific observations:
- General Appropriateness: The translations provided match the intended functionality described in the PR summary, such as "Value的全局搜索" for "Global.Title" which aligns with the global search for Value values.
- Consistency: The keys are consistently prefixed, which is good for maintaining a structured namespace within the localization files.
- Coverage: The keys cover various aspects of the UI that will interact with the global search feature, including error messages, tips, and pagination, which is comprehensive.
Overall, the changes are well-implemented and should provide the necessary localization support for the new global search functionality.
...ice/src/test/java/com/ctrip/framework/apollo/adminservice/controller/ItemControllerTest.java
Show resolved
Hide resolved
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/ItemService.java
Show resolved
Hide resolved
...portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
Outdated
Show resolved
Hide resolved
...portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchValueService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchService.java (1 hunks)
- apollo-portal/src/main/resources/static/global_search_value.html (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchControllerTest.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchServiceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apollo-portal/src/main/resources/static/global_search_value.html
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Additional comments not posted (11)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchService.java (2)
32-56: LGTM!The
GlobalSearchServiceclass is properly structured and annotated as a Spring service. The constructor correctly injects the required dependency.
42-54: LGTM!The
getPerEnvItemInfoBySearchmethod is implemented correctly. It retrieves the paginated data from the API, converts it to the required format, and returns aPageDTO. The error logging is also handled properly.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchServiceTest.java (2)
55-68: LGTM!The test method is well-written and covers the happy path scenario. It verifies that the
globalSearchService.getPerEnvItemInfoBySearchmethod returns the expected results when theitemAPI.getPerEnvItemInfoBySearchmethod returns a non-emptyPageDTO.
70-76: LGTM!The test method is well-written and covers the scenario where the search returns no results. It verifies that the
globalSearchService.getPerEnvItemInfoBySearchmethod returns an emptyPageDTOwhen theitemAPI.getPerEnvItemInfoBySearchmethod returns an emptyPageDTO.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java (1)
42-52: LGTM!The
GlobalSearchControllerclass is properly annotated with@RestControllerand the dependencies (PortalSettings,GlobalSearchService, andPortalConfig) are injected using constructor injection, which is a good practice.apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchControllerTest.java (6)
1-16: LGTM!The Apache License 2.0 header is correctly formatted with the appropriate license text and copyright year.
77-85: LGTM!The test method correctly covers the scenario when both key and value search parameters are empty, expecting a bad request status and a specific error message in the response.
87-96: LGTM!The test method correctly covers the scenario when there are no active environments, expecting a bad request status and a specific error message in the response.
98-116: LGTM!The test method correctly covers the scenario when there are active environments but the search returns no results, mocking the service method to return an empty page and expecting an OK status with a specific JSON response containing an empty data array and
hasMoreDataset to false.
118-148: LGTM!The test method correctly covers the scenario when there are active environments and the search returns results, but the number of results exceeds the per-environment limit. It mocks the service method to return pages with a total count exceeding the limit for each environment and expects an OK status with a specific JSON response containing the combined data from all environments,
hasMoreDataset to true, and a message indicating that the limit was exceeded.
150-177: LGTM!The test method correctly covers the scenario when there are active environments and the search returns results within the per-environment limit. It mocks the service method to return pages with a total count within the limit for each environment and expects an OK status with a specific JSON response containing the combined data from all environments and
hasMoreDataset to false.
| @PreAuthorize(value = "@permissionValidator.isSuperAdmin()") | ||
| @GetMapping("/global-search/item-info/by-key-or-value") | ||
| public ResponseEntity<?> getItemInfoBySearch(@RequestParam(value = "key", required = false, defaultValue = "") String key, | ||
| @RequestParam(value = "value", required = false , defaultValue = "") String value) { | ||
|
|
||
| if(key.isEmpty() && value.isEmpty()) { | ||
| return ResponseEntity | ||
| .status(HttpStatus.BAD_REQUEST) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(new Gson().toJson("Please enter at least one search criterion in either key or value.")); | ||
| } | ||
|
|
||
| List<Env> activeEnvs = portalSettings.getActiveEnvs(); | ||
| if(activeEnvs.isEmpty()){ | ||
| return ResponseEntity | ||
| .status(HttpStatus.BAD_REQUEST) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(new Gson().toJson("Request accepted. Looking for available admin service.")); | ||
|
|
||
| } | ||
|
|
||
| List<ItemInfo> allEnvItemInfos = new ArrayList<>(); | ||
| List<String> envBeyondLimit = new ArrayList<>(); | ||
| AtomicBoolean hasMoreData = new AtomicBoolean(false); | ||
| activeEnvs.forEach(env -> { | ||
| PageDTO<ItemInfo> perEnvItemInfos = globalSearchService.getPerEnvItemInfoBySearch(env, key, value,0, portalConfig.getPerEnvSearchMaxResults()); | ||
| if (!perEnvItemInfos.hasContent()) { | ||
| return; | ||
| } | ||
| if(perEnvItemInfos.getTotal() > portalConfig.getPerEnvSearchMaxResults()){ | ||
| envBeyondLimit.add(env.getName()); | ||
| hasMoreData.set(true); | ||
| } | ||
| allEnvItemInfos.addAll(perEnvItemInfos.getContent()); | ||
| }); | ||
|
|
||
| Map<String, Object> body = new HashMap<>(); | ||
| if(hasMoreData.get()){ | ||
| body.put("data", allEnvItemInfos); | ||
| body.put("hasMoreData", true); | ||
| body.put("message", String.format( | ||
| "In %s , more than %d items found (Exceeded the maximum search quantity for a single environment). Please enter more precise criteria to narrow down the search scope.", | ||
| String.join(" , ", envBeyondLimit), portalConfig.getPerEnvSearchMaxResults())); | ||
| return ResponseEntity | ||
| .ok() | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(body); | ||
| } | ||
|
|
||
| body.put("data", allEnvItemInfos); | ||
| body.put("hasMoreData", false); | ||
| return ResponseEntity | ||
| .ok() | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .body(body); | ||
| } |
There was a problem hiding this comment.
Refactor the method to improve readability, error handling, and response structure.
The getItemInfoBySearch method is well-structured and follows the required logic for performing the global search across environments. However, consider the following improvements:
-
Break down the method into smaller, more focused methods to improve readability and maintainability. For example, you can extract the logic for iterating over environments and aggregating search results into separate methods.
-
Include proper error handling and response for unauthorized access. Although the
@PreAuthorizeannotation is used for authorization, there is no explicit handling of unauthorized access. Consider adding a custom exception handler to return an appropriate error response in case of unauthorized access. -
Replace the
envBeyondLimitlist andhasMoreDataflag with a more descriptive data structure, such as a map of environment names to their respective search result counts. This would provide more information in the response and make it easier to understand the search results per environment. -
Improve the response structure by using a more standardized format, such as a common response object with fields for data, pagination, and messages. This would make the API more consistent and easier to consume by clients.
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| $scope.totalItems = allItemInfo.length; | ||
| $scope.allItemInfo = allItemInfo; | ||
| $scope.totalPages = Math.ceil($scope.totalItems / parseInt($scope.pageSize, 10)); | ||
| const startIndex = ($scope.currentPage - 1) * parseInt($scope.pageSize, 10); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| }else{ | ||
| $scope.needToBeHighlightedKey = $scope.itemInfoSearchKey; | ||
| $scope.needToBeHighlightedValue = $scope.itemInfoSearchValue; | ||
| result.body.forEach((itemInfo, index) => { |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5182 +/- ##
============================================
- Coverage 50.30% 50.27% -0.04%
- Complexity 2013 2038 +25
============================================
Files 388 393 +5
Lines 12228 12359 +131
Branches 1209 1213 +4
============================================
+ Hits 6151 6213 +62
- Misses 5712 5777 +65
- Partials 365 369 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchService.java (1)
46-75: LGTM! The method implementation looks good.The
getAllEnvItemInfoBySearchmethod enhances the Apollo portal's functionality by enabling users to perform searches for items across different environments. It improves the usability and accessibility of configuration data.Some observations and suggestions:
The method handles potential exceptions during the conversion of
ItemInfoDTOtoItemInfo, ensuring a smooth execution. Good job!It provides informative messages to the user when the search results exceed the specified limit, which is helpful.
Consider extracting the logic for converting
ItemInfoDTOtoItemInfointo a separate method for better readability and reusability.You can use a
StreamandCollectors.toList()instead of aforEachloop to convert the list ofItemInfoDTOtoItemInfo. This can make the code more concise and readable.Consider adding some unit tests to verify the behavior of this method under different scenarios, such as when there are no results, when the results exceed the limit, etc.
Overall, great work on implementing this feature!
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/GlobalSearchService.java (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchControllerTest.java (1 hunks)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchServiceTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchControllerTest.java
Additional context used
GitHub Check: codecov/patch
apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java
[warning] 21-21: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L21
Added line #L21 was not covered by tests
[warning] 29-34: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L29-L34
Added lines #L29 - L34 were not covered by tests
[warning] 38-43: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L38-L43
Added lines #L38 - L43 were not covered by tests
[warning] 47-50: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L47-L50
Added lines #L47 - L50 were not covered by tests
[warning] 54-54: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L54
Added line #L54 was not covered by tests
[warning] 58-58: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L58
Added line #L58 was not covered by tests
[warning] 62-62: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L62
Added line #L62 was not covered by tests
[warning] 65-65: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L65
Added line #L65 was not covered by testsapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java
[warning] 48-48: apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java#L48
Added line #L48 was not covered by tests
Additional comments not posted (12)
apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java (2)
21-26: LGTM!The class declaration and fields are well-structured and encapsulate the necessary information for a search response entity.
Tools
GitHub Check: codecov/patch
[warning] 21-21: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L21
Added line #L21 was not covered by tests
21-67: The class design is clear, modular, and follows best practices.The
SearchResponseEntityclass provides a structured way to represent HTTP response entities. It encapsulates the necessary information for a search response entity, including the response body, status code, message, and a flag indicating whether more data is available.The use of a generic type parameter
<T>enhances the flexibility and reusability of the class, allowing it to handle different types of response bodies.The static factory methods
ok,okWithMessage, anderrorprovide a clean and intuitive API for creating response entities with different states. They encapsulate the logic of setting the appropriate status codes, messages, and flags, making it easy to create consistent and meaningful response entities.The getter methods
getCode,getMessage,getBody, andisHasMoreDataprovide convenient access to the response information, following the standard naming conventions.Overall, the class design is well-thought-out, modular, and adheres to best practices. It enhances the clarity and usability of HTTP responses in the application.
Tools
GitHub Check: codecov/patch
[warning] 21-21: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L21
Added line #L21 was not covered by tests
[warning] 29-34: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L29-L34
Added lines #L29 - L34 were not covered by tests
[warning] 38-43: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L38-L43
Added lines #L38 - L43 were not covered by tests
[warning] 47-50: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L47-L50
Added lines #L47 - L50 were not covered by tests
[warning] 54-54: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L54
Added line #L54 was not covered by tests
[warning] 58-58: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L58
Added line #L58 was not covered by tests
[warning] 62-62: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L62
Added line #L62 was not covered by tests
[warning] 65-65: apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java#L65
Added line #L65 was not covered by testsapollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java (2)
32-40: LGTM!The
GlobalSearchControllerclass follows the Spring REST controller conventions and adheres to the dependency inversion principle by injecting its dependencies through the constructor. The class structure and dependencies look good.
42-52: Solid implementation of the search endpoint!The
getItemInfoBySearchmethod is well-structured and follows the required logic for performing the global search based on the provided key and value. The use of@PreAuthorizeensures proper authorization, and the method handles the case when both search parameters are empty by throwing aBadRequestExceptionwith a clear error message.The integration with
globalSearchServiceand the use of pagination based onportalConfigare good design choices that promote separation of concerns and efficient resource utilization.Overall, the method implementation looks solid and adheres to best practices.
Tools
GitHub Check: codecov/patch
[warning] 48-48: apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java#L48
Added line #L48 was not covered by testsapollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/GlobalSearchServiceTest.java (3)
66-89: LGTM!The test method
testGet_PerEnv_ItemInfo_BySearch_withKeyAndValue_ReturnExpectedItemInfosis well-written and covers the scenario where the search returns expected item infos for the given key and value across multiple environments.The test setup is correct, mocking the required dependencies and setting up the expected responses. The assertions verify that the service method returns the expected
ItemInfoobjects for each environment and also checks the number of invocations of the mocked methods.Good job on writing a comprehensive test case!
91-99: LGTM!The test method
testGet_PerEnv_ItemInfo_withKeyAndValue_BySearch_ReturnEmptyItemInfosis well-written and covers the scenario where the search returns empty item infos for a non-existent key and value.The test setup is correct, mocking the required dependencies and setting up the expected empty response. The assertion verifies that the service method returns an empty list of
ItemInfoobjects when the search criteria do not match any items.Good job on writing a test case for the negative scenario!
101-125: LGTM!The test method
testGet_PerEnv_ItemInfo_BySearch_withKeyAndValue_ReturnExpectedItemInfos_ButOverPerEnvLimitis well-written and covers the scenario where the search returns expected item infos for the given key and value across multiple environments, but the number of items exceeds the per-environment limit.The test setup is correct, mocking the required dependencies and setting up the expected responses with a total count exceeding the per-environment limit. The assertions verify that the service method returns the expected
ItemInfoobjects for each environment and also checks the presence of the message indicating the need to refine search criteria.Good job on writing a test case for the scenario where the search results exceed the per-environment limit!
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js (5)
17-20: Controller initialization looks good!The controller is properly defined using the AngularJS
module.controllersyntax, and the dependencies are correctly injected using the array notation. The function signature matches the injected services.
22-41: Scope variable initializations are appropriate.The scope variables are initialized with suitable default values based on their intended purpose. The naming convention is consistent and descriptive, making the code more readable and maintainable.
65-136:getItemInfoByKeyAndValuefunction is well-structured and handles data fetching effectively.The function follows a clear pattern for fetching item information based on the provided key and value. It resets the necessary scope variables before making the API call and updates them accordingly in the success callback. The error scenario is also handled gracefully.
142-159:changePagefunction handles pagination effectively.The function validates the requested page number and updates the current page and related scope variables correctly. It calculates the start and end indexes for the current page items and updates the displayed items accordingly. The call to
getPagesArrayensures that the page range is updated as well.
263-267:highlightKeywordfunction effectively highlights the keyword.The function uses a regular expression to find and replace the keyword occurrences with a
<span>element. It applies a yellow background color and padding to visually highlight the keyword. The check for an empty keyword avoids unnecessary processing.
apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java
Show resolved
Hide resolved
apollo-common/src/main/java/com/ctrip/framework/apollo/common/http/SearchResponseEntity.java
Show resolved
Hide resolved
...ortal/src/main/java/com/ctrip/framework/apollo/portal/controller/GlobalSearchController.java
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
apollo-portal/src/main/resources/static/scripts/controller/GlobalSearchValueController.js
Show resolved
Hide resolved
|
PR 审核通过
|
在管理员工具处添加了Value的全局搜索功能，使得能够快速、便捷的找到哪个资源（例如：Redis、MySQL等）在哪个项目中使用。
Summary by CodeRabbit
New Features
Documentation