Skip to content

feat: Convert common module XML Spring configs to Java @Configuration classes#7

Open
devin-ai-integration[bot] wants to merge 5 commits intodevelop-7.0.xfrom
devin/ticket-2.1-common-xml-to-java-config
Open

feat: Convert common module XML Spring configs to Java @Configuration classes#7
devin-ai-integration[bot] wants to merge 5 commits intodevelop-7.0.xfrom
devin/ticket-2.1-common-xml-to-java-config

Conversation

@devin-ai-integration
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot commented Apr 17, 2026

A Brief Overview

This is the first step in migrating Broadleaf's Spring XML configuration to Java @Configuration classes. The common module is the foundation — all other modules depend on it — so it must be converted first.

All 10 XML application context files in common now have equivalent Java @Configuration classes in org.broadleafcommerce.common.config.javaconfig. The original XML files are kept in place and unchanged until all downstream modules are migrated.

Add Labels:

  • Enhancement
  • Status: ready-for-code-review

Mapping of XML → Java config:

XML File Java Config Class
bl-common-applicationContext-persistence.xml CommonPersistenceConfiguration
bl-common-applicationContext.xml CommonConfiguration
bl-common-applicationContext-entity.xml CommonEntityConfiguration
bl-common-applicationContext-mbeans.xml CommonMBeansConfiguration
bl-common-applicationContext-servlet.xml CommonServletConfiguration
bl-common-applicationContext-wrapper.xml CommonWrapperConfiguration
blc-config/admin/framework/bl-common-admin-applicationContext.xml CommonAdminConfiguration
blc-config/admin/framework/bl-common-admin-applicationContext-servlet.xml CommonAdminServletConfiguration
blc-config/site/framework/bl-common-applicationContext.xml CommonSiteConfiguration
blc-config/site/framework/bl-common-applicationContext-servlet.xml CommonSiteServletConfiguration

Verification: mvn compile -DskipTests -pl common passes with no errors.

Updates since initial revision:

  • Added missing blStatisticsService bean to CommonMBeansConfiguration. The XML defined this bean with appName and adapter setter injection, but it was omitted in the initial commit. Without it, StatisticsServiceImpl.adapter remains null and activateLogging()/disableLogging() become no-ops.

  • Excluded javaconfig package from CommonConfiguration's @ComponentScan. Without this filter, all @Configuration classes in the javaconfig package (CommonServletConfiguration, CommonSiteConfiguration, CommonAdminConfiguration, CommonEntityConfiguration, etc.) were being picked up by the root context scan. This broke Spring context isolation — notably, CommonServletConfiguration re-scans the very web.controller and web.config packages that CommonConfiguration explicitly excludes. The fix adds org.broadleafcommerce.common.config.javaconfig to the exclude filters. Configs that should be loaded in the root context (CommonPersistenceConfiguration, CommonWrapperConfiguration) are already explicitly @Import-ed and unaffected.

  • Defined PersistenceUnitPostProcessors as Spring-managed @Bean methods in CommonPersistenceConfiguration. Previously, JPAPropertiesPersistenceUnitPostProcessor, ORMConfigPersistenceUnitPostProcessor, and JCachePersistenceUnitPostProcessor were instantiated via new inside the list factory bean method, bypassing Spring dependency injection. All three classes rely heavily on @Value, @Resource, @Autowired, and @PostConstruct — none of which fire on plain new instances. Each is now a separate @Bean and injected into the blPersistenceUnitPostProcessors list bean. Note: unlike the original XML inner <bean> elements (which are anonymous), these are now named top-level beans — this is a minor semantic difference but necessary to enable Spring lifecycle management.

  • Fixed FactoryBean unwrapping in blDefaultTargetModeMap. The prodEntityManager parameter was typed as SharedEntityManagerBean (the FactoryBean itself) instead of EntityManager (the product it creates). This caused Spring to inject the raw FactoryBean object rather than the EntityManager proxy it produces. Consumers like PersistenceServiceImpl.getEntityManager() (line 184) cast map values to (EntityManager), which would throw ClassCastException at runtime. Similarly, blTransactionManager was typed as LifecycleAwareJpaTransactionManager — changed to PlatformTransactionManager to match what the XML value-ref resolves and what consumers expect. The XML value-ref="prodEntityManager" transparently resolves through the FactoryBean; the Java config must use the product type to get the same behavior.

