-
Notifications
You must be signed in to change notification settings - Fork 53
feat: Service extends IService #242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update migrates the project from Spring MVC to Spring WebFlux, replacing relevant dependencies and configuration interfaces. All service interfaces now extend MyBatis-Plus's Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Service (extends ServiceImpl)
participant baseMapper (MyBatis-Plus)
participant Database
Controller->>Service: Call serviceMethod(id)
Service->>baseMapper: baseMapper.method(id)
baseMapper->>Database: Execute SQL
Database-->>baseMapper: Return result
baseMapper-->>Service: Return entity/result
Service-->>Controller: Return response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🔭 Outside diff range comments (13)
base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformHistoryServiceImpl.java (2)
80-84
: 🛠️ Refactor suggestionReturn value check can silently swallow DB errors
deleteResult != 1
→CM008
;==1
→ success.
If the record did not exist (0
affected rows) the caller gets generic “success” (line 78) which hides the fact that nothing was deleted. Consider distinguishing:-if (deleteResult != 1) { ... } +if (deleteResult == 0) { + return Result.failed(ExceptionEnum.DATA_NOT_FOUND); +}This yields clearer feedback to API consumers.
98-104
: 🛠️ Refactor suggestionSame silent-failure pattern on update
A
0
-row update currently maps toCM008
(generic SQL fail).
Return a “not found” / “concurrent update” code instead so clients can react properly.base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java (1)
75-85
: 🛠️ Refactor suggestionMark write operations
@Transactional
to ensure atomicity
deletePlatformById
,updatePlatformById
, andcreatePlatform
each perform a read-then-write (or write-then-read) sequence.
Without@Transactional
, a failure or concurrent update between the two statements can leave the service returning stale or inconsistent data.
Annotating the methods (or the whole class) with@Transactional(rollbackFor = Exception.class)
will guarantee the read/write pair participates in one DB transaction.@Override -public Result<Platform> deletePlatformById(Integer id) { +@Transactional(rollbackFor = Exception.class) +public Result<Platform> deletePlatformById(Integer id) { ... } @Override -public Result<Platform> updatePlatformById(Platform platform) { +@Transactional(rollbackFor = Exception.class) +public Result<Platform> updatePlatformById(Platform platform) { ... } @Override -public Result<Platform> createPlatform(Platform platform) { +@Transactional(rollbackFor = Exception.class) +public Result<Platform> createPlatform(Platform platform) { ... }Also applies to: 94-104, 112-124
base/src/main/java/com/tinyengine/it/service/app/I18nEntryService.java (1)
35-97
: 🛠️ Refactor suggestionLarge interface surface after extending
IService
This interface now mixes:
- Generic CRUD (already covered by
IService<I18nEntry>
).- High-level domain operations (batch upload, file parsing, formatting).
Consider splitting into two interfaces:
I18nEntryCrudService extends IService<I18nEntry>
I18nEntryDomainService
with the higher-level methodsThis adheres to the Interface Segregation Principle, eases testing, and clarifies responsibilities.
base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
84-97
: 🛠️ Refactor suggestionAdd transaction boundaries for write operations.
updateUserById
andcreateUser
mutate state but are not annotated with@Transactional
, so partial updates could escape rollback on exception.@@ @Override -public Integer updateUserById(User user) { - return baseMapper.updateUserById(user); +@Transactional(rollbackFor = Exception.class) +public Integer updateUserById(User user) { + return baseMapper.updateUserById(user); } @@ @Override -public Integer createUser(User user) { - return baseMapper.createUser(user); +@Transactional(rollbackFor = Exception.class) +public Integer createUser(User user) { + return baseMapper.createUser(user); }base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java (3)
330-336
:⚠️ Potential issuePossible SQL-injection through manual OR-clause concatenation
labelsCondition
is built via raw string interpolation:String labelsCondition = block.stream() .map(name -> "label = '" + name + "'") .collect(Collectors.joining(" OR ")); wrapper.apply(labelsCondition);Any illegal character in
name
(e.g.' OR 1=1 --
) will be injected verbatim.Fix by using the fluent API instead of
apply
:- String labelsCondition = block.stream() - .map(name -> "label = '" + name + "'") - .collect(Collectors.joining(" OR ")); - queryWrapper.and(w -> w.apply(labelsCondition)); + queryWrapper.in("label", block);
590-598
: 🛠️ Refactor suggestionRedundant full fetch before paging – switch to
selectCount
selectList(queryWrapper)
is executed solely to obtain the record count, immediately followed byselectPage
.
That doubles the query cost for large tables.- List<Block> blocksList = baseMapper.selectList(queryWrapper); - Page<Block> page = new Page<>(1, blocksList.size()); - return baseMapper.selectPage(page, queryWrapper); + long total = baseMapper.selectCount(queryWrapper); + Page<Block> page = new Page<>(1, total); + return baseMapper.selectPage(page, queryWrapper);
702-709
:⚠️ Potential issueNull-pointer risk when
blockDto.getGroups()
is absent
blockDto.getGroups().stream()
will throw ifgetGroups()
returnsnull
(e.g. when a client omits the field).Guard the call:
- List<Integer> groups = blockDto.getGroups().stream() - .map(BlockGroup::getId).collect(Collectors.toList()); + List<Integer> groups = Optional.ofNullable(blockDto.getGroups()) + .orElse(Collections.emptyList()) + .stream() + .map(BlockGroup::getId) + .collect(Collectors.toList());base/src/main/java/com/tinyengine/it/service/material/impl/MaterialServiceImpl.java (1)
76-82
:⚠️ Potential issueDeleted record is fetched after deletion – returned payload will always be null
queryMaterialById
is executed after the row has been removed, so the API will returndata=null
, while the caller probably expects the deleted object (yourMaterialHistoryServiceImpl
does it the right way).- int deleteResult = baseMapper.deleteMaterialById(id); - if (deleteResult != 1) { - return Result.failed(ExceptionEnum.CM008); - } - return this.queryMaterialById(id); + // fetch first so we can echo the deleted entity + Material material = baseMapper.queryMaterialById(id); + int deleteResult = baseMapper.deleteMaterialById(id); + if (deleteResult != 1) { + return Result.failed(ExceptionEnum.CM008); + } + return Result.success(material);base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java (1)
177-185
:⚠️ Potential issueNull-pointer risk when page / app is not found
pageInfo
or theApp
returned byappMapper
can benull
. Calling methods on them without a guard (getApp()
,getFramework()
) will throwNullPointerException
, turning a “not found” case into HTTP 500.- Page pageInfo = baseMapper.queryPageById(id); + Page pageInfo = baseMapper.queryPageById(id); + if (pageInfo == null) { + throw new ServiceException(ExceptionEnum.CM005.getResultCode(), "Page not found"); + } String framework = appMapper.queryAppById(pageInfo.getApp()).getFramework(); - if (framework.isEmpty()) { + if (framework == null || framework.isEmpty()) { throw new ServiceException(ExceptionEnum.CM312.getResultCode(), ExceptionEnum.CM312.getResultMsg()); }base/src/main/java/com/tinyengine/it/service/app/impl/AppExtensionServiceImpl.java (2)
82-88
:⚠️ Potential issuePotential time-of-check / time-of-use race in delete flow.
Between
queryAppExtensionById(id)
anddeleteAppExtensionById(id)
another request could delete or modify the row, leading to a misleading success response (Result.success(appExtension)
) even though theDELETE
affected 0 rows.Mitigate by:
- Deleting first and checking
rowsAffected
; if 0 →CM009
.- Executing both statements inside one transaction (see previous comment).
116-124
: 🛠️ Refactor suggestionDuplicate-check and insert should be atomic.
queryAppExtensionByCondition
followed bycreateAppExtension
is vulnerable to race conditions. Use a DB unique constraint and rely on exception handling, or enclose both operations in a transaction with aSELECT … FOR UPDATE
.base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (1)
191-208
: 🛠️ Refactor suggestionN+1 query pattern – fetch groups in bulk instead of per-ID loop.
Looping over
ids
and invokingqueryBlockGroupAndBlockById
one-by-one incurs a DB round-trip per element. Replace with a single mapper method that accepts a collection:- for (int blockgroupId : ids) { - blockGroup = baseMapper.queryBlockGroupAndBlockById(blockgroupId, blockCreatedBy, groupCreatedBy); - blockGroupsListResult.add(blockGroup); - } + blockGroupsListResult = + baseMapper.queryBlockGroupAndBlockByIds(ids, blockCreatedBy, groupCreatedBy);This will significantly improve latency when
ids.size()
is large.
♻️ Duplicate comments (6)
base/src/main/java/com/tinyengine/it/service/material/MaterialService.java (2)
26-26
: Same note asTaskRecordService
: duplicated CRUD afterIService
IService<Material>
already offers generic CRUD methods.
40-41
:@Param
removal can break mapper XML – verify build flagsSee earlier comment in
TaskRecordService
.Also applies to: 56-57
base/src/main/java/com/tinyengine/it/service/app/PageService.java (2)
28-28
: Duplicated CRUD afterIService
The interface now exposes both
getById
(viaIService
) and customqueryPageById
. Harmonise or remove duplicates to avoid API confusion.
43-44
: Potential MyBatis parameter-name mismatchDropping
@Param("id")
bears the same runtime risk described earlier.Also applies to: 59-60
base/src/main/java/com/tinyengine/it/service/material/MaterialHistoryService.java (2)
26-26
: Redundant custom CRUD after adoptingIService
Same rationale as previous service interfaces.
40-41
: Verify MyBatis XML after@Param
removalSame concern as noted for the other services.
Also applies to: 56-57
🧹 Nitpick comments (40)
base/src/main/java/com/tinyengine/it/service/platform/TenantService.java (1)
25-55
: ExtendingIService
but still redefining CRUD-like APIs – consider consolidationNow that the interface inherits MyBatis-Plus
IService<Tenant>
, common methods such asgetById
,removeById
,updateById
,save
, andlist
are already available.
Keeping custom equivalents (findTenantById
,deleteTenantById
, etc.) can confuse clients and inflate maintenance cost.If no domain-specific behaviour is added, consider:
-public interface TenantService extends IService<Tenant> { - ... - Tenant findTenantById(Integer id); - ... - Integer deleteTenantById(Integer id); - ... -} +public interface TenantService extends IService<Tenant> { + // Add only domain-specific queries that are NOT covered by IService +}Otherwise, document clearly why both variants coexist.
base/src/main/java/com/tinyengine/it/service/platform/PlatformService.java (1)
26-56
: Duplicate CRUD surface after inheritingIService<Platform>
Same observation as in
TenantService
:queryPlatformById
,deletePlatformById
, etc., overlap withIService
defaults, with extra wrapping intoResult<T>
.
Decide either to:
- Fully rely on
IService
and drop custom methods, or- Keep the richer
Result<T>
contract but remove/alias the redundant reads (queryPlatformById
vsgetById
) to avoid two ways of doing the same thing.Aligning method names will ease onboarding and reduce cognitive load.
base/src/main/java/com/tinyengine/it/service/platform/PlatformHistoryService.java (1)
26-56
: Interface inheritsIService
but still exposes CRUD methodsAs with the other services, decide whether these custom methods add value over
IService
defaults; otherwise trim for clarity or rename to emphasise domain semantics (e.g.,archiveHistory(...)
vscreate
).base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java (2)
52-54
:baseMapper.queryTenantById
duplicatesgetById
Since
ServiceImpl
already offersgetById(id)
, consider delegating to it or removing the wrapper unless extra logic is injected.-return baseMapper.queryTenantById(id); +return this.getById(id);
85-98
: Consider leveragingsaveOrUpdate
fromServiceImpl
Manual
baseMapper.updateTenantById
/createTenant
pairs can be replaced with the framework’ssaveOrUpdate(tenant)
, reducing boilerplate and centralising optimistic-lock handling (if enabled).base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java (2)
43-66
: Prefer the built-in CRUD helpers to cut duplication
queryAllPlatform
,queryPlatformById
, andqueryPlatformByCondition
still delegate straight tobaseMapper
.
If these methods merely wrap standardlist()
,getById(id)
or a simple lambda query, you can delete them altogether and expose the inheritedlist
/getById
fromServiceImpl
.
This keeps the service layer thinner and avoids maintaining duplicate SQL inPlatformMapper
.-@Override -public List<Platform> queryAllPlatform() { - return baseMapper.queryAllPlatform(); -} +// Callers can now just use platformService.list();
115-119
: Stricter name validation
platform.getName().isEmpty()
treats strings containing only whitespace as valid.
Switching toStringUtils.hasText(platform.getName())
(Spring) orStringUtils.isNotBlank
(Apache Commons) closes that gap.-if (platform.getName() == null || platform.getName().isEmpty()) { +if (!StringUtils.hasText(platform.getName())) { return Result.failed(ExceptionEnum.CM002); }app/src/main/java/com/tinyengine/it/config/filter/WebConfig.java (1)
33-49
: Nit: typo in variable name
crosDomainList
→corsDomainList
to keep naming consistent with the CORS acronym.base/src/main/java/com/tinyengine/it/service/material/ComponentLibraryService.java (1)
26-72
: Redundant CRUD methods after adoptingIService
By extending
IService<ComponentLibrary>
you already inherit a full CRUD API (getById
,removeById
,updateById
,save
,list
, etc.).
Keeping additional methods (queryComponentLibraryById
,deleteComponentLibraryById
,updateComponentLibraryById
,createComponentLibrary
) that merely wrap the same semantics introduces duplication and cognitive overhead.If the intent is to add domain-specific behaviour (e.g. wrapping the result in
Result<>
), consider:
- Renaming the wrapper methods to clarify the added value (e.g.
getWithValidation
).- Or drop them and let callers use the inherited MyBatis-Plus methods directly.
Cleaning this up will shrink the service interface surface and make the codebase more consistent with other modules that rely solely on
IService
.base/src/main/java/com/tinyengine/it/service/app/UserService.java (2)
25-56
: Consolidate CRUD to the inheritedIService
APISame observation as for
ComponentLibraryService
:queryUserById
,deleteUserById
, etc. overlap withgetById
,removeById
,list()
provided byIService<User>
.Unless these methods deliver extra behaviour (e.g. audit, validation,
Result<>
wrapping), consider deleting them and relying on the default API to avoid bloat and maintenance friction.
31-39
: Naming consistency: plural vs singularMethod
queryAllUser()
mixes plural semantics with a singular noun.
Recommend renaming toqueryAllUsers()
(or simplylistUsers()
if retained).base/src/main/java/com/tinyengine/it/service/material/ComponentService.java (2)
32-70
: Consider eliminating duplicatedfind*/delete*
methodsAfter extending
IService<Component>
the interface already exposes:
getById(id)
➜findComponentById
removeById(id)
➜deleteComponentById
list()
➜findAllComponent
updateById(entity)
➜updateComponentById
save(entity)
➜createComponent
Unless you need the custom return types (
Result<>
) or additional processing, drop the duplicates to keep the contract minimal.
62-63
: Return type semantics
deleteComponentById
returnsInteger
, presumably the affected row count.
MyBatis-Plus’sremoveById
returnsboolean
, which is clearer for consumers.
If you keep the custom method, consider switching toboolean
to express intent unambiguously.base/src/main/java/com/tinyengine/it/service/material/TaskRecordService.java (1)
28-36
:queryTaskRecordById
duplicatesgetById
Given the new
IService<TaskRecord>
parent,getById(id)
is available out-of-the-box.
Please remove or repurposequeryTaskRecordById
to avoid two ways of doing the same thing.base/src/main/java/com/tinyengine/it/service/app/I18nEntryService.java (1)
35-50
: Camel-case inconsistency & typoMethod
readFilesAndbulkCreate
has a lowercase “b” in “bulk”.
Recommend renaming toreadFilesAndBulkCreate
for standard camel-case formatting.base/src/main/java/com/tinyengine/it/service/app/I18nLangService.java (1)
15-26
: Redundant CRUD declarations now thatIService
is inherited.
IService<I18nLang>
already exposeslist()
,getById()
,removeById()
,save()
,updateById()
…
Keeping custom aliases (queryAllI18nLang
,deleteI18nLangById
…) creates two parallel APIs for the same thing and inflates the surface area.Consider:
- Drop the duplicated methods and rely on the generic ones; or
- Keep the methods but mark them
@Deprecated
and delegate to generic calls.This will reduce maintenance cost and confusion for future contributors.
base/src/main/java/com/tinyengine/it/service/app/AppExtensionService.java (1)
15-27
: Same duplication concern as above – streamline againstIService
.The interface now inherits full CRUD; reevaluate whether
findAppExtensionById
,deleteAppExtensionById
, etc. need to stay. If they provide extra behaviour (e.g. wrappingResult<>
), document the difference clearly to avoid accidental misuse.base/src/main/java/com/tinyengine/it/service/app/AppService.java (1)
29-29
: Avoid redundant CRUD wrappers after inheritingIService
IService<App>
already offers generic CRUD operations (list()
,getById()
,removeById()
, etc.).
Keeping bespoke equivalents (queryAllApp
,queryAppById
,deleteAppById
, …) inflates the API surface and sends mixed signals to callers.Consider either
-interface AppService extends IService<App> { +interface AppService extends IService<App> { + // keep only domain-specific methods …or drop the
IService
inheritance if you need fully customized semantics.base/src/main/java/com/tinyengine/it/service/material/BlockService.java (1)
34-34
: Duplicate CRUD contractsExtending
IService<Block>
while still declaringqueryBlockById
,deleteBlockById
, etc., repeats functionality and may confuse clients (which one to call?).If domain-specific wrapping is required, rename the methods (e.g.
fetchDtoById
) to communicate the added value, otherwise rely onIService
alone.base/src/main/java/com/tinyengine/it/service/app/DatasourceService.java (1)
26-26
: Consistency vs. verbositySame observation: inheriting
IService<Datasource>
plus hand-rolled CRUD style methods increases maintenance overhead. Evaluate whether the custom wrappers are still justified.base/src/main/java/com/tinyengine/it/service/app/PageTemplateService.java (1)
26-26
: Potential overlap withIService<PageTemplate>
IService
already exposesgetById
,removeByIds
, etc. Reconsider keepingqueryPageTemplateById
,deletePageTemplateByIds
, … to avoid redundancy.base/src/main/java/com/tinyengine/it/service/material/BusinessCategoryService.java (1)
25-25
: Redundant CRUD methodsSame duplication concern:
IService<BusinessCategory>
already supplies CRUD; extra wrappers should have clear added value or be removed.base/src/main/java/com/tinyengine/it/service/app/TaskRecordService.java (1)
25-25
: Consider pruning duplicate CRUD declarations after extendingIService
By inheriting from
IService<TaskRecord>
you already getgetById
,removeById
,save
, etc.
Keeping home-grown equivalents (queryTaskRecordById
,deleteTaskRecordById
, …) bloats the API surface and may confuse callers about which variant to use.base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
94-100
: CustomqueryAppById
duplicatesServiceImpl#getById
The bespoke method adds minimal value over the built-in
getById
. Consider delegating to or removing this wrapper to keep the interface lean.base/src/main/java/com/tinyengine/it/service/material/BlockHistoryService.java (1)
15-26
: Duplicate CRUD surface after introducingIService
Now that the interface extends
IService<BlockHistory>
, generic CRUD helpers (getById
,list
,removeById
,save
…) come for free. Keeping bespoke counterparts (findBlockHistoryById
,deleteBlockHistoryById
, etc.) bloats the API surface and invites inconsistent use.Consider dropping the hand-rolled methods (or delegating them to the inherited ones) to keep the service lean and consistent.
base/src/main/java/com/tinyengine/it/service/material/impl/ComponentServiceImpl.java (2)
55-56
:ServiceImpl
adopted — leverage its built-insExtending
ServiceImpl<ComponentMapper, Component>
already providesgetById
,list
,saveBatch
,saveOrUpdateBatch
, etc. Most of the wrapper methods below re-expose mapper calls 1-for-1, defeating the purpose and adding maintenance overhead.If no extra domain logic is injected, prefer the out-of-the-box methods or add thin adapters that simply call
super
helpers rather than rawbaseMapper
calls.
69-83
: Missed opportunity to replace bespoke queries with MP wrappers
queryAllComponent
,queryComponentById
andqueryComponentByCondition
look like straight-through SQL pores.
Unless they rely on custom joins/filters, they could be replaced with:return list(); // all return getById(id); // single return lambdaQuery() .setEntity(component) // or build conditions explicitly .list(); // conditionalThis reduces mapper code and aligns with the new MyBatis-Plus direction.
base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (2)
48-60
: Redundant wrappers around simple CRUDWith
ServiceImpl<PageHistoryMapper, PageHistory>
you already havelist()
,getById()
, etc.
findAllPageHistory
andfindPageHistoryById
presently just proxy to the mapper. Unless they encapsulate complex SQL, consider deleting them or delegating to inherited helpers to avoid duplicating the API.
81-82
: Custom delete mirrorsremoveById
return baseMapper.deletePageHistoryById(id);If this is a plain PK delete,
boolean removeById(id)
fromServiceImpl
does the job and returns success status. Swapping would simplify code and testing.base/src/main/java/com/tinyengine/it/service/app/impl/DatasourceServiceImpl.java (2)
35-45
:ServiceImpl
migration not fully utilisedMost service methods still fall back to mapper calls (
queryDatasourceById
,createDatasource
, …). Evaluate replacing straightforward CRUD with the generic helpers provided byServiceImpl
to minimise boilerplate and mapper surface.
100-101
: Prefersave()
over customcreateDatasource
if no special SQL is neededIf
createDatasource
performs a simple insert, the built-insave(datasource)
(orsaveOrUpdate
) would remove the need for a mapper method entirely and benefit from MP features such as automatic field filling.base/src/main/java/com/tinyengine/it/service/app/impl/I18nLangServiceImpl.java (1)
33-42
: Prefer MyBatis-Plus default CRUD over 1-to-1 passthrough methodsNow that the class extends
ServiceImpl
, the framework already provideslist() getById(id) save(entity) updateById(entity) removeById(id)and other helpers.
queryAllI18nLang
,queryI18nLangById
, etc. are simple passthroughs to custom mapper methods that replicate this behaviour and add maintenance surface.If no extra SQL is really needed, consider deleting the custom mapper methods and, in the service, delegating to the built-ins instead.
This will:
- reduce boilerplate
- keep the codebase consistent with MyBatis-Plus idioms
- ease unit-testing / mocking
base/src/main/java/com/tinyengine/it/service/material/impl/BlockHistoryServiceImpl.java (1)
33-42
: Same CRUD duplication issue as above
findAllBlockHistory()
is functionally identical tolist()
that you already inherit fromServiceImpl
.
Re-evaluate whether the customqueryAllBlockHistory
mapper method is still justified.base/src/main/java/com/tinyengine/it/service/material/impl/ComponentLibraryServiceImpl.java (1)
35-44
: Reuse inherited CRUD where possible
queryAllComponentLibrary()
is equivalent tolist()
.
Unless custom SQL is essential, migrate to the built-in API and delete the extra mapper method to keep the service lean.base/src/main/java/com/tinyengine/it/service/app/impl/PageTemplateServiceImpl.java (1)
52-57
: Potential elimination of duplicated mapper callsWithin the
name == null || name.isEmpty()
branch you already fetch all
page templates once.
Right after the branch you perform a second fetch based onname
.
Consider restructuring the method to make exactly one mapper call:List<PageTemplate> pageTemplates = StringUtils.isBlank(name) ? baseMapper.queryAllPageTemplate(type) : baseMapper.queryPageTemplateByName(name, type); return Result.success(pageTemplates);Minor, but saves an allocation and keeps the method concise.
base/src/main/java/com/tinyengine/it/service/material/impl/MaterialServiceImpl.java (1)
108-112
: Minor: useforEach
for side-effects instead ofmap
The stream pipeline here is only used for its side-effects; using
map
is semantically misleading and allocates an unusedList<Integer>
. Replace withforEach
to express intent.-i18nEntriesList.stream().map(entry -> baseMapper.createMaterial(entry)).collect(Collectors.toList()); +i18nEntriesList.forEach(entry -> baseMapper.createMaterial(entry));base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java (1)
153-156
: Replacestream().map(..).collect(..)
used only for side-effectsUsing
map
here hides the intent and creates an unnecessary intermediate list. A simpleforEach
is clearer and avoids the extra allocation.-i18nEntriesList.stream().map(entry -> baseMapper.createI18nEntry(entry)).collect(Collectors.toList()); +i18nEntriesList.forEach(baseMapper::createI18nEntry);Apply the same change in the
bulkCreate
path below.Also applies to: 199-201
base/src/main/java/com/tinyengine/it/service/app/impl/TaskRecordServiceImpl.java (1)
41-41
: Leverage built-in MyBatis-Plus helpers instead of hand-rolled mapper calls.
ServiceImpl
already exposeslist()
which returns all rows.
Keeping bothqueryAllTaskRecord()
andlist()
creates API duplication.Example replacement:
- public List<TaskRecord> queryAllTaskRecord() { - return baseMapper.queryAllTaskRecord(); - } + public List<TaskRecord> queryAllTaskRecord() { + return list(); + }base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (2)
63-64
:queryAllBlockGroup()
duplicateslist()
fromServiceImpl
.Unless you require custom SQL, prefer
list()
to reduce mapper code and improve maintainability.
211-211
: Avoid full-table scan when no filter supplied.Calling
queryAllBlockGroupAndBlock
with noids
and noappId
can explode in large installations.
Require at least one filtering parameter or enforce pagination to protect the database.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
app/src/main/java/com/tinyengine/it/config/filter/WebConfig.java
(1 hunks)base/src/main/java/com/tinyengine/it/service/app/AppExtensionService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/AppService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/DatasourceService.java
(2 hunks)base/src/main/java/com/tinyengine/it/service/app/I18nEntryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/I18nLangService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/PageHistoryService.java
(2 hunks)base/src/main/java/com/tinyengine/it/service/app/PageService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/PageTemplateService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/TaskRecordService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/UserService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppExtensionServiceImpl.java
(8 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java
(8 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/DatasourceServiceImpl.java
(6 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java
(13 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nLangServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java
(9 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java
(18 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageTemplateServiceImpl.java
(8 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/TaskRecordServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockGroupService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockHistoryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockService.java
(4 hunks)base/src/main/java/com/tinyengine/it/service/material/BusinessCategoryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/ComponentLibraryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/ComponentService.java
(4 hunks)base/src/main/java/com/tinyengine/it/service/material/MaterialHistoryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/MaterialService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/material/TaskRecordService.java
(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java
(10 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockHistoryServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java
(18 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BusinessCategoryServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ComponentLibraryServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ComponentServiceImpl.java
(9 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/MaterialHistoryServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/MaterialServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/TaskRecordMaterialServiceImpl.java
(4 hunks)base/src/main/java/com/tinyengine/it/service/platform/PlatformHistoryService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/platform/PlatformService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/platform/TenantService.java
(3 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformHistoryServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java
(7 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java
(7 hunks)pom.xml
(2 hunks)
🔇 Additional comments (14)
base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java (1)
15-15
: Smooth migration toServiceImpl
acknowledged
ExtendingServiceImpl<PlatformMapper, Platform>
cleanly removes the manual mapper wiring and brings the class in-line with MyBatis-Plus idioms.Also applies to: 35-35
pom.xml (2)
33-33
: Verify Springdoc 2.5.0 compatibility with Boot 3.1.9Springdoc 2.5.0 is the first GA release on the new starter coordinates, but its BOM is aligned with Spring-Boot 3.2.x. Running it on Boot 3.1.9 may introduce subtle version mismatches (e.g., spring-webflux transitive dependencies).
Please double-check the full dependency tree or pin an explicit springdoc BOM compatible with Boot 3.1.x before merging.
103-107
: Confirm legacyspringdoc-openapi-ui
artefact is fully removedThe project now depends on the WebFlux starter (
springdoc-openapi-starter-webflux-ui
). Make sure no residual<dependency>
or plugin still pulls in the MVC-orientedspringdoc-openapi-ui
, otherwise duplicate OpenAPI auto-configurations will be registered at runtime.base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java (1)
15-16
: Solid move to MyBatis-PlusServiceImpl
.Extending
ServiceImpl<UserMapper, User>
removes boiler-plate mapper wiring and keeps the class lean. Type parameters are correct (UserMapper
,User
) sobaseMapper
will resolve as expected.Also applies to: 33-33
base/src/main/java/com/tinyengine/it/service/material/BlockGroupService.java (1)
41-49
: DTO ↔︎ Entity inconsistency may leak persistence objects to callers.
findBlockGroupById
returnsBlockGroup
(entity) whilefindBlockGroupByCondition
returnsBlockGroupDto
. Mixing the two can confuse clients and break serialization rules (e.g. JPA lazy fields).Verify which type the REST/API layer expects; align both methods (prefer DTO).
If keeping the entity, annotate it with@JsonIgnore
on internal fields or move mapping to a converter.base/src/main/java/com/tinyengine/it/service/app/AppService.java (1)
43-43
: Mapper XML must be reviewed after dropping@Param
@Param
provided stable names for parameters used inside XML mappers (#{id}
etc.).
Without it, MyBatis resorts to positional names likeparam1
, unlessmybatis-plus.configuration.use-actual-param-name=true
and the byte-code retains parameter names (compiled with-parameters
).
Please verify all XML / annotation queries referencingid
still resolve.Also applies to: 59-59
base/src/main/java/com/tinyengine/it/service/material/BlockService.java (1)
48-48
: Re-check mapper statements for removed@Param
annotationsAs with other services, ensure XML statements that previously used
#{id}
do not break after annotation removal.Also applies to: 64-64
base/src/main/java/com/tinyengine/it/service/app/DatasourceService.java (1)
33-33
: Confirm parameter bindings post-@Param
removalValidate that mapper XML or annotations no longer rely on named parameters.
Also applies to: 49-49
base/src/main/java/com/tinyengine/it/service/app/PageTemplateService.java (1)
42-42
: Parameter naming after annotation cleanupDouble-check the underlying mapper (
PageTemplateMapper.xml
) for usages of#{id}
/#{ids}
.Also applies to: 58-58
base/src/main/java/com/tinyengine/it/service/material/BusinessCategoryService.java (1)
39-39
: Verify mapper compatibility after@Param
deletionEnsure SQL mappings still bind the
id
parameter correctly.Also applies to: 55-55
base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
51-51
: Good move toServiceImpl
Leveraging
ServiceImpl<AppMapper, App>
eliminates boiler-plate mapper injection and aligns with MyBatis-Plus idioms.base/src/main/java/com/tinyengine/it/service/material/impl/BusinessCategoryServiceImpl.java (1)
34-43
: No functional issues detected – implementation looks solidThe refactor correctly leverages
ServiceImpl
and replaces the handwritten mapper field withbaseMapper
. The delegated calls compile and behave identically.base/src/main/java/com/tinyengine/it/service/material/impl/MaterialHistoryServiceImpl.java (1)
36-45
: LGTM – straightforward delegationThe switch to
ServiceImpl
is clean and mirrors the pattern used elsewhere.base/src/main/java/com/tinyengine/it/service/app/impl/TaskRecordServiceImpl.java (1)
51-53
: Method name duplicatesgetById
and can be removed.
ServiceImpl
offersgetById(id)
out-of-the-box; maintainingqueryTaskRecordById
adds surface area without extra value.Do any callers outside this module still rely on the custom name? If not, consider deleting the wrapper.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Refactor
Chores
Style