Copilot commented on code in PR #11066: URL: https://github.com/apache/gravitino/pull/11066#discussion_r3238938016
########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/mapper/it/BackendTypes.java: ########## @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; Review Comment: This file is located at `.../mapper/it/BackendTypes.java` but declares the package `org.apache.gravitino.storage.relational`. Java requires the package declaration to match the directory path, so this file will fail to compile. Either move the file to `.../storage/relational/BackendTypes.java` or change the package declaration to `org.apache.gravitino.storage.relational.mapper.it` (and update all importers accordingly). ########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/mapper/TestIdpUserMetaSQLProviderFactory.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import org.apache.gravitino.storage.relational.BackendTypes; +import org.apache.gravitino.storage.relational.mapper.provider.base.IdpUserMetaBaseSQLProvider; +import org.apache.gravitino.storage.relational.mapper.provider.postgresql.IdpUserMetaPostgreSQLProvider; +import org.junit.jupiter.api.TestTemplate; + +@BackendTypes({"h2"}) +public class TestIdpUserMetaSQLProviderFactory extends IdpMapperTestBase { + + @TestTemplate + void testGetProviderReturnsBackendSpecificProvider() { + IdpUserMetaBaseSQLProvider provider = IdpUserMetaSQLProviderFactory.getProvider(); + + switch (backendType) { + case "h2": + assertInstanceOf(IdpUserMetaBaseSQLProvider.class, provider); Review Comment: `assertInstanceOf(IdpUserMetaBaseSQLProvider.class, provider)` will also succeed if the factory returns an instance of any subclass (e.g. `IdpUserMetaPostgreSQLProvider`), so this assertion doesn't actually verify that the H2 backend returns the base provider specifically. Consider asserting on the exact class (e.g. `assertEquals(IdpUserMetaBaseSQLProvider.class, provider.getClass())`). ########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/mapper/it/IdpUserMetaMapperTest.java: ########## @@ -0,0 +1,125 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertIterableEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Comparator; +import java.util.List; +import org.apache.gravitino.storage.relational.po.IdpUserPO; + +interface IdpUserMetaMapperTest { + + default void testInsertIdpUserAndSelectIdpUser() { + IdpMapperTestBase testBase = testBase(); + IdpUserPO firstUser = testBase.insertUser(1L, "alice", "hash-a", 1L, 0L, 0L); + + assertEquals(firstUser, testBase.idpUserMetaMapper.selectIdpUser("alice")); + assertNull(testBase.idpUserMetaMapper.selectIdpUser("unknown")); + } + + default void testSelectIdpUsers() { + IdpMapperTestBase testBase = testBase(); + IdpUserPO firstUser = testBase.insertUser(1L, "alice", "hash-a", 1L, 0L, 0L); + IdpUserPO secondUser = testBase.insertUser(2L, "bob", "hash-b", 1L, 0L, 0L); + + List<IdpUserPO> users = testBase.idpUserMetaMapper.selectIdpUsers(List.of("bob", "alice")); + users.sort(Comparator.comparing(IdpUserPO::getUserId)); + assertIterableEquals(List.of(firstUser, secondUser), users); + assertTrue(testBase.idpUserMetaMapper.selectIdpUsers(List.of()).isEmpty()); + assertTrue(testBase.idpUserMetaMapper.selectIdpUsers(null).isEmpty()); + } + + default void testSelectIdpUsersIgnoresDeletedUsers() { + IdpMapperTestBase testBase = testBase(); + IdpUserPO activeUser = testBase.insertUser(1L, "alice", "hash-a", 1L, 0L, 0L); + testBase.insertUser(2L, "bob", "hash-b", 1L, 0L, 10L); + + assertIterableEquals( + List.of(activeUser), testBase.idpUserMetaMapper.selectIdpUsers(List.of("alice", "bob"))); + assertNull(testBase.idpUserMetaMapper.selectIdpUser("bob")); + } + + default void testUpdateIdpUserPassword() { + IdpMapperTestBase testBase = testBase(); + testBase.insertUser(1L, "alice", "hash-a", 1L, 0L, 0L); + + assertEquals(1, testBase.idpUserMetaMapper.updateIdpUserPassword(1L, "hash-a-2")); + assertEquals("hash-a-2", testBase.idpUserMetaMapper.selectIdpUser("alice").getPasswordHash()); + assertEquals(1L, testBase.idpUserMetaMapper.selectIdpUser("alice").getCurrentVersion()); + assertEquals(0L, testBase.idpUserMetaMapper.selectIdpUser("alice").getLastVersion()); + } + + default void testUpdateIdpUserPasswordReturnsZeroForVersionMismatch() { + IdpMapperTestBase testBase = testBase(); + testBase.insertUser(1L, "alice", "hash-a", 1L, 0L, 0L); + + assertEquals(1, testBase.idpUserMetaMapper.updateIdpUserPassword(1L, "hash-a-2")); + assertEquals("hash-a-2", testBase.idpUserMetaMapper.selectIdpUser("alice").getPasswordHash()); + assertEquals(1L, testBase.idpUserMetaMapper.selectIdpUser("alice").getCurrentVersion()); + assertEquals(0L, testBase.idpUserMetaMapper.selectIdpUser("alice").getLastVersion()); Review Comment: The test name implies it covers the case where `updateIdpUserPassword` returns zero due to a version mismatch, but the body is identical to `testUpdateIdpUserPassword` and never triggers a mismatch — it asserts the update returns `1`, not `0`. Either change the name/intent to reflect what is actually tested, or implement an actual version-mismatch scenario (e.g. by simulating a stale `currentVersion`) and assert the expected zero result. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/storage/relational/mapper/IdpUserMetaMapper.java: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; + +import java.util.List; +import org.apache.gravitino.storage.relational.po.IdpUserPO; +import org.apache.ibatis.annotations.DeleteProvider; +import org.apache.ibatis.annotations.InsertProvider; +import org.apache.ibatis.annotations.Param; +import org.apache.ibatis.annotations.SelectProvider; +import org.apache.ibatis.annotations.UpdateProvider; + +/** + * A MyBatis mapper for built-in IdP user metadata operations. + * + * <p>This interface defines the SQL statements MyBatis executes for the built-in IdP user metadata + * store. The SQLs can be provided by XML files or annotations on this mapper interface. See the <a + * href="https://mybatis.org/mybatis-3/getting-started.html";>MyBatis getting started guide</a>. + */ +public interface IdpUserMetaMapper { + String IDP_USER_TABLE_NAME = "idp_user_meta"; + + @SelectProvider(type = IdpUserMetaSQLProviderFactory.class, method = "selectIdpUser") + IdpUserPO selectIdpUser(@Param("username") String username); + + @SelectProvider(type = IdpUserMetaSQLProviderFactory.class, method = "selectIdpUsers") + List<IdpUserPO> selectIdpUsers(@Param("usernames") List<String> usernames); + + @InsertProvider(type = IdpUserMetaSQLProviderFactory.class, method = "insertIdpUser") + void insertIdpUser(@Param("userMeta") IdpUserPO userPO); + + @UpdateProvider(type = IdpUserMetaSQLProviderFactory.class, method = "updateIdpUserPassword") + Integer updateIdpUserPassword( + @Param("userId") Long userId, @Param("passwordHash") String passwordHash); + + @UpdateProvider(type = IdpUserMetaSQLProviderFactory.class, method = "softDeleteIdpUser") + void softDeleteIdpUser(@Param("userId") Long userId); Review Comment: `softDeleteIdpUser` returns `void`, but `updateIdpUserPassword` and `deleteIdpUserMetasByLegacyTimeline` both return `Integer` (affected rows). Callers cannot tell whether the soft-delete actually matched a row, which is a useful signal for idempotency / not-found handling. Consider returning `Integer` here for consistency with the other write methods. ########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/mapper/it/IdpUserMetaMapperTest.java: ########## @@ -0,0 +1,125 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; Review Comment: The file lives under `.../mapper/it/` but declares the package `org.apache.gravitino.storage.relational.mapper`. This package/directory mismatch will cause a Java compile error. Update the package to `...mapper.it` or move the file up one directory. The same issue applies to `TestIdpUserMetaMapperH2.java`, `TestIdpUserMetaMapperMySQL.java`, and `TestIdpUserMetaMapperPostgreSQL.java` in the same directory. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/IdpUserMetaBaseSQLProvider.java: ########## @@ -0,0 +1,97 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper.provider.base; + +import java.util.List; +import org.apache.gravitino.storage.relational.mapper.IdpUserMetaMapper; +import org.apache.gravitino.storage.relational.po.IdpUserPO; +import org.apache.ibatis.annotations.Param; + +public class IdpUserMetaBaseSQLProvider { + + public String selectIdpUser(@Param("username") String username) { + return "SELECT user_id as userId, user_name as userName, password_hash as passwordHash," + + " current_version as currentVersion," + + " last_version as lastVersion, deleted_at as deletedAt" + + " FROM " + + IdpUserMetaMapper.IDP_USER_TABLE_NAME + + " WHERE user_name = #{username} AND deleted_at = 0"; + } + + public String selectIdpUsers(@Param("usernames") List<String> usernames) { + return "<script>" + + "SELECT user_id as userId, user_name as userName, password_hash as passwordHash," + + " current_version as currentVersion," + + " last_version as lastVersion, deleted_at as deletedAt" + + " FROM " + + IdpUserMetaMapper.IDP_USER_TABLE_NAME + + " WHERE deleted_at = 0 " + + "<choose>" + + "<when test='usernames != null and usernames.size() > 0'>" + + "AND user_name IN (" + + "<foreach item='item' collection='usernames' separator=','>" + + "#{item}" + + "</foreach>" + + ") " + + "</when>" + + "<otherwise>" + + "AND 1 = 0 " + + "</otherwise>" + + "</choose>" + + "</script>"; + } + + public String insertIdpUser(@Param("userMeta") IdpUserPO userPO) { + return "INSERT INTO " + + IdpUserMetaMapper.IDP_USER_TABLE_NAME + + " (user_id, user_name, password_hash, current_version, last_version, deleted_at)" + + " VALUES (" + + " #{userMeta.userId}," + + " #{userMeta.userName}," + + " #{userMeta.passwordHash}," + + " #{userMeta.currentVersion}," + + " #{userMeta.lastVersion}," + + " #{userMeta.deletedAt}" + + " )"; + } + + public String updateIdpUserPassword( + @Param("userId") Long userId, @Param("passwordHash") String passwordHash) { + return "UPDATE " + + IdpUserMetaMapper.IDP_USER_TABLE_NAME + + " SET password_hash = #{passwordHash}" + + " WHERE user_id = #{userId}" + + " AND deleted_at = 0"; + } + + public String softDeleteIdpUser(@Param("userId") Long userId) { + return "UPDATE " + + IdpUserMetaMapper.IDP_USER_TABLE_NAME + + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" Review Comment: `deleted_at` is a `BIGINT`-like timestamp column, but `UNIX_TIMESTAMP() * 1000.0` produces a floating-point value (the `.0` literal forces double arithmetic). This relies on implicit truncation, which differs between MySQL and H2 (in MYSQL mode H2 may round rather than truncate). Use integer arithmetic — e.g. `UNIX_TIMESTAMP() * 1000` — or `CAST(... AS BIGINT)` to match the PostgreSQL override's behavior and ensure consistent millisecond values across backends. ########## plugins/idp-basic/src/main/java/org/apache/gravitino/storage/relational/mapper/IdpUserMetaSQLProviderFactory.java: ########## @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; + +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; +import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType; +import org.apache.gravitino.storage.relational.mapper.provider.base.IdpUserMetaBaseSQLProvider; +import org.apache.gravitino.storage.relational.mapper.provider.postgresql.IdpUserMetaPostgreSQLProvider; +import org.apache.gravitino.storage.relational.po.IdpUserPO; +import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.ibatis.annotations.Param; + +public class IdpUserMetaSQLProviderFactory { + private static final Map<JDBCBackendType, IdpUserMetaBaseSQLProvider> + IDP_USER_META_SQL_PROVIDER_MAP = + ImmutableMap.of( + JDBCBackendType.MYSQL, new IdpUserMetaMySQLProvider(), + JDBCBackendType.H2, new IdpUserMetaBaseSQLProvider(), + JDBCBackendType.POSTGRESQL, new IdpUserMetaPostgreSQLProvider()); + + public static IdpUserMetaBaseSQLProvider getProvider() { + String databaseId = + SqlSessionFactoryHelper.getInstance() + .getSqlSessionFactory() + .getConfiguration() + .getDatabaseId(); + + JDBCBackendType jdbcBackendType = JDBCBackendType.fromString(databaseId); + return IDP_USER_META_SQL_PROVIDER_MAP.get(jdbcBackendType); Review Comment: If `JDBCBackendType.fromString(databaseId)` returns a value not present in `IDP_USER_META_SQL_PROVIDER_MAP`, this method silently returns `null`, which will surface later as a confusing `NullPointerException` at the call site. Consider throwing an explicit `IllegalArgumentException` (or similar) with a message identifying the unsupported backend. ########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/mapper/TestIdpUserMetaSQLProviderFactory.java: ########## @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational.mapper; + +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import org.apache.gravitino.storage.relational.BackendTypes; +import org.apache.gravitino.storage.relational.mapper.provider.base.IdpUserMetaBaseSQLProvider; +import org.apache.gravitino.storage.relational.mapper.provider.postgresql.IdpUserMetaPostgreSQLProvider; +import org.junit.jupiter.api.TestTemplate; + +@BackendTypes({"h2"}) +public class TestIdpUserMetaSQLProviderFactory extends IdpMapperTestBase { + + @TestTemplate + void testGetProviderReturnsBackendSpecificProvider() { + IdpUserMetaBaseSQLProvider provider = IdpUserMetaSQLProviderFactory.getProvider(); + + switch (backendType) { + case "h2": + assertInstanceOf(IdpUserMetaBaseSQLProvider.class, provider); + break; + case "postgresql": + assertInstanceOf(IdpUserMetaPostgreSQLProvider.class, provider); + break; + case "mysql": + assertInstanceOf(IdpUserMetaSQLProviderFactory.IdpUserMetaMySQLProvider.class, provider); + break; Review Comment: The class is annotated `@BackendTypes({"h2"})`, so only the `h2` branch ever executes — the `postgresql` and `mysql` cases are dead code and the factory dispatch logic isn't actually verified for those backends. Either remove the `@BackendTypes({"h2"})` restriction (so all configured backends run) or split out separate test classes per backend, similar to the other `TestIdpUserMetaMapperXxx` classes. ########## plugins/idp-basic/src/main/resources/META-INF/services/org.apache.gravitino.storage.relational.mapper.provider.MapperPackageProvider: ########## @@ -0,0 +1,17 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. Review Comment: There is no blank line (or trailing newline) separating the license header from the service class entry, and the file appears to lack a trailing newline. While `ServiceLoader` tolerates this, some tools and editors expect a final newline; please add a blank line between the header and the class entry and ensure the file ends with a newline for consistency with other service files in this repo. ########## plugins/idp-basic/src/test/java/org/apache/gravitino/storage/relational/BackendTestExtension.java: ########## @@ -0,0 +1,167 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.gravitino.storage.relational; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.Comparator; +import java.util.List; +import java.util.stream.Stream; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.apache.gravitino.integration.test.util.BaseIT; +import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.Extension; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.jupiter.api.extension.TestTemplateInvocationContext; +import org.junit.jupiter.api.extension.TestTemplateInvocationContextProvider; + +class BackendTestExtension implements TestTemplateInvocationContextProvider { + private static final String DOCKER_TEST_FLAG = "dockerTest"; + + @Override + public boolean supportsTestTemplate(ExtensionContext context) { + return true; + } + + @Override + public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContexts( + ExtensionContext context) { + BackendTypes backendTypes = context.getRequiredTestClass().getAnnotation(BackendTypes.class); + List<String> backends = + backendTypes != null + ? List.of(backendTypes.value()) + : ("true".equalsIgnoreCase(System.getenv(DOCKER_TEST_FLAG)) + ? List.of("h2", "mysql", "postgresql") + : List.of("h2")); Review Comment: `@BackendTypes` is only fetched via `getAnnotation` (not `findAnnotation`), so subclasses that inherit `@BackendTypes` indirectly will not be detected unless the annotation is `@Inherited`. The current `BackendTypes` declaration in this PR does not include `@Inherited`, so any subclassing pattern that relies on inherited backend constraints will silently fall back to the env-based default. Consider using `AnnotationSupport.findAnnotation(...)` or adding `@Inherited` to the annotation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