Additional context — items for reviewer attention:

  1. Dual registration risk: Both XML and Java configs now coexist. Consumers must take care to load one or the other — not both — in the same ApplicationContext, or beans will be double-registered. There is no guard mechanism added in this PR; that coordination is deferred to the per-module migration PRs.

  2. FactoryBean return types: CommonPersistenceConfiguration.blCacheManager() returns a MergeJCacheManagerFactoryBean and CommonMBeansConfiguration.blJmxNamingBean() returns a JndiObjectFactoryBean. Downstream beans inject the resolved values as Object then convert via String.valueOf(). This relies on Spring's automatic FactoryBean.getObject() unwrapping. Verify this matches the XML behavior where <bean class="JndiObjectFactoryBean"> transparently exposes the looked-up JNDI string.

  3. Merge-bean collections: Several beans (blMergedClassTransformers, blMergedCacheConfigLocations, blMergedPersistenceXmlLocations, blTargetModeMaps, etc.) use ListFactoryBean/MapFactoryBean. Broadleaf's merge infrastructure patches these at runtime. Confirm the Java-config-produced collections integrate correctly with MergeXmlWebApplicationContext.

  4. Bean name fidelity: Entity beans in CommonEntityConfiguration use FQCN names (e.g., org.broadleafcommerce.common.email.domain.EmailTracking). These must match XML exactly since EntityConfiguration resolves by name.

  5. blPropertyPlaceholderConfigurer is defined as a static @Bean in CommonConfiguration, CommonAdminServletConfiguration, and CommonSiteServletConfiguration. This mirrors the XML where each context level (root vs. servlet) has its own configurer. No conflict expected as these load in separate Spring contexts, but worth verifying.

  6. blTxAdvice not translated: The XML persistence config defines <tx:advice id="blTxAdvice"> with method-level propagation rules (e.g. REQUIRES_NEW for findNextId). This is covered by @EnableTransactionManagement for @Transactional-annotated code, but the explicit per-method-name advice is not replicated. Verify downstream modules don't reference blTxAdvice by name or rely on its pointcut binding.

  7. No runtime/integration testing was performed — only compilation. Bean wiring correctness is not validated until an integration test or application startup exercises these configs.

Human review checklist:

  • Spot-check 2–3 entity bean FQCN names in CommonEntityConfiguration against their XML source
  • Confirm blStatisticsService setter signatures match XML property injection (appName as String, adapter as StatisticsServiceLogAdapter)
  • Verify no downstream module references blTxAdvice by name
  • Confirm ListFactoryBean/MapFactoryBean beans are compatible with Broadleaf's merge infrastructure
  • Verify the javaconfig package exclude filter in CommonConfiguration is sufficient — check that no downstream module's @ComponentScan could still pick up context-specific configs unintentionally
  • Confirm CommonEntityConfiguration is NOT loaded into the main context (it should only be consumed via EntityConfiguration's separate GenericXmlApplicationContext or explicit @Import)
  • Verify PersistenceUnitPostProcessor beans receive their injected dependencies correctly at runtime (especially JPAPropertiesPersistenceUnitPostProcessor which has 18 @Value fields and a @PostConstruct method)
  • Verify blDefaultTargetModeMap entries contain actual EntityManager proxy and PlatformTransactionManager at runtime — not the SharedEntityManagerBean FactoryBean wrapper
  • Confirm blTransactionManager typed as PlatformTransactionManager in the map still resolves to the LifecycleAwareJpaTransactionManager instance (it should, since that class implements PlatformTransactionManager)

Link to Devin session: https://app.devin.ai/sessions/a94b82fd29a44c7892a89ecf8907a729
Requested by: @Colhodm


Open with Devin

…classes

Convert all 10 XML Spring application context files in the common module
to equivalent Java @configuration classes in the
org.broadleafcommerce.common.config.javaconfig package:

- CommonPersistenceConfiguration: EntityManagerFactory, TransactionManager, cache, PU management
- CommonConfiguration: core beans, component scanning, email, resources, class transformers
- CommonEntityConfiguration: prototype-scoped entity beans for EntityConfiguration
- CommonMBeansConfiguration: JMX/MBean beans (disabled via 'mbeansdisabled' profile)
- CommonServletConfiguration: servlet context component scanning
- CommonWrapperConfiguration: REST API wrapper beans
- CommonAdminConfiguration: admin root context (imports CommonConfiguration)
- CommonAdminServletConfiguration: admin servlet context
- CommonSiteConfiguration: site context with resource handlers
- CommonSiteServletConfiguration: site servlet context

All bean names are preserved exactly for backward compatibility.
XML files are kept in place until all modules are migrated.
The common module compiles successfully with mvn compile -DskipTests -pl common.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

Addresses Devin Review finding - the XML defines blStatisticsService with
appName and adapter properties that were accidentally omitted from the
Java config. Without this bean, the adapter field remains null and
logging operations become no-ops.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…t isolation

Add exclude filter for org.broadleafcommerce.common.config.javaconfig
to CommonConfiguration's @componentscan. Without this, all @configuration
classes in the javaconfig package (servlet, site, admin, entity configs)
would be picked up by the root context scan, breaking Spring context
isolation. The explicitly @imported configs (CommonPersistenceConfiguration,
CommonWrapperConfiguration, CommonMBeansConfiguration) are unaffected
since @import works independently of component scanning.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…hods

JPAPropertiesPersistenceUnitPostProcessor, ORMConfigPersistenceUnitPostProcessor,
and JCachePersistenceUnitPostProcessor were instantiated via new, bypassing
Spring dependency injection. All three classes rely heavily on @value, @resource,
@Autowired, and @PostConstruct. Define each as a separate @bean so Spring
manages their lifecycle, then inject them into the list bean.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

…argetModeMap

SharedEntityManagerBean is a FactoryBean, so typing the parameter as
SharedEntityManagerBean causes Spring to inject the FactoryBean itself
rather than its product (the EntityManager proxy). The XML value-ref
resolves through the FactoryBean automatically. Changed parameter types
to EntityManager and PlatformTransactionManager to match what consumers
like PersistenceServiceImpl expect when casting map values.

Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
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