test: add DynamicModelServiceTest

[msslulu]

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our Commit Message Guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

Summary by CodeRabbit

Release Notes

• Tests
  • Added comprehensive test suite for the dynamic model service, covering table management, data initialization, querying operations, pagination, and CRUD functionality to enhance code reliability and quality assurance.

[coderabbitai]

Walkthrough

A new JUnit 5 test class DynamicModelServiceTest was added to provide comprehensive test coverage for DynamicModelService, including test cases for table creation, schema initialization, CRUD operations, and dynamic query execution with mocked JDBC dependencies.

Changes

Cohort / File(s)	Summary
DynamicModelServiceTest base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java	Added 357 lines of JUnit 5 test suite with mocked JdbcTemplate, NamedParameterJdbcTemplate, and LoginUserContext. Covers DDL/DML operations (table creation/dropping), schema initialization, dynamic select/count queries, pagination with queryWithPage, and CRUD operations (createData, getDataById, updateDateById, deleteDataById). Includes test duplicates for several behaviors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hopping through the test suite with glee,
Mock objects dancing wild and free,
DDL, CRUD, queries galore,
Coverage blooming forevermore! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding a new test class for DynamicModelService.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (3)

base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java (3)

45-46: Duplicate MockitoAnnotations.openMocks(this) call.

Line 46 is redundant and should be removed.

Proposed fix

 	`@BeforeEach`
 	void setUp() {
 		MockitoAnnotations.openMocks(this);
-		MockitoAnnotations.openMocks(this);
 		ReflectUtil.setFieldValue(dynamicModelService, "jdbcTemplate", jdbcTemplate);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java`
around lines 45 - 46, Remove the duplicate Mockito initialization in the test
setup: in DynamicModelServiceTest remove the redundant second call to
MockitoAnnotations.openMocks(this) so only a single invocation remains; ensure
the remaining call is in the appropriate setup method (e.g., `@BeforeEach`) and
that no other test initialization relies on the removed duplicate.

---

47-49: Reflection-based field injection is redundant with @InjectMocks.

@InjectMocks already injects @Mock fields into dynamicModelService. The reflection calls are unnecessary unless there's a specific injection issue. If @InjectMocks isn't working correctly, consider verifying the field names match or using constructor injection in the service class instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java`
around lines 47 - 49, The test uses ReflectUtil.setFieldValue to inject
jdbcTemplate, loginUserContext, and namedParameterJdbcTemplate into
dynamicModelService, but `@InjectMocks` should already perform this injection;
remove the three ReflectUtil.setFieldValue(...) calls and rely on the
`@InjectMocks-driven` injection for dynamicModelService, and if injection still
fails verify that the mocked field names (jdbcTemplate, loginUserContext,
namedParameterJdbcTemplate) match the fields in the DynamicModelService class or
switch the service to constructor injection to make mocks injectable.

---

287-354: Duplicate test methods with test* prefix.

testCreateDynamicTable, testDropDynamicTable, testDynamicQuery, testCreateData, and testDeleteDataById duplicate the earlier tests (createDynamicTable, dropDynamicTable, etc.) with minimal variation. This adds maintenance burden without additional coverage.

Consider removing these duplicates or, if they test different scenarios, renaming them to describe the specific scenario (e.g., createDynamicTable_withNullOrderBy).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java`
around lines 287 - 354, The test class contains duplicate test methods
(testCreateDynamicTable, testDropDynamicTable, testDynamicQuery, testCreateData,
testDeleteDataById) that mirror existing tests (createDynamicTable,
dropDynamicTable, dynamicQuery, createData, deleteDataById); remove the
redundant test methods or convert them into clearly-scoped tests by renaming and
adjusting their assertions to reflect distinct scenarios (e.g.,
createDynamicTable_withNullOrderBy, deleteDataById_returnsZeroRows) so they no
longer duplicate coverage; locate the methods by name in DynamicModelServiceTest
and either delete the duplicate methods or rename and modify their setup/verify
expectations to exercise a different behavior, keeping only one authoritative
test per scenario.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java`:
- Around line 190-191: The two consecutive stubs for
namedParameterJdbcTemplate.queryForList in DynamicModelServiceTest overwrite
each other, breaking the queryWithPage test; update the stub for
namedParameterJdbcTemplate.queryForList (used by queryWithPage) to return
different values for the two sequential calls by chaining
thenReturn().thenReturn() or by using thenAnswer with a call counter so the
first invocation returns mockData and the second returns List.of(Map.of("count",
1L)); target the stub around the
when(namedParameterJdbcTemplate.queryForList(...)) in the
DynamicModelServiceTest so assertions later in the test validate the expected
paged data and count correctly.
- Around line 275-283: The test in DynamicModelServiceTest's deleteDataById uses
the invalid matcher Optional.ofNullable(any()) for jdbcTemplate.update; replace
that with a proper matcher that matches the Long id parameter (e.g., anyLong()
or eq(dto.getId())) in both the when(...) stubbing and the verify(...) call so
the stub matches the actual jdbcTemplate.update(sql, id) invocation in
DynamicModelService.deleteDataById.
- Around line 349-353: The test uses Optional.ofNullable(any()) as a Mockito
argument matcher which is invalid; in DynamicModelServiceTest's
testDeleteDataById replace the matcher so jdbcTemplate.update(...) uses a proper
matcher like any(Optional.class) or ArgumentMatchers.<Optional<?>>any() (i.e.,
change the second argument matcher from Optional.ofNullable(any()) to
any(Optional.class) or equivalent) and keep the existing
when(...).thenReturn(...) and verify(...) calls referring to
jdbcTemplate.update.
- Around line 238-246: The Mockito stub for jdbcTemplate.queryForList
incorrectly uses Optional.ofNullable(any()) which evaluates to Optional.empty()
at stub-time; update the stubbing in DynamicModelServiceTest to use a proper
matcher for the second parameter (e.g., any(Long.class) or anyVararg() / any())
so the stub matches the actual call to jdbcTemplate.queryForList(String,
Object...) made by dynamicModelService.getDataById, and update the corresponding
verify(...) to use the same matcher.
- Around line 336-340: The test stubs and verifies a 3-arg jdbcTemplate.update
signature but production calls jdbcTemplate.update(PreparedStatementCreator,
KeyHolder) from DynamicModelService.createData; change the Mockito stubbing to
match the 2-argument method (use any(PreparedStatementCreator.class) and
any(KeyHolder.class)) and update the verify call to verify(jdbcTemplate,
times(1)).update(any(PreparedStatementCreator.class), any(KeyHolder.class)) so
the mock matches the real call and thenReturn(1) continues to provide the
expected int result.

---

Nitpick comments:
In
`@base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java`:
- Around line 45-46: Remove the duplicate Mockito initialization in the test
setup: in DynamicModelServiceTest remove the redundant second call to
MockitoAnnotations.openMocks(this) so only a single invocation remains; ensure
the remaining call is in the appropriate setup method (e.g., `@BeforeEach`) and
that no other test initialization relies on the removed duplicate.
- Around line 47-49: The test uses ReflectUtil.setFieldValue to inject
jdbcTemplate, loginUserContext, and namedParameterJdbcTemplate into
dynamicModelService, but `@InjectMocks` should already perform this injection;
remove the three ReflectUtil.setFieldValue(...) calls and rely on the
`@InjectMocks-driven` injection for dynamicModelService, and if injection still
fails verify that the mocked field names (jdbcTemplate, loginUserContext,
namedParameterJdbcTemplate) match the fields in the DynamicModelService class or
switch the service to constructor injection to make mocks injectable.
- Around line 287-354: The test class contains duplicate test methods
(testCreateDynamicTable, testDropDynamicTable, testDynamicQuery, testCreateData,
testDeleteDataById) that mirror existing tests (createDynamicTable,
dropDynamicTable, dynamicQuery, createData, deleteDataById); remove the
redundant test methods or convert them into clearly-scoped tests by renaming and
adjusting their assertions to reflect distinct scenarios (e.g.,
createDynamicTable_withNullOrderBy, deleteDataById_returnsZeroRows) so they no
longer duplicate coverage; locate the methods by name in DynamicModelServiceTest
and either delete the duplicate methods or rename and modify their setup/verify
expectations to exercise a different behavior, keeping only one authoritative
test per scenario.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d4e53c44-a13b-47b9-b423-ef344d631eca

📥 Commits

Reviewing files that changed from the base of the PR and between 2b97f64 and 7dfdd6b.

📒 Files selected for processing (1)

• base/src/test/java/com/tinyengine/it/dynamic/service/DynamicModelServiceTest.java