Align GeoFence to GeoServer3#285
Draft
etj wants to merge 4 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns the GeoFence codebase with GeoServer 3 by restructuring modules and updating package coordinates/namespaces, while modernizing parts of the persistence/services layers and tests.
Changes:
- Migrates many Java packages from
org.geoserver.geofence...toorg.geofence...and introduces newsrc/core/*Maven modules. - Reworks persistence/services wiring toward Spring Java config + Jakarta (
jakarta.persistence,jakarta.xml.bind) APIs. - Replaces/removes legacy REST/login/webtest artifacts and updates tests to JUnit Jupiter + (new) Testcontainers-based infrastructure.
Reviewed changes
Copilot reviewed 227 out of 408 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/modules/rest/client/src/main/java/org/geoserver/geofence/services/rest/AdminRuleServiceHelper.java | Removed legacy REST client helper. |
| src/services/modules/rest/client/pom.xml | Removed legacy REST client module POM. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/model/util/Identifier.java_ | Removed legacy REST API utility model. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/model/config/RESTFullGRUserList.java | Removed legacy REST config model. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/model/config/RESTConfigurationRemapping.java | Removed legacy REST configuration remapping model. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/exception/NotFoundRestEx.java | Removed legacy REST exception type. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/exception/InternalErrorRestEx.java | Removed legacy REST exception type. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/exception/ConflictRestEx.java | Removed legacy REST exception type. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/exception/BadRequestRestEx.java | Removed legacy REST exception type. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/RESTBatchService.java | Removed legacy REST batch service interface. |
| src/services/modules/rest/api/src/main/java/org/geoserver/geofence/services/rest/RESTAdminRuleService.java | Removed legacy REST admin rule service interface. |
| src/services/modules/rest/api/pom.xml | Removed legacy REST API module POM. |
| src/services/modules/pom.xml | Removed legacy modules aggregator POM. |
| src/services/modules/login/impl/src/test/resources/applicationContext-test.xml | Removed legacy login Spring test context. |
| src/services/modules/login/impl/src/test/java/org/geoserver/geofence/login/util/MD5UtilTest.java | Removed legacy login hashing test. |
| src/services/modules/login/impl/src/main/resources/applicationContext.xml | Removed legacy login Spring context. |
| src/services/modules/login/impl/src/main/java/org/geoserver/geofence/login/util/TokenEncoder.java | Removed legacy token encoder. |
| src/services/modules/login/impl/src/main/java/org/geoserver/geofence/login/util/SessionManager.java | Removed legacy session manager. |
| src/services/modules/login/impl/src/main/java/org/geoserver/geofence/login/util/MD5Util.java | Removed legacy MD5 helper. |
| src/services/modules/login/impl/src/main/java/org/geoserver/geofence/login/util/GrantAll.java | Removed legacy auth provider. |
| src/services/modules/login/impl/src/main/java/org/geoserver/geofence/login/LoginServiceImpl.java | Removed legacy login service implementation. |
| src/services/modules/login/impl/pom.xml | Removed legacy login impl module POM. |
| src/services/modules/login/api/src/main/java/org/geoserver/geofence/login/LoginService.java | Removed legacy SOAP login API. |
| src/services/modules/login/api/pom.xml | Removed legacy login API module POM. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/exception/AuthException.java | Removed legacy generic API exception. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/dto/RegisteredUser.java | Removed legacy generic API DTO. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/dto/GrantedAuths.java | Removed legacy generic API DTO. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/dto/Authority.java | Removed legacy auth DTO enum. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/UserRegistry.java | Removed legacy user registry API. |
| src/services/modules/generic-api/src/main/java/org/geoserver/geofence/api/AuthProvider.java | Removed legacy auth provider API. |
| src/services/modules/generic-api/pom.xml | Removed legacy generic-api module POM. |
| src/services/core/webtest/src/main/webapp/WEB-INF/web.xml | Removed legacy webtest web.xml. |
| src/services/core/webtest/src/main/webapp/WEB-INF/remoting-servlet.xml | Removed legacy webtest Spring remoting servlet config. |
| src/services/core/webtest/src/main/resources/log4j2.xml | Removed legacy webtest logging config. |
| src/services/core/webtest/src/main/resources/geofence-datasource.properties | Removed legacy webtest datasource props. |
| src/services/core/webtest/src/main/resources/geofence-datasource-ovr.properties | Removed legacy webtest overrides. |
| src/services/core/webtest/src/main/resources/applicationContext.xml | Removed legacy webtest spring context. |
| src/services/core/services-impl/src/test/resources/geofence-datasource-ovr.properties | Removed legacy test datasource overrides. |
| src/services/core/services-impl/src/test/java/org/geoserver/test/AbstractSpringContextTest.java | Removed legacy Spring context base test. |
| src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/GFUserAdminServiceImplTest.java | Removed legacy service impl test. |
| src/services/core/services-impl/src/main/resources/applicationContext.xml | Removed legacy service impl Spring context. |
| src/services/core/services-api/src/test/java/org/geoserver/geofence/services/util/RuleFilterTest.java | Removed legacy RuleFilter test. |
| src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/ShortUser.java | Removed legacy DTO. |
| src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/ShortAdminRule.java | Removed legacy DTO. |
| src/services/core/services-api/src/main/java/org/geoserver/geofence/services/RuleReaderService.java | Removed legacy services API interface. |
| src/services/core/services-api/src/main/java/org/geoserver/geofence/services/GFUserAdminService.java | Removed legacy GFUser admin service API. |
| src/services/core/services-api/pom.xml | Removed legacy services-api module POM. |
| src/services/core/pom.xml | Removed legacy core aggregator POM. |
| src/services/core/persistence/src/test/resources/geofence-datasource-ovr.properties | Removed legacy persistence test overrides. |
| src/services/core/persistence/src/test/java/org/geoserver/geofence/core/dao/util/DaoRegistryTest.java | Removed legacy DAO registry test. |
| src/services/core/persistence/src/test/java/org/geoserver/geofence/core/dao/UserGroupDAOTest.java | Removed legacy DAO test. |
| src/services/core/persistence/src/test/java/org/geoserver/geofence/core/dao/GFUserDAOTest.java | Removed legacy DAO test. |
| src/services/core/persistence/src/main/resources/geofence-externalize-aux.properties | Removed legacy externalize props. |
| src/services/core/persistence/src/main/java/org/geoserver/geofence/core/dao/util/GeofenceDaoRegistry.java | Removed legacy DAO registry implementation. |
| src/services/core/persistence/src/main/java/org/geoserver/geofence/core/dao/impl/GFUserDAOImpl.java | Removed legacy GF user DAO impl. |
| src/services/core/persistence/src/main/java/org/geoserver/geofence/core/dao/RestrictedGenericDAO.java | Removed legacy restricted DAO API. |
| src/services/core/persistence/src/main/java/org/geoserver/geofence/core/dao/RegistrableDAO.java | Removed legacy marker interface. |
| src/services/core/persistence/src/main/java/org/geoserver/geofence/core/dao/GFUserDAO.java | Removed legacy DAO interface. |
| src/services/core/persistence-pg-test/src/test/resources/log4j.properties | Removed legacy pg test logging config. |
| src/services/core/persistence-pg-test/src/test/resources/geofence-datasource-ovr.properties | Removed legacy pg test overrides. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/dual/MapType.java | Removed legacy JAXB map adapter type. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/dual/MapEntryType.java | Removed legacy JAXB map entry type. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/XMultiPolygonAdapter.java | Removed legacy geometry adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/PolygonAdapter.java | Removed legacy geometry adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/MapAdapter.java | Removed legacy map adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/FKUserAdapter.java | Removed legacy FK adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/FKGSInstanceAdapter.java | Removed legacy FK adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/FK2UserGroupSetAdapter2.java | Removed legacy set adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/FK2UserGroupAdapter.java | Removed legacy FK2 adapter. |
| src/services/core/model/src/main/java/org/geoserver/geofence/core/model/adapter/FK2UserAdapter.java | Removed legacy FK2 adapter. |
| src/services/core/model/pom.xml | Removed legacy model-internal module POM. |
| src/misc/csv2geofence/src/test/resources/log4j.properties | Updated logger namespace. |
| src/misc/csv2geofence/src/main/resources/log4j.properties | Updated logger namespace. |
| src/misc/csv2geofence/src/main/java/org/geoserver/csv2geofence/impl/UsersProcessor.java | Updated imports to new namespaces. |
| src/misc/csv2geofence/src/main/java/org/geoserver/csv2geofence/impl/RulesProcessor.java | Updated imports to new namespaces. |
| src/misc/csv2geofence/src/main/java/org/geoserver/csv2geofence/Runner.java | Updated imports to new namespaces. |
| src/misc/csv2geofence/pom.xml | Updated Maven coordinates/version. |
| src/core/services-impl/src/test/resources/log4j2.xml | Updated logger namespace. |
| src/core/services-impl/src/test/java/org/geofence/core/services/util/CategorizedCircularBufferTest.java | Migrated test package + JUnit Jupiter. |
| src/core/services-impl/src/test/java/org/geofence/core/services/UserGroupAdminServiceImplTest.java | Added JUnit Jupiter service test. |
| src/core/services-impl/src/test/java/org/geofence/core/services/UserAdminServiceImplTest.java | Migrated to new packages + JUnit Jupiter. |
| src/core/services-impl/src/test/java/org/geofence/core/services/AdminRuleAdminServiceImplTest.java | Migrated to new packages + JUnit Jupiter. |
| src/core/services-impl/src/main/java/org/geofence/core/services/util/CategorizedCircularBuffer.java | Migrated package + minor refactors. |
| src/core/services-impl/src/main/java/org/geofence/core/services/util/AccessInfoInternal.java | Migrated package + minor refactors. |
| src/core/services-impl/src/main/java/org/geofence/core/services/config/GeofenceServiceConfig.java | Added Spring Java config for services. |
| src/core/services-impl/src/main/java/org/geofence/core/services/UserGroupAdminServiceImpl.java | Migrated to Spring @Service + new DAO package. |
| src/core/services-impl/src/main/java/org/geofence/core/services/UserAdminServiceImpl.java | Migrated to Spring @Service + new DAO package. |
| src/core/services-impl/src/main/java/org/geofence/core/services/InstanceAdminServiceImpl.java | Migrated to Spring @Service + new DAO package. |
| src/core/services-impl/src/main/java/org/geofence/core/services/DefaultUserResolver.java | Migrated to Spring @Service + SPI package change. |
| src/core/services-impl/src/main/java/org/geofence/core/services/AuthorizationServiceImpl.java | Migrated to Spring @Service + new DAO package. |
| src/core/services-api/src/test/java/org/geofence/core/services/util/RuleFilterTest.java | Added migrated RuleFilter JUnit Jupiter test. |
| src/core/services-api/src/test/java/org/geofence/core/services/util/IPUtilsTest.java | Migrated JUnit test to new package/Jupiter. |
| src/core/services-api/src/main/java/org/geofence/core/services/util/IPUtils.java | Migrated utility package + minor formatting. |
| src/core/services-api/src/main/java/org/geofence/core/services/spi/UserResolver.java | Migrated SPI package + removed deprecated methods. |
| src/core/services-api/src/main/java/org/geofence/core/services/exception/NotFoundServiceEx.java | Migrated exception package + formatting. |
| src/core/services-api/src/main/java/org/geofence/core/services/exception/InternalErrorServiceEx.java | Migrated exception package + formatting. |
| src/core/services-api/src/main/java/org/geofence/core/services/exception/BadRequestServiceEx.java | Migrated exception package + formatting. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/ShortUser.java | Added DTO in new services-api module. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/ShortInstance.java | Migrated DTO + Jakarta JAXB. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/ShortGroup.java | Migrated DTO + Jakarta JAXB. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/ShortAdminRule.java | Added DTO in new services-api module. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/CatalogModeDTO.java | Migrated enum to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/AuthUser.java | Migrated DTO to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/dto/AccessInfo.java | Migrated DTO to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/UserGroupAdminService.java | Migrated service API to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/UserAdminService.java | Migrated service API to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/RuleReaderService.java | Added migrated services API interface. |
| src/core/services-api/src/main/java/org/geofence/core/services/InstanceAdminService.java | Migrated service API to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/GetProviderService.java | Migrated base service API to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/ConfigAdminService.java | Migrated service API to new package. |
| src/core/services-api/src/main/java/org/geofence/core/services/AuthorizationService.java | Migrated service API to new package. |
| src/core/services-api/pom.xml | Added new services-api module POM. |
| src/core/pom.xml | Added new core aggregator POM. |
| src/core/persistence/src/test/resources/log4j2.xml | Updated logger namespace. |
| src/core/persistence/src/test/java/org/geofence/core/db/dao/util/PwEncoderTest.java | Migrated test package/JUnit Jupiter. |
| src/core/persistence/src/test/java/org/geofence/core/db/dao/UserDAOTest.java | Migrated test package/JUnit Jupiter. |
| src/core/persistence/src/test/java/org/geofence/core/db/TestContainerBootstrap.java | Added Testcontainers bootstrap for tests. |
| src/core/persistence/src/test/java/org/geofence/core/db/BaseContainerTest.java | Added base Testcontainers + Spring test config. |
| src/core/persistence/src/main/java/org/geofence/core/db/utils/SchemaExporter.java | Migrated schema exporter package + model imports. |
| src/core/persistence/src/main/java/org/geofence/core/db/datasource/DynamicRoutingDataSource.java | Added dynamic routing datasource. |
| src/core/persistence/src/main/java/org/geofence/core/db/datasource/DatabaseRegistry.java | Added runtime database registry for routing datasource. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/util/PwEncoder.java | Migrated persistence util package. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/search/SearchUtil.java | Migrated search util package + formatting. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/search/Search.java | Migrated to Jakarta persistence. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/search/LongSearch.java | Migrated to Jakarta persistence. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/search/BaseSearch.java | Migrated to Jakarta persistence + formatting. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/UserGroupDAOImpl.java | Migrated DAO impl + Spring component metadata. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/RuleLimitsDAOImpl.java | Migrated DAO impl + Spring repository metadata. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/RuleDAOImpl.java | Migrated DAO impl + API changes to single-entity persist. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/LayerDetailsDAOImpl.java | Migrated DAO impl + API changes to single-entity persist. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/GSInstanceDAOImpl.java | Migrated DAO impl + API changes to single-entity persist. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/BaseDAOImpl.java | Replaced legacy BaseDAO with Jakarta-based BaseDAOImpl. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/impl/AdminRuleDAOImpl.java | Migrated DAO impl + API changes to single-entity persist. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/UserGroupDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/SearchableDAO.java | Migrated DAO API package. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/RuleLimitsDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/RuleDAO.java | Migrated DAO API package. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/PrioritizableDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/LayerDetailsDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/GSUserDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/GSInstanceDAO.java | Migrated DAO API to new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/DuplicateKeyException.java | Migrated exception package + formatting. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/BaseDAO.java | Added new base DAO interface. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/AllowedStylesProvider.java | Migrated interface package + indentation fix. |
| src/core/persistence/src/main/java/org/geofence/core/db/dao/AdminRuleDAO.java | Migrated DAO API package. |
| src/core/persistence/src/main/java/org/geofence/core/db/config/HibernateConfig.java | Added transaction manager config. |
| src/core/persistence/src/main/java/org/geofence/core/db/config/GeofenceUserDAOSelector.java | Added DAO selector config. |
| src/core/persistence/src/main/java/org/geofence/core/db/config/GeofencePersistenceConfig.java | Added Spring Java config for persistence/JPA. |
| src/core/model/src/test/java/org/geofence/core/model/util/PwEncoderTest.java | Migrated model util test to JUnit Jupiter. |
| src/core/model/src/test/java/org/geofence/core/model/IPAddressRangeTest.java | Migrated model test to JUnit Jupiter. |
| src/core/model/src/test/java/org/geofence/core/model/Base64EncodersTest.java | Migrated JAXB import and JUnit Jupiter assertions. |
| src/core/model/src/main/java/org/geofence/core/model/util/package-info.java | Fixed package-info package declaration. |
| src/core/model/src/main/java/org/geofence/core/model/util/PwEncoder.java | Migrated model util to Jakarta JAXB. |
| src/core/model/src/main/java/org/geofence/core/model/util/EWKTParser.java | Migrated package + minor signature cleanup. |
| src/core/model/src/main/java/org/geofence/core/model/package-info.java | Added (commented) placeholder package-info. |
| src/core/model/src/main/java/org/geofence/core/model/enums/package-info.java | Fixed enums package-info package declaration. |
| src/core/model/src/main/java/org/geofence/core/model/enums/ValueType.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/enums/SpatialFilterType.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/enums/LayerType.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/enums/KnownServices.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/enums/InsertPosition.java | Migrated package. |
| src/core/model/src/main/java/org/geofence/core/model/enums/GrantType.java | Migrated package. |
| src/core/model/src/main/java/org/geofence/core/model/enums/CatalogMode.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/enums/AdminGrantType.java | Migrated package. |
| src/core/model/src/main/java/org/geofence/core/model/enums/AccessType.java | Migrated package. |
| src/core/model/src/main/java/org/geofence/core/model/adapter/package-info.java | Fixed adapter package-info package declaration. |
| src/core/model/src/main/java/org/geofence/core/model/adapter/MultiPolygonAdapter.java | Migrated adapter package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/adapter/IdentifiableAdapter.java | Migrated JAXB adapter to Jakarta. |
| src/core/model/src/main/java/org/geofence/core/model/adapter/GeometryAdapter.java | Migrated JAXB adapter to Jakarta + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/adapter/FKUserGroupAdapter.java | Migrated adapter package. |
| src/core/model/src/main/java/org/geofence/core/model/RuleLimits.java | Migrated entity to Jakarta + refactored rule mapping. |
| src/core/model/src/main/java/org/geofence/core/model/Prioritizable.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/LayerAttribute.java | Migrated entity to Jakarta + doc formatting. |
| src/core/model/src/main/java/org/geofence/core/model/Identifiable.java | Migrated package + formatting. |
| src/core/model/src/main/java/org/geofence/core/model/IPRangeProvider.java | Migrated package + formatting. |
| src/core/model/pom.xml | Added new model module POM under new groupId/version. |
Comments suppressed due to low confidence (1)
src/core/persistence/src/main/java/org/geofence/core/db/config/GeofenceUserDAOSelector.java:1
- Selecting DAO implementations via
daos.get(0)is brittle: bean ordering is not guaranteed unless explicitly controlled, and this will also throw at runtime if no implementations are registered. A more robust approach is to mark the preferred DAO as@Primary, or select by@Qualifier/bean name, or enforce deterministic ordering (e.g., sort by@Orderand validate the result) and fail with a clear message when no candidate is available.
package org.geofence.core.db.config;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+9
| // @XmlJavaTypeAdapters({ | ||
| // @XmlJavaTypeAdapter(type = LocalDateTime.class, value = LocalDateTimeAdapter.class) | ||
| // }) | ||
| // package org.geofence.core.model; | ||
| // | ||
| // import java.time.LocalDateTime; | ||
| // import jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapter; | ||
| // import jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapters; | ||
| // import org.geofence.core.model.adapter.LocalDateTimeAdapter; |
There was a problem hiding this comment.
This package-info.java has no actual package ...; declaration (it’s fully commented out). A package-info.java file without a package statement will fail Java compilation. Either delete this file entirely, or restore a real package declaration (and any required annotations/imports) so it becomes valid Java source.
Suggested change
| // @XmlJavaTypeAdapters({ | |
| // @XmlJavaTypeAdapter(type = LocalDateTime.class, value = LocalDateTimeAdapter.class) | |
| // }) | |
| // package org.geofence.core.model; | |
| // | |
| // import java.time.LocalDateTime; | |
| // import jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapter; | |
| // import jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapters; | |
| // import org.geofence.core.model.adapter.LocalDateTimeAdapter; | |
| @jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapters({ | |
| @jakarta.xml.bind.annotation.adapters.XmlJavaTypeAdapter( | |
| type = java.time.LocalDateTime.class, | |
| value = org.geofence.core.model.adapter.LocalDateTimeAdapter.class | |
| ) | |
| }) | |
| package org.geofence.core.model; |
| public InternalErrorServiceEx(String message) | ||
| { | ||
| public InternalErrorServiceEx(String message) { | ||
| super(Response.Status.BAD_REQUEST); |
There was a problem hiding this comment.
InternalErrorServiceEx is mapped to Response.Status.BAD_REQUEST (400), which contradicts the class name and typical semantics (should be 500). This will cause internal errors to be reported as client errors. Use Response.Status.INTERNAL_SERVER_ERROR here.
Suggested change
| super(Response.Status.BAD_REQUEST); | |
| super(Response.Status.INTERNAL_SERVER_ERROR); |
| name = "gf_rule_limits", | ||
| uniqueConstraints = | ||
| @UniqueConstraint( | ||
| columnNames = "rule_id", // @InternalModel |
There was a problem hiding this comment.
The table uniqueConstraints references a rule_id column, but the relationship is mapped via @JoinColumn(name = \"id\") (and @MapsId). This introduces a schema mismatch (constraint on a non-existent column) and will break schema generation / migrations. Align the constraint and join column naming consistently (e.g., make the FK column rule_id and map it accordingly, or update the unique constraint to match the actual join column).
Suggested change
| columnNames = "rule_id", // @InternalModel | |
| columnNames = "id", // @InternalModel |
Comment on lines
+57
to
+60
| @MapsId | ||
| @OneToOne(optional = false) | ||
| @Check(constraints = "rule.access='LIMIT'") // ??? check this | ||
| @JoinColumn(foreignKey = @ForeignKey(name = "fk_limits_rule")) | ||
| private Rule rule; | ||
| @JoinColumn(name = "id", foreignKey = @ForeignKey(name = "fk_limits_rule")) | ||
| Rule rule; |
There was a problem hiding this comment.
The table uniqueConstraints references a rule_id column, but the relationship is mapped via @JoinColumn(name = \"id\") (and @MapsId). This introduces a schema mismatch (constraint on a non-existent column) and will break schema generation / migrations. Align the constraint and join column naming consistently (e.g., make the FK column rule_id and map it accordingly, or update the unique constraint to match the actual join column).
Comment on lines
+3
to
+15
| import org.hibernate.SessionFactory; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.orm.jpa.hibernate.HibernateTransactionManager; | ||
| import org.springframework.transaction.PlatformTransactionManager; | ||
|
|
||
| @Configuration | ||
| public class HibernateConfig { | ||
|
|
||
| @Bean | ||
| public PlatformTransactionManager transactionManager(SessionFactory sessionFactory) { | ||
|
|
||
| return new HibernateTransactionManager(sessionFactory); |
There was a problem hiding this comment.
This config declares a transactionManager(SessionFactory) bean, but the persistence setup in GeofencePersistenceConfig creates a JPA EntityManagerFactory + JpaTransactionManager, and it doesn’t define a SessionFactory bean. As-is, this is likely to fail application context startup due to an unsatisfied SessionFactory dependency (or introduce multiple transaction managers with ambiguous wiring). Prefer removing this config, or define/provide a SessionFactory bean explicitly, or consolidate on the existing geofenceTransactionManager.
Suggested change
| import org.hibernate.SessionFactory; | |
| import org.springframework.context.annotation.Bean; | |
| import org.springframework.context.annotation.Configuration; | |
| import org.springframework.orm.jpa.hibernate.HibernateTransactionManager; | |
| import org.springframework.transaction.PlatformTransactionManager; | |
| @Configuration | |
| public class HibernateConfig { | |
| @Bean | |
| public PlatformTransactionManager transactionManager(SessionFactory sessionFactory) { | |
| return new HibernateTransactionManager(sessionFactory); | |
| /** | |
| * Legacy placeholder for Hibernate configuration. | |
| * | |
| * The actual transaction management is configured elsewhere | |
| * (for example via a JpaTransactionManager). | |
| */ | |
| public final class HibernateConfig { | |
| private HibernateConfig() { | |
| // Prevent instantiation |
Comment on lines
+25
to
+29
| private DynamicRoutingDataSource routingDataSource; | ||
|
|
||
| public GeofencePersistenceConfig() { | ||
| routingDataSource = new DynamicRoutingDataSource(); | ||
| } |
There was a problem hiding this comment.
DynamicRoutingDataSource routingDataSource is instantiated but never exposed as a Spring bean nor used as the DataSource returned by dataSource(...). At the same time, DatabaseRegistry is designed to mutate a DynamicRoutingDataSource. If dynamic routing is intended, the config should wire routingDataSource as the primary DataSource and register at least one default target datasource; otherwise, this routing infrastructure should be removed to avoid dead/unused code paths that will confuse future maintenance.
Comment on lines
+47
to
+60
| @Bean | ||
| public DataSource dataSource( | ||
| @Value("${geofence.datasource.url}") String url, | ||
| @Value("${geofence.datasource.username}") String username, | ||
| @Value("${geofence.datasource.password}") String password, | ||
| @Value("${geofence.datasource.driver}") String driver) { | ||
|
|
||
| HikariDataSource dataSource = new HikariDataSource(); | ||
| dataSource.setJdbcUrl(url); | ||
| dataSource.setUsername(username); | ||
| dataSource.setPassword(password); | ||
| dataSource.setDriverClassName(driver); | ||
| return dataSource; | ||
| } |
There was a problem hiding this comment.
DynamicRoutingDataSource routingDataSource is instantiated but never exposed as a Spring bean nor used as the DataSource returned by dataSource(...). At the same time, DatabaseRegistry is designed to mutate a DynamicRoutingDataSource. If dynamic routing is intended, the config should wire routingDataSource as the primary DataSource and register at least one default target datasource; otherwise, this routing infrastructure should be removed to avoid dead/unused code paths that will confuse future maintenance.
| LinkedList<T> typedList = typedLists.get(key); | ||
| if (typedList == null) | ||
| return Collections.EMPTY_LIST; | ||
| if (typedList == null) return Collections.EMPTY_LIST; |
There was a problem hiding this comment.
Returning Collections.EMPTY_LIST is a raw type and can trigger unchecked warnings / type-safety issues. Prefer returning Collections.emptyList() (which is generically typed) to keep the method type-safe.
Suggested change
| if (typedList == null) return Collections.EMPTY_LIST; | |
| if (typedList == null) return Collections.emptyList(); |
| ret.setAllowedStyles( | ||
| allowedStyles == null | ||
| ? null | ||
| : new HashSet(allowedStyles)); // new Set will prevent marshalling hibernate classes |
There was a problem hiding this comment.
This uses a raw new HashSet(allowedStyles), which loses generic type information and produces unchecked warnings. Use a parameterized copy (new HashSet<>(allowedStyles)) to preserve type safety.
Suggested change
| : new HashSet(allowedStyles)); // new Set will prevent marshalling hibernate classes | |
| : new HashSet<>(allowedStyles)); // new Set will prevent marshalling hibernate classes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With the first commit the project build and tests pass.
Todo: