Skip to content

Align keyColumn support in <generatedKey> tag #1309

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dparo
Copy link

@dparo dparo commented May 7, 2025

Standardize keyColumn handling in the tag for the following MyBatis Generator targets:

  • MyBatis3Kotlin
  • MyBatis3DynamicSql
  • MyBatis3 ANNOTATEDMAPPER & MIXEDMAPPER

This update brings their behavior in line with the existing implementation in the MyBatis3 XMLMAPPER generator (refer to AbstractXmlElementGenerator::buildInitialInsert).

Standardize keyColumn handling in the <generatedKey> tag for the following MyBatis Generator targets:

- KotlinMyBatis3Kotlin
- MyBatis3DynamicSql
- MyBatis3 ANNOTATEDMAPPER & MIXEDMAPPER

This update brings their behavior in line with the existing implementation
in the MyBatis3 XMLMAPPER generator (refer to AbstractXmlElementGenerator::buildInitialInsert).
@hazendaz hazendaz requested a review from Copilot May 24, 2025 16:51
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Standardize keyColumn handling in the <generatedKey> tag across MyBatis Generator targets to match XMLMAPPER behavior.

  • Append keyColumn attribute to the @Options annotation using Optional and StringUtility.escapeString
  • Introduce necessary imports (Optional, StringUtility) in Kotlin and dynamic SQL fragment generators
  • Update Java mapper generator to include the same keyColumn logic

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
KotlinFragmentGenerator.java Added Optional import and appended , keyColumn="…" to @Options
FragmentGenerator.java Added Optional import and appended , keyColumn="…" to @Options
AbstractJavaMapperMethodGenerator.java Added StringUtility import and appended , keyColumn="…" to @Options
Comments suppressed due to low confidence (3)

core/mybatis-generator-core/src/main/java/org/mybatis/generator/runtime/kotlin/elements/KotlinFragmentGenerator.java:193

  • Add or update tests to cover scenarios where generatedKey.column is provided and omitted, ensuring the keyColumn attribute is correctly emitted or skipped.
sb.append(String.format(", keyColumn=\"%s\"", Optional.ofNullable(gk.getColumn()).map(StringUtility::escapeStringForKotlin).orElse("")));

core/mybatis-generator-core/src/main/java/org/mybatis/generator/codegen/mybatis3/javamapper/elements/AbstractJavaMapperMethodGenerator.java:43

  • Missing import for java.util.Optional. Add import java.util.Optional; so that Optional.ofNullable(...) compiles.
import org.mybatis.generator.internal.util.StringUtility;

core/mybatis-generator-core/src/main/java/org/mybatis/generator/runtime/kotlin/elements/KotlinFragmentGenerator.java:193

  • [nitpick] Consider only appending the , keyColumn="…" segment when gk.getColumn() is non-null and non-empty to avoid emitting an empty keyColumn attribute.
sb.append(String.format(", keyColumn=\"%s\"", Optional.ofNullable(gk.getColumn()).map(StringUtility::escapeStringForKotlin).orElse("")));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant