Skip to content
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

Issues when mixing @Arg with and without name #2932

Open
dirk-tf opened this issue Aug 10, 2023 · 2 comments
Open

Issues when mixing @Arg with and without name #2932

dirk-tf opened this issue Aug 10, 2023 · 2 comments

Comments

@dirk-tf
Copy link

dirk-tf commented Aug 10, 2023

i was using simple @arg(column, type). Tried to add name=... to some args that had the same type, to make things more resilient. this broke the mapper.

  1. having name's on just some args of a resultMap doesn't seem supported (would be nice)
  2. since it isn't, can an additional check be added for such situations that throws something better worded about this not being supported?
  3. a broken mapper may actually be created if another constructor matches the args with name. see test case below, that should throw a different exception right now

MyBatis version

4.5.13, current master

Test case or example project

public class PartialArgsTest {

  record Post(String section, Timestamp createdOn, String subject) {
    public Post(String section, Timestamp subject) {
      this(null, null, null); // absurd, but doesn't matter anyway
    }
  }

  interface Mapper {
    @Select("SELECT * FROM post")
    @Arg(column = "SECTION", name = "section", javaType = String.class)
    @Arg(column = "CREATED_ON", javaType = Timestamp.class)
    @Arg(column = "SUBJECT", name = "subject", javaType = String.class)
    List<Post> getPosts();
  }

  @Test()
  void shouldGetDataOrFailOnMapperCreation() throws Exception {
    final Environment environment = new Environment("test", new JdbcTransactionFactory(),
      BaseDataTest.createBlogDataSource());
    final Configuration config = new Configuration(environment);

    assertThrows(BuilderException.class, () -> {
      config.addMapper(Mapper.class); // this should throw, if all or no @Arg's must have names

      // this continuation should not throw if partial names are ok (but actually throws right now):
      var sqlSessionFactory = new SqlSessionFactoryBuilder().build(config);
      try (SqlSession session = sqlSessionFactory.openSession()) {
        Mapper mapper = session.getMapper(Mapper.class);
        var result = mapper.getPosts();
      }
    });
  }
}
@wickedtornado
Copy link

wickedtornado commented Aug 10, 2023

record Post(String section, Timestamp createdOn, String subject) {
public Post(String section, Timestamp subject) {
this(null, null, null);
}
}
Something is wrong here, seems to be a incorrect constructor

@dirk-tf
Copy link
Author

dirk-tf commented Aug 10, 2023

Something is wrong here

This minimalist example is absurd, yes, since it is stripped down to only the parts necessary to highlight the bug.

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

No branches or pull requests

2 participants