Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ public interface OptionService {

List<OptionSet> getAllOptionSets();

/**
* Initializes the {@link OptionSet#getOptions()} collections of the supplied managed option sets.
* Collections missing from the second-level cache are loaded together with one bulk query;
* collections present in the cache are initialized from it without SQL.
*
* @param optionSets managed option sets whose options collections should be initialized
*/
void preloadOptions(@Nonnull Collection<OptionSet> optionSets);

List<Option> findOptionsByNamePattern(
@Nonnull String optionSet, @CheckForNull String infix, @CheckForNull Integer maxResults);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
@Setter
@Entity
@Table(name = "optionset")
@Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE)
public class OptionSet extends BaseMetadataObject implements IdentifiableObject, VersionedObject {

@Id
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (c) 2004-2026, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* 3. Neither the name of the copyright holder nor the names of its contributors
* may be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.option;

import java.util.Collection;
import javax.annotation.Nonnull;
import org.hisp.dhis.common.IdentifiableObjectStore;

/**
* Persistence operations specific to {@link OptionSet}.
*
* @author Morten Svanæs <msvanaes@dhis2.org>
*/
public interface OptionSetStore extends IdentifiableObjectStore<OptionSet> {

/**
* Initializes the {@link OptionSet#getOptions()} collections of the supplied managed option sets.
* Collections missing from the second-level cache are loaded together with one bulk query;
* collections present in the cache are initialized from it without SQL.
*
* @param optionSets managed option sets whose options collections should be initialized
*/
void preloadOptions(@Nonnull Collection<OptionSet> optionSets);
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.hisp.dhis.indicator.IndicatorGroupSet;
import org.hisp.dhis.indicator.IndicatorType;
import org.hisp.dhis.legend.LegendSet;
import org.hisp.dhis.option.OptionSet;
import org.hisp.dhis.predictor.PredictorGroup;
import org.hisp.dhis.program.ProgramExpression;
import org.hisp.dhis.program.ProgramIndicatorGroup;
Expand Down Expand Up @@ -124,12 +123,6 @@ public HibernateIdentifiableObjectStore<Constant> constantStore() {
entityManager, jdbcTemplate, publisher, Constant.class, aclService, true);
}

@Bean("org.hisp.dhis.option.OptionSetStore")
public HibernateIdentifiableObjectStore<OptionSet> optionSetStore() {
return new HibernateIdentifiableObjectStore<>(
entityManager, jdbcTemplate, publisher, OptionSet.class, aclService, true);
}

@Bean("org.hisp.dhis.legend.LegendSetStore")
public HibernateIdentifiableObjectStore<LegendSet> legendSetStore() {
return new HibernateIdentifiableObjectStore<>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,10 @@
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import org.hisp.dhis.common.IdentifiableObjectStore;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.common.ValueType;
import org.hisp.dhis.feedback.ConflictException;
import org.hisp.dhis.feedback.ErrorCode;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -50,8 +48,7 @@
@RequiredArgsConstructor
@Service("org.hisp.dhis.option.OptionService")
public class DefaultOptionService implements OptionService {
@Qualifier("org.hisp.dhis.option.OptionSetStore")
private final IdentifiableObjectStore<OptionSet> optionSetStore;
private final OptionSetStore optionSetStore;

private final OptionStore optionStore;

Expand Down Expand Up @@ -132,6 +129,12 @@ public List<OptionSet> getAllOptionSets() {
return optionSetStore.getAll();
}

@Override
@Transactional(readOnly = true)
public void preloadOptions(@Nonnull Collection<OptionSet> optionSets) {
optionSetStore.preloadOptions(optionSets);
}

// -------------------------------------------------------------------------
// Option
// -------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* Copyright (c) 2004-2026, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* 1. Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
*
* 3. Neither the name of the copyright holder nor the names of its contributors
* may be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.option.hibernate;

import jakarta.persistence.EntityManager;
import java.util.Collection;
import java.util.List;
import javax.annotation.Nonnull;
import org.hibernate.Cache;
import org.hibernate.Hibernate;
import org.hisp.dhis.common.hibernate.HibernateIdentifiableObjectStore;
import org.hisp.dhis.option.OptionSet;
import org.hisp.dhis.option.OptionSetStore;
import org.hisp.dhis.security.acl.AclService;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.stereotype.Repository;

/**
* @author Morten Svanæs <msvanaes@dhis2.org>
*/
@Repository("org.hisp.dhis.option.OptionSetStore")
public class HibernateOptionSetStore extends HibernateIdentifiableObjectStore<OptionSet>
implements OptionSetStore {

private static final String OPTIONS_COLLECTION_ROLE = OptionSet.class.getName() + ".options";

public HibernateOptionSetStore(
EntityManager entityManager,
JdbcTemplate jdbcTemplate,
ApplicationEventPublisher publisher,
AclService aclService) {
super(entityManager, jdbcTemplate, publisher, OptionSet.class, aclService, true);
}

@Override
public void preloadOptions(@Nonnull Collection<OptionSet> optionSets) {
if (optionSets.isEmpty()) {
return;
}

Cache cache = getSession().getSessionFactory().getCache();
List<OptionSet> missing =
optionSets.stream()
.filter(
optionSet -> !cache.containsCollection(OPTIONS_COLLECTION_ROLE, optionSet.getId()))
.toList();

if (!missing.isEmpty()) {
// One bulk fetch for every collection absent from the second-level cache. The query must not
// be cacheable: the Hibernate query cache stores entity identifiers only and cannot
// rematerialize join-fetched collection state, so caching it would recreate the per-owner
// load storm this method exists to remove.
getQuery(
"""
select distinct optionSet from OptionSet optionSet
left join fetch optionSet.options
where optionSet in :optionSets
""",
OptionSet.class)
.setParameter("optionSets", missing)
.setCacheable(false)
.list();
}

// Initializes the remaining collections from the second-level cache (collection ids plus the
// Option entity region); collections loaded by the bulk fetch above are already initialized.
optionSets.forEach(optionSet -> Hibernate.initialize(optionSet.getOptions()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@
<expiry><ttl unit="seconds">3600</ttl></expiry>
<resources><heap unit="entries">200000</heap></resources>
</cache>
<!-- Entity-level cache restored after the OptionSet.hbm.xml -> JPA migration dropped it: the
metadata query cache stores OptionSet ids only, so without this region every query-cache
hit re-selects each OptionSet row by id (one select per option set in the result). -->
<cache alias="org.hisp.dhis.option.OptionSet">
<expiry><ttl unit="seconds">3600</ttl></expiry>
<resources><heap unit="entries">20000</heap></resources>
</cache>
<cache alias="org.hisp.dhis.trackedentity.TrackedEntityAttribute">
<expiry><ttl unit="seconds">3600</ttl></expiry>
<resources><heap unit="entries">20000</heap></resources>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.hisp.dhis.attribute.Attribute;
import org.hisp.dhis.cache.HibernateEhcacheConfigFileTest.DhisConfig;
import org.hisp.dhis.external.conf.ConfigurationKey;
import org.hisp.dhis.option.OptionSet;
import org.hisp.dhis.test.config.PostgresTestConfigOverride;
import org.hisp.dhis.test.integration.PostgresIntegrationTestBase;
import org.hisp.dhis.user.User;
Expand Down Expand Up @@ -94,6 +95,13 @@ public PostgresTestConfigOverride postgresTestConfigOverride() {
*/
private static final long EHCACHE_XML_USER_HEAP_ENTRIES = 100_000;

/**
* Heap bound declared explicitly for the {@code org.hisp.dhis.option.OptionSet} entity region in
* ehcache.xml. The entity-level cache was lost when OptionSet.hbm.xml was migrated to JPA
* annotations (commit fa6a8558f8c0); without it a query-cache hit reloads every OptionSet row.
*/
private static final long EHCACHE_XML_OPTION_SET_HEAP_ENTRIES = 20_000;

@Autowired private EntityManagerFactory entityManagerFactory;

@Test
Expand Down Expand Up @@ -124,6 +132,10 @@ void regionsCarryTheEhcacheXmlHeapBounds() {
EHCACHE_XML_USER_HEAP_ENTRIES,
heapEntries(cacheManager, User.class.getName()),
"explicitly declared entity regions must carry their own ehcache.xml heap bound");
assertEquals(
EHCACHE_XML_OPTION_SET_HEAP_ENTRIES,
heapEntries(cacheManager, OptionSet.class.getName()),
"OptionSet entity state must use its explicit store-by-reference region");
assertEquals(
EHCACHE_XML_TIMESTAMPS_HEAP_ENTRIES,
heapEntries(cacheManager, "default-update-timestamps-region"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@
@ContextConfiguration(classes = {DhisConfig.class})
class HibernateQueryCacheTest extends PostgresIntegrationTestBase {

/**
* Dedicated query-cache region for this test. Must not equal the {@link OptionSet} entity region
* name ({@code org.hisp.dhis.option.OptionSet}), otherwise query results and entity state would
* share one JCache alias and the region statistics would mix both.
*/
private static final String OPTION_SET_QUERY_REGION = OptionSet.class.getName() + ".query-test";

static class DhisConfig {
@Bean
public PostgresTestConfigOverride postgresTestConfigOverride() {
Expand Down Expand Up @@ -129,11 +136,37 @@ void testHouseKeepingJobWithCache() {
10,
sessionFactory
.getStatistics()
.getCacheRegionStatistics(OptionSet.class.getName())
.getCacheRegionStatistics(OPTION_SET_QUERY_REGION)
.getHitCount());
assertTrue(sessionFactory.getStatistics().getQueryCacheHitCount() > queryCacheHitCountBefore);
}

@Test
@DisplayName(
"OptionSet query-cache hits resolve entity state from L2 across persistence contexts")
void queryCacheHitUsesOptionSetEntityCacheAcrossPersistenceContexts() {
setUpData();
sessionFactory.getCache().evictEntityData(OptionSet.class);
sessionFactory.getCache().evictQueryRegion(OPTION_SET_QUERY_REGION);
sessionFactory.getStatistics().clear();
entityManager.clear();

createSelectQuery(1);
entityManager.clear();
createSelectQuery(1);

Statistics statistics = sessionFactory.getStatistics();
assertEquals(1, statistics.getQueryCacheMissCount());
assertEquals(1, statistics.getQueryCacheHitCount());
assertEquals(
1,
statistics.getEntityLoadCount(),
"the query-cache hit must not reload OptionSet from PostgreSQL");
assertTrue(
statistics.getSecondLevelCacheHitCount() >= 1,
"the second persistence context must resolve OptionSet from entity L2");
}

private void createSelectQuery(int numberOfQueries) {
for (int i = 0; i < numberOfQueries; i++) {
entityManager.getTransaction().begin();
Expand All @@ -147,7 +180,7 @@ private TypedQuery<OptionSet> createQuery(EntityManager entityManager) {
return entityManager
.createQuery("from OptionSet where code = :code", OptionSet.class)
.setParameter("code", "OptionSetCodeA")
.setHint(QueryHints.HINT_CACHE_REGION, "org.hisp.dhis.option.OptionSet")
.setHint(QueryHints.HINT_CACHE_REGION, OPTION_SET_QUERY_REGION)
.setHint(QueryHints.HINT_CACHEABLE, true);
}
}
Loading
Loading