feat: Service extends IService #241
Conversation
WalkthroughThis update refactors multiple service interfaces and their implementations to extend MyBatis-Plus's generic Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant ServiceImpl
participant Mapper
participant Database
Controller->>ServiceImpl: callServiceMethod()
ServiceImpl->>Mapper: callMapperMethod()
Mapper->>Database: execute SQL
Database-->>Mapper: return results
Mapper-->>ServiceImpl: return data
ServiceImpl-->>Controller: return response
Poem
✨ 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 (
|
![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 comments (5)
base/src/main/java/com/tinyengine/it/service/app/PageService.java (1)
30-87: 🛠️ Refactor suggestionRe-evaluate custom CRUD wrappers after switching to
IServiceNow that the interface extends
IService<Page>, default CRUD helpers (save,updateById,removeById,list, etc.) are already available. The bespoke variantscreatePage,delPage,updatePage, etc. duplicate that functionality and may confuse users about the “preferred” API.Consider either:
- Removing the custom methods and delegating to the generic ones, or
- Renaming them to truly domain-specific operations that wrap extra logic beyond a plain CRUD call.
base/src/main/java/com/tinyengine/it/service/app/I18nEntryService.java (1)
36-99: 🛠️ Refactor suggestionPotential overlap with
IServiceCRUDAfter inheriting
IService<I18nEntry>, methods likeupdateI18nEntryById,bulkCreate,bulkUpdate, etc. partially overlap withIService’supdateById,saveBatch, andupdateBatchById. Audit these signatures and consolidate where possible to reduce surface area and cognitive load.base/src/main/java/com/tinyengine/it/service/platform/PlatformHistoryService.java (1)
28-75: 🛠️ Refactor suggestionDuplicate CRUD verbs versus
IServiceMethods like
deletePlatformHistoryById,updatePlatformHistoryById, andcreatePlatformHistorynow parallelremoveById,updateById, andsave. Streamline to maintain a single CRUD vocabulary.base/src/main/java/com/tinyengine/it/service/app/impl/DatasourceServiceImpl.java (1)
104-113:⚠️ Potential issueMissing
returncauses wrong error code and mis-reports failureIf
res != 1, the code callsResult.failed(ExceptionEnum.CM001);but doesn’t return it, so execution falls through and returnsCM002instead.- int res = datasourceMapper.createDatasource(datasource); - if (res == 1) { - int id = datasource.getId(); - return Result.success(queryDatasourceById(id)); - } else { - Result.failed(ExceptionEnum.CM001); - } + int res = datasourceMapper.createDatasource(datasource); + if (res == 1) { + return Result.success(queryDatasourceById(datasource.getId())); + } + return Result.failed(ExceptionEnum.CM001);While here, also guard against
nullnames to avoidNullPointerException:String name = datasource.getName(); if (appId > 0 && StringUtils.hasText(name)) { ... }base/src/main/java/com/tinyengine/it/service/app/AppService.java (1)
15-24: 🛠️ Refactor suggestion
@Paramimports don’t belong in a service interface
org.apache.ibatis.annotations.Paramis meant for mapper signatures, not service APIs. It leaks persistence-layer details into the service layer and couples callers to MyBatis.Delete the annotation (and the import) from all method signatures; parameter names are preserved by the compiler when using
-parameters.
♻️ Duplicate comments (1)
base/src/main/java/com/tinyengine/it/service/platform/TenantService.java (1)
15-16: Same duplication caveat as inDatasourceService
TenantServicenow exposes bothIService<Tenant>CRUD and bespokefind*/delete*
methods. Audit the interface for redundant operations and either rely on the generic
CRUD helpers or rename / remove overlapping ones for clarity.Also applies to: 27-27
🧹 Nitpick comments (40)
base/src/main/java/com/tinyengine/it/service/app/DatasourceService.java (1)
15-16:IService<Datasource>gives you CRUD – reconsider the extra bespoke CRUD-like methodsBy extending
IService<Datasource>the interface already inherits:
getById(id)save(entity)updateById(entity)removeById(id)- etc.
The custom
queryDatasourceById,deleteDatasourceById,createDatasource, … now duplicate
functionality and add a second vocabulary (query*,delete*,create*) that future
contributors have to learn.Consider either:
- Relying on the default
IServiceCRUD helpers and keeping only the truly
domain-specific queries; or- Keeping the custom methods but dropping the
IServiceinheritance to avoid an API
surface that does the same thing twice.This keeps the service contract minimal and avoids maintenance overhead.
Also applies to: 28-28
base/src/main/java/com/tinyengine/it/service/app/PageTemplateService.java (1)
15-16: Check return-type consistency withIService
IService<PageTemplate>already declaresremoveByIds(Collection<?>).
The customdeletePageTemplateByIds(List<Integer>)overlaps but wraps the integer in
Result<>, creating two different ways to delete records.This asymmetry can confuse callers and complicate transactional handling.
Standardise on one approach (plain boolean / count vs. wrappedResult)
across the whole service layer.Also applies to: 28-28
base/src/main/java/com/tinyengine/it/service/material/BlockService.java (1)
15-17: Potential name clash with MyBatis-Plus paging helpers
BlockServicedefinesfindBlocksByPagetionListandfindBlocksByConditionPagetion
whileIService+ServiceImplalready give youpage(Page,Wrapper).If you keep the custom names:
- Fix the typo “Pagetion” → “Pagination”.
- Document why the built-in
pagehelper is insufficient (e.g., extra joins).Otherwise consider deleting these methods and delegate to
IService#pagein the
implementation to reduce surface area.Also applies to: 36-36
base/src/main/java/com/tinyengine/it/service/app/UserService.java (1)
15-16: Consider pruning redundant CRUD methods after introducingIServiceNow that the interface inherits all generic CRUD helpers from
IService<User>, several hand-rolled methods (createUser,deleteUserById,updateUserById,queryUserById, etc.) overlap in responsibility withsave,removeById,updateById,getById,list, … supplied by MyBatis-Plus.
Keeping both sets increases surface area, maintenance cost, and the risk of divergent behaviour.If project conventions allow, deprecate or remove the bespoke variants and let callers use the standard ones. At minimum, add JavaDoc explaining why both coexist.
Also applies to: 27-27
base/src/main/java/com/tinyengine/it/service/material/BusinessCategoryService.java (1)
15-16: Same duplication concern as inUserServiceExtending
IService<BusinessCategory>already exposeslist(),getById(),save(),removeById()and others. Re-evaluating the custom CRUD-style methods could slim the API and avoid confusion.Also applies to: 27-27
base/src/main/java/com/tinyengine/it/service/app/I18nLangService.java (1)
15-16: Potential overlap between MyBatis-Plus defaults and bespoke calls
IService<I18nLang>exposes generic CRUD helpers identical in purpose tocreateI18nLang,deleteI18nLangById, etc.
Unless these methods add extra business constraints (validation, auditing…), consider delegating to / or replacing them with the framework-provided ones.Also applies to: 27-27
base/src/main/java/com/tinyengine/it/service/material/BlockGroupService.java (2)
15-16: Javadoc–signature mismatch: return type
findBlockGroupById’s Javadoc advertisesBlockGroupDto, yet the method returnsBlockGroup.
Either update the Javadoc or change the return type so that documentation and API stay aligned.Also applies to: 29-29
15-16: Reassess bespoke CRUD after inheritingIService<BlockGroup>Similar to previous files: with
IServicenow in place, methods such ascreateBlockGroup,deleteBlockGroupById,updateBlockGroupByIdduplicatesave,removeById,updateById. Streamlining will reduce boilerplate.Also applies to: 29-29
base/src/main/java/com/tinyengine/it/service/material/BlockHistoryService.java (1)
15-16: Duplicate CRUD surfaceThe custom
createBlockHistory,deleteBlockHistoryById, etc. echo built-ins fromIService<BlockHistory>. Consider delegating to—or deprecating in favour of—the generic operations to keep the service layer lean.Also applies to: 27-27
base/src/main/java/com/tinyengine/it/service/material/ComponentService.java (1)
33-33: ExtendingIServicemakes many handwritten CRUD methods redundant
IService<Component>already supplieslist(),getById(),save(),updateById(),removeById()and batch variants. Consider deleting the duplicated custom methods (or at least those that map 1-to-1) to cut noise and future maintenance.base/src/main/java/com/tinyengine/it/service/app/PageHistoryService.java (1)
30-30:IService<PageHistory>added – rethink custom CRUD signaturesAfter inheriting from
IService, a good chunk of the interface overlaps with the base CRUD API. Keeping both increases cognitive load and requires duplicate tests.base/src/main/java/com/tinyengine/it/service/app/TaskRecordService.java (1)
27-27: Redundant CRUD declarationsExtending
IService<TaskRecord>renders several handcrafted CRUD methods superfluous. Streamlining the interface will simplify the implementation layer as well.base/src/main/java/com/tinyengine/it/service/platform/PlatformService.java (1)
28-28: LeverageIServicedefault methodsNow that the interface extends
IService<Platform>, consider eliminating boiler-plate CRUD declarations unless they add domain-specific behavior (e.g., wrapping results inResult<>).base/src/main/java/com/tinyengine/it/service/material/ComponentLibraryService.java (1)
28-28: Assess duplication vs.IServiceExtending
IService<ComponentLibrary>makes most CRUD helpers unnecessary; trimming them will keep the API minimal and clearer.base/src/main/java/com/tinyengine/it/service/app/PageService.java (1)
15-30: Remove MyBatis@Paramimports from service layer
@Paramis a MyBatis-Mapper annotation. Service interfaces are not scanned by MyBatis, so the annotation is a no-op here and just adds an unnecessary compile-time dependency onmybatis-spring. Drop the import (and the annotation usages further down) to keep the service layer framework-agnostic.-import org.apache.ibatis.annotations.Param;Follow-up: remove the
@Paramon every method parameter in this interface.base/src/main/java/com/tinyengine/it/service/app/I18nEntryService.java (1)
15-36: Strip framework annotations from service contractsSame as in
PageService:@Parambelongs in mapper interfaces only. Eliminating it (and its import) keeps the service module cleanly decoupled from persistence details.base/src/main/java/com/tinyengine/it/service/material/TaskRecordService.java (2)
15-30: Avoid leaking persistence annotations to the service layer
@Paramand its import should be removed for the same reasons mentioned in previous files.
30-47: Custom “status” query might be better placed in mapper
IService<TaskRecord>already exposes read helpers; placing the complexstatusquery directly in the service interface couples consumers to implementation details. Consider declaring it in the mapper and exposing it via the service implementation instead of the interface, unless it is intentionally part of the public domain API.base/src/main/java/com/tinyengine/it/service/platform/PlatformHistoryService.java (1)
15-28: Remove persistence-layer annotationsAs with other services, drop
@Paramusages/imports to keep the service layer agnostic of MyBatis.base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java (1)
36-39: Favour constructor injection (or rely onServiceImpl) over field injectionIf you decide the dedicated mapper is still required (e.g. for clarity), prefer constructor injection to keep the field
finaland simplify testing:private final TenantMapper tenantMapper; public TenantServiceImpl(TenantMapper tenantMapper) { this.tenantMapper = tenantMapper; }Field injection hides dependencies and complicates immutability.
base/src/main/java/com/tinyengine/it/service/app/AppService.java (1)
31-32: Double-exposed CRUD surface: consider delegating toIServicedefaultsBy extending
IService<App>you already getgetById,list,save,removeById, etc. The custom methodsqueryAppById,deleteAppById,createApp, … are semantically identical and add maintenance overhead. Unless they apply extra business rules, you can delete them or delegate internally to the generic methods.base/src/main/java/com/tinyengine/it/service/app/AppExtensionService.java (1)
28-29: Redundant CRUD wrappersAs with
AppService, evaluate whether custom CRUD methods are still required now thatIService<AppExtension>provides them out of the box.base/src/main/java/com/tinyengine/it/service/material/MaterialHistoryService.java (1)
28-29: Overlap withIService<MaterialHistory>Consider removing or delegating the CRUD-only methods to the inherited defaults to reduce duplication.
base/src/main/java/com/tinyengine/it/service/material/MaterialService.java (1)
15-16: Re-evaluate the need for custom CRUD declarations after extendingIService.By extending
IService<Material>you already inherit the default CRUD APIs (save,getById,removeById, etc.).
Keeping hand-rolled methods such ascreateMaterial,deleteMaterialById, etc. is perfectly valid when you need the customisedResult<Material>wrapper, but it does introduce two parallel CRUD surfaces that can drift.If the wrapper is not strictly required everywhere, consider:
-public interface MaterialService extends IService<Material> { - … - Result<Material> createMaterial(Material material); -} +public interface MaterialService extends IService<Material> { + // Domain-specific queries only; reuse default CRUD from IService. +}This keeps the service footprint minimal and leans fully on MyBatis-Plus.
Also applies to: 28-30
base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java (1)
17-18: Consider consolidating mapper access tobaseMapperfor consistency.With
ServiceImpl<I18nEntryMapper, I18nEntry>in place you can eliminate the explicit@Autowired I18nEntryMapper i18nEntryMapperand usebaseMapper. Keeping both variants is harmless but increases mental load for maintainers.Also applies to: 69-77
base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java (1)
40-45: Consider relying onServiceImpl’s built-inbaseMapperinstead of injecting your own mapperNow that the class extends
ServiceImpl<PageHistoryMapper, PageHistory>, an initializedPageHistoryMapperis already available via the protectedbaseMapperfield.
Keeping a separate@Autowired PageHistoryMapper pageHistoryMapper;duplicates the dependency and slightly complicates maintenance/testing.- @Autowired - private PageHistoryMapper pageHistoryMapper; + // `baseMapper` provided by ServiceImpl already points to PageHistoryMapperUnless you need two distinct mapper instances, drop the manual field and use
baseMapper(or add an accessor) throughout the class.
This keeps the codebase leaner and avoids accidental divergence.base/src/main/java/com/tinyengine/it/service/material/impl/MaterialHistoryServiceImpl.java (1)
38-44: Duplicate mapper injection after extendingServiceImpl
ServiceImpl<MaterialHistoryMapper, MaterialHistory>supplies abaseMapperalready wired by Spring.
The explicit@Autowired private MaterialHistoryMapper materialHistoryMapper;is therefore redundant. Consider removing it and migrating calls to
baseMapperto reduce boiler-plate and keep one single injection point.base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java (1)
37-40: RedundantPlatformMapperfieldBecause the class now inherits from
ServiceImpl<PlatformMapper, Platform>, the mapper is injected inbaseMapper.
Unless you need special qualification, you can safely delete the@Autowired private PlatformMapper platformMapper;field and referencebaseMapperinstead.
This avoids duplicate beans and keeps the service thinner.base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java (1)
80-83: Remove manualBlockMapperinjection; usebaseMapperAfter extending
ServiceImpl<BlockMapper, Block>the framework already autowires aBlockMapperinstance (fieldbaseMapper).
Keeping another injectedblockMapperintroduces two mapper handles that can diverge in transactional context or mock setups. Switch existing calls tobaseMapperand drop the field to simplify the service.base/src/main/java/com/tinyengine/it/service/material/impl/ComponentServiceImpl.java (1)
56-63: LeverageServiceImpl’sbaseMapperinstead of custom fieldWith the new inheritance you already get a Spring-managed mapper. Replacing
@Autowired private ComponentMapper componentMapper;by
baseMappertrims code and avoids bean duplication.
Note: adjust downstream calls (queryAllComponent,createComponent, etc.) accordingly.base/src/main/java/com/tinyengine/it/service/app/impl/PageTemplateServiceImpl.java (1)
40-42: Drop the explicitPageTemplateMapperfield –ServiceImplalready wires it in.
ServiceImplinjects abaseMapperinstance of the generic type (PageTemplateMapper). Keeping a second, manually-wired@Autowiredfield is redundant noise and may lead to confusion about which instance is being used or mocked in tests.- @Autowired - private PageTemplateMapper pageTemplateMapper; + // `ServiceImpl` provides a protected `baseMapper`; use it directly.Follow-up: Replace every
pageTemplateMapper.call in this class withbaseMapper.(orgetBaseMapper()if you prefer explicit access).base/src/main/java/com/tinyengine/it/service/app/impl/TaskRecordServiceImpl.java (1)
35-37: Same redundancy here – remove the custom-wired mapper.Extending
ServiceImpl<TaskRecordMapper, TaskRecord>already gives youbaseMapper. Keep one source of truth.- @Autowired - private TaskRecordMapper taskRecordMapper; + // Rely on `baseMapper` supplied by `ServiceImpl`.Adjust method bodies accordingly.
base/src/main/java/com/tinyengine/it/service/material/impl/ComponentLibraryServiceImpl.java (1)
37-40: Consolidate mapper access – avoid duplicate injection.
ServiceImplautowiresComponentLibraryMapper; there’s no need for an extra field.- @Autowired - private ComponentLibraryMapper componentLibraryMapper; + // Use `baseMapper` from the superclass.This clean-up will also make unit testing simpler.
base/src/main/java/com/tinyengine/it/service/material/impl/BusinessCategoryServiceImpl.java (1)
36-38: Drop manual mapper wiring; leveragebaseMapper.Consistent with other services, remove the redundant
@Autowiredmapper and use the one provided byServiceImpl.- @Autowired - private BusinessCategoryMapper businessCategoryMapper;After removal, update method bodies to reference
baseMapper.base/src/main/java/com/tinyengine/it/service/app/impl/AppExtensionServiceImpl.java (1)
38-41: Replace explicit mapper field with the inheritedbaseMapper.Redundant injection adds boilerplate and potential for inconsistent mocks.
- @Autowired - private AppExtensionMapper appExtensionMapper; + // Leverage `baseMapper` from `ServiceImpl`.Update method implementations to use
baseMapper.base/src/main/java/com/tinyengine/it/service/app/impl/I18nLangServiceImpl.java (1)
35-37: Redundant manual mapper injection after extendingServiceImpl
ServiceImplalready autowires a protectedBaseMapperinstance (baseMapper) of the same generic type.
Keeping the extra@Autowired private I18nLangMapper i18nLangMapper;duplicates the dependency, increases memory footprint, and may confuse future maintainers.
Consider deleting the field and switching method bodies to
baseMapper:- @Autowired - private I18nLangMapper i18nLangMapper; + // baseMapper is inherited from ServiceImpl<I18nLangMapper, I18nLang>Example refactor:
- return i18nLangMapper.queryAllI18nLang(); + return baseMapper.queryAllI18nLang();(The same replacement applies to all other mapper usages in this class.)
base/src/main/java/com/tinyengine/it/service/material/impl/TaskRecordMaterialServiceImpl.java (1)
40-43: Duplicate mapper field – favourbaseMapperfromServiceImplAfter inheriting
ServiceImpl<TaskRecordMapper, TaskRecord>, the superclass wiresbaseMapper.
The explicit@Autowired TaskRecordMapper taskRecordMapper;is now superfluous and can be removed to avoid ambiguity.- @Autowired - TaskRecordMapper taskRecordMapper; + // baseMapper is available from ServiceImplUpdate the method bodies accordingly (
taskRecordMapper→baseMapper).base/src/main/java/com/tinyengine/it/service/material/impl/BlockGroupServiceImpl.java (1)
47-55: Remove unnecessary field injections – rely onbaseMapperchain
BlockGroupServiceImplnow extendsServiceImpl<BlockGroupMapper, BlockGroup>but still keeps:@Autowired private BlockGroupMapper blockGroupMapper;and similar mapper fields.
This is redundant;baseMapperand the other injected mappers can be accessed through constructor injection or separate service classes if really needed. At minimum, dropblockGroupMapperto prevent two independent instances of the same bean.- @Autowired - private BlockGroupMapper blockGroupMapper; + // baseMapper provided by ServiceImpl covers BlockGroupMapper(Adapt usages throughout the class.)
base/src/main/java/com/tinyengine/it/service/material/impl/BlockHistoryServiceImpl.java (1)
35-38: Leverage inheritedbaseMapperinstead of explicit mapper injectionSame pattern:
ServiceImpl<BlockHistoryMapper, BlockHistory>injectsbaseMapper.
Eliminate the manual@Autowired BlockHistoryMapper blockHistoryMapper;and switch tobaseMapper.This trims boilerplate and aligns with MyBatis-Plus conventions.
base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformHistoryServiceImpl.java (1)
36-38: Superfluous mapper field after extendingServiceImpl
PlatformHistoryServiceImplnow extendsServiceImpl<PlatformHistoryMapper, PlatformHistory>yet still defines its ownPlatformHistoryMapper. Remove it and use the inheritedbaseMapper.Also consider switching to constructor injection if additional mappers are required; this improves testability and immutability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
base/src/main/java/com/tinyengine/it/service/app/AppExtensionService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/AppService.java(2 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(2 hunks)base/src/main/java/com/tinyengine/it/service/app/I18nLangService.java(2 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(2 hunks)base/src/main/java/com/tinyengine/it/service/app/PageTemplateService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/TaskRecordService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/UserService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppExtensionServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/DatasourceServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nEntryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/I18nLangServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageHistoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/PageTemplateServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/TaskRecordServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockGroupService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockHistoryService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/BlockService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/BusinessCategoryService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/ComponentLibraryService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/ComponentService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/MaterialHistoryService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/MaterialService.java(2 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(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockHistoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BlockServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/BusinessCategoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ComponentLibraryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ComponentServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/MaterialHistoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/MaterialServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/TaskRecordMaterialServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/PlatformHistoryService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/PlatformService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/TenantService.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformHistoryServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/PlatformServiceImpl.java(2 hunks)base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java(2 hunks)pom.xml(2 hunks)
🔇 Additional comments (7)
pom.xml (1)
33-33:Details
❌ Incorrect review comment
Remove the orphaned
springdoc-openapi-uiproperty to avoid confusion.A new property
springdoc-openapi-starter-webmvc-ui.versionhas been added, but the legacy
springdoc-openapi-ui.versionproperty (defined elsewhere in the POM or in child modules) is now obsolete.
Keeping both properties around risks version drift and mistaken references.- <springdoc-openapi-ui.version>1.6.14</springdoc-openapi-ui.version>Run the grep below to make sure no module still references the old property / artifact:
🏁 Script executed:
#!/bin/bash rg -n "springdoc-openapi-ui" -g '*.xml'Length of output: 39
No obsolete
springdoc-openapi-ui.versionproperty found
- Ran
rg -n "springdoc-openapi-ui" -g '*.xml'across the repository; no matches were found.- The legacy
<springdoc-openapi-ui.version>property does not exist in any POM, so no removal is necessary.Likely an incorrect or invalid review comment.
base/src/main/java/com/tinyengine/it/service/material/ComponentService.java (1)
15-16: Dependency scope check for MyBatis-Plus importThe new
IServiceimport is correct, but please verify that the project BOM / parent POM exposes a single, consistent MyBatis-Plus version; otherwise several modules may silently pull different transitive versions and break the generic‐service hierarchy at runtime.base/src/main/java/com/tinyengine/it/service/app/PageHistoryService.java (1)
16-16: Import is fine – double-check shading conflictsSame note as other services: ensure a single MyBatis-Plus version is used across the multi-module build.
base/src/main/java/com/tinyengine/it/service/app/TaskRecordService.java (1)
15-15: Verify MyBatis-Plus dependency alignmentConfirm there are no conflicting
IServiceclasses on the class-path (e.g., old3.4.xvs.3.5.x).base/src/main/java/com/tinyengine/it/service/platform/PlatformService.java (1)
15-15: Ensure consistent MyBatis-Plus version across modulesRepeated reminder to lock the dependency to a single version to avoid
ClassCastExceptionbetween different class-loaders.base/src/main/java/com/tinyengine/it/service/material/ComponentLibraryService.java (1)
15-15: Version sanity check for MyBatis-PlusAs with the other services, please confirm a unified MyBatis-Plus version to avoid linkage errors.
base/src/main/java/com/tinyengine/it/service/app/impl/AppServiceImpl.java (1)
52-55: 👍 ExtendingServiceImplcorrectly integrates MyBatis-PlusThe switch to
ServiceImpl<AppMapper, App>is spot-on and unlocks generic CRUD helpers while keeping existing custom logic intact.
| <groupId>org.springdoc</groupId> | ||
| <artifactId>springdoc-openapi-ui</artifactId> | ||
| <version>${springdoc-openapi-ui.version}</version> | ||
| <artifactId>springdoc-openapi-starter-webmvc-ui</artifactId> | ||
| <version>${springdoc-openapi-starter-webmvc-ui.version}</version> |
There was a problem hiding this comment.
💡 Verification agent
🛠️ Refactor suggestion
🧩 Analysis chain
Confirm that the new starter is compatible with both spring-boot-starter-web and spring-boot-starter-webflux.
springdoc-openapi-starter-webmvc-ui pulls in spring-webmvc.
Because this POM also includes spring-boot-starter-webflux (line 48), the application
will have both the servlet and reactive stacks on the class-path, which can lead to
ambiguous handler mappings at runtime.
If the project does not serve WebFlux endpoints, drop the webflux starter; otherwise
use springdoc-openapi-starter-webflux-ui instead.
🏁 Script executed:
#!/bin/bash
set -e
# Check for Spring WebFlux starter dependency
grep -R "<artifactId>spring-boot-starter-webflux</artifactId>" -n pom.xml
# Check for Spring WebMVC/Web starter dependency
grep -R "<artifactId>spring-boot-starter-web</artifactId>" -n pom.xml
# Check if the WebFlux-specific SpringDoc starter is referenced
grep -R "<artifactId>springdoc-openapi-starter-webflux-ui</artifactId>" -n pom.xml || echo "No springdoc-openapi-starter-webflux-ui found"Length of output: 474
pom.xml: Ensure SpringDoc starter matches your web stack
Detected dependencies:
- Line 41:
<artifactId>spring-boot-starter-web</artifactId> - Line 49:
<artifactId>spring-boot-starter-webflux</artifactId> - SpringDoc starter in use (lines 108–110):
<groupId>org.springdoc</groupId> <artifactId>springdoc-openapi-starter-webmvc-ui</artifactId> <version>${springdoc-openapi-starter-webmvc-ui.version}</version>
Having both servlet (WebMVC) and reactive (WebFlux) on the classpath can lead to ambiguous handler mappings at runtime.
Actions:
- If you only serve servlet endpoints, remove the
spring-boot-starter-webfluxdependency. - If you expose WebFlux endpoints, swap in the reactive SpringDoc starter:
<artifactId>springdoc-openapi-starter-webflux-ui</artifactId>
🤖 Prompt for AI Agents
In pom.xml around lines 41, 49, and 108 to 110, there is a conflict between the
SpringDoc starter and the web stack dependencies. The project includes both
spring-boot-starter-webflux (line 49) and spring-boot-starter-web (line 41), but
the SpringDoc starter used is springdoc-openapi-starter-webmvc-ui (lines
108-110), which is for the servlet stack. To fix this, either remove the
spring-boot-starter-webflux dependency if you only serve servlet endpoints, or
if you serve reactive WebFlux endpoints, replace the SpringDoc starter
artifactId with springdoc-openapi-starter-webflux-ui to match the reactive
stack.
| import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl; | ||
| import com.tinyengine.it.mapper.TenantMapper; | ||
| import com.tinyengine.it.model.entity.Tenant; | ||
| import com.tinyengine.it.service.platform.TenantService; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ServiceImpl already provides baseMapper; the explicit @Autowired TenantMapper becomes redundant
Now that the class inherits from ServiceImpl<TenantMapper, Tenant>, the protected baseMapper field gives you full access to the mapper. Keeping a second, manually-wired tenantMapper:
- duplicates the dependency,
- slightly increases startup time,
- makes unit-testing harder (two fields to mock), and
- risks accidental divergence if one of them is refactored.
Consider deleting the explicit field and updating the methods to use baseMapper instead.
Example:
-@Autowired
-private TenantMapper tenantMapper;
+// ServiceImpl already exposes `protected TenantMapper baseMapper;`-return tenantMapper.queryAllTenant();
+return baseMapper.queryAllTenant();Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
base/src/main/java/com/tinyengine/it/service/platform/impl/TenantServiceImpl.java
around lines 15 to 18, remove the explicit @Autowired TenantMapper field since
ServiceImpl already provides the protected baseMapper field. Refactor all usages
of tenantMapper in the class to use baseMapper instead, eliminating the
redundant dependency and simplifying testing and maintenance.
| public class DatasourceServiceImpl extends ServiceImpl<DatasourceMapper, Datasource> implements DatasourceService { | ||
| @Autowired | ||
| private DatasourceMapper datasourceMapper; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same mapper-duplication issue as in TenantServiceImpl
ServiceImpl<DatasourceMapper, Datasource> already wires a mapper. Remove the extra @Autowired DatasourceMapper or switch your code to baseMapper.
🤖 Prompt for AI Agents
In
base/src/main/java/com/tinyengine/it/service/app/impl/DatasourceServiceImpl.java
around lines 37 to 40, there is a redundant injection of DatasourceMapper via
@Autowired despite extending ServiceImpl which already provides a mapper
instance as baseMapper. Remove the @Autowired DatasourceMapper field and update
any references from datasourceMapper to baseMapper to avoid duplication and use
the inherited mapper correctly.
| import com.baomidou.mybatisplus.extension.service.IService; | ||
| import com.tinyengine.it.common.base.Result; | ||
| import com.tinyengine.it.model.entity.AppExtension; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same layering concern: remove @Param and keep service layer free from MyBatis specifics
See comment in AppService.
🤖 Prompt for AI Agents
In base/src/main/java/com/tinyengine/it/service/app/AppExtensionService.java
around lines 15 to 18, remove all usages and imports of MyBatis-specific
annotations like @Param from the service layer. Refactor method signatures to
use plain Java parameters without MyBatis annotations to keep the service layer
independent of persistence framework details, following the same layering
principle applied in AppService.
| import com.baomidou.mybatisplus.extension.service.IService; | ||
| import com.tinyengine.it.common.base.Result; | ||
| import com.tinyengine.it.model.entity.MaterialHistory; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Service interface should not import MyBatis @Param
Keep persistence annotations in mapper interfaces only.
🤖 Prompt for AI Agents
In
base/src/main/java/com/tinyengine/it/service/material/MaterialHistoryService.java
around lines 15 to 18, remove any import of MyBatis `@Param` annotation from the
service interface. Ensure that `@Param` and other persistence-related
annotations are only used in the mapper interfaces, not in service layer
interfaces, to maintain proper separation of concerns.
| public class MaterialServiceImpl extends ServiceImpl<MaterialMapper, Material> implements MaterialService { | ||
| @Autowired | ||
| private MaterialMapper materialMapper; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Drop the redundant @Autowired MaterialMapper – ServiceImpl already wires one.
ServiceImpl<MaterialMapper, Material> injects a protected baseMapper instance.
Keeping a second MaterialMapper materialMapper:
- duplicates the bean injection,
- risks accidental divergence (someone might use
baseMapperin a future edit), - slightly increases memory footprint.
Suggested clean-up:
-@Autowired
-private MaterialMapper materialMapper;and replace internal usages with baseMapper.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
base/src/main/java/com/tinyengine/it/service/material/impl/MaterialServiceImpl.java
around lines 37 to 40, remove the redundant @Autowired MaterialMapper field
since ServiceImpl already provides a protected baseMapper instance. Replace all
internal usages of materialMapper with baseMapper to avoid duplicate injection
and potential inconsistencies.
| import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl; | ||
| import com.tinyengine.it.common.base.Result; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same duplication pattern – rely on baseMapper instead of a second mapper field.
ServiceImpl<PageMapper, Page> already gives you baseMapper; additionally wiring pageMapper adds no value and muddies the waters when both names appear in the file.
Apply the same removal pattern as suggested for MaterialServiceImpl:
-@Autowired
-private PageMapper pageMapper;Then rename references (pageMapper → baseMapper) in this class.
This also applies to other *ServiceImpl classes introduced in this PR.
Also applies to: 75-82
🤖 Prompt for AI Agents
In base/src/main/java/com/tinyengine/it/service/app/impl/PageServiceImpl.java
around lines 17-18 and 75-82, remove the explicit declaration and injection of
the pageMapper field since ServiceImpl<PageMapper, Page> already provides
baseMapper. Replace all references of pageMapper with baseMapper throughout the
class to avoid duplication and confusion, following the same pattern used in
MaterialServiceImpl and other similar service implementations in this PR.
| import com.baomidou.mybatisplus.extension.service.impl.ServiceImpl; | ||
| import com.tinyengine.it.mapper.UserMapper; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate mapper injection – remove and use baseMapper.
Same comment as above: ServiceImpl<UserMapper, User> already wires a UserMapper. Recommend deleting the explicit field to avoid redundancy.
-@Autowired
-private UserMapper userMapper;Then update the method bodies to baseMapper.*.
Also applies to: 34-37
🤖 Prompt for AI Agents
In base/src/main/java/com/tinyengine/it/service/app/impl/UserServiceImpl.java
around lines 15-16 and 34-37, there is a redundant explicit injection of
UserMapper despite extending ServiceImpl which already provides a baseMapper
instance. Remove the explicit UserMapper field declaration and replace all
references to the explicit mapper in method bodies with baseMapper to avoid
duplication and simplify the code.
1 participant
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