Fix rollouts status cache eviction and NPE in UI (#530)

* Fix nullpointer in UI and rollout cache invalidation prob.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Sonar issue and util usage.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>

* Evict cache on tenant delete.

Signed-off-by: kaizimmerm <kai.zimmermann@bosch-si.com>
This commit is contained in:
Kai Zimmermann
2017-06-01 20:08:47 +02:00
committed by GitHub
parent 67a4677ef6
commit 672d4270d7
17 changed files with 86 additions and 60 deletions

View File

@@ -31,7 +31,11 @@
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
</dependency>
<dependency>
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context-support</artifactId>

View File

@@ -24,10 +24,10 @@ import org.eclipse.hawkbit.repository.model.RolloutGroup;
import org.eclipse.hawkbit.repository.model.TotalTargetCountActionStatus;
import org.eclipse.hawkbit.tenancy.TenantAware;
import org.springframework.cache.Cache;
import org.springframework.cache.guava.GuavaCacheManager;
import org.springframework.cache.caffeine.CaffeineCacheManager;
import org.springframework.context.event.EventListener;
import com.google.common.cache.CacheBuilder;
import com.github.benmanes.caffeine.cache.Caffeine;
/**
* Internal cache for Rollout status.
@@ -49,11 +49,11 @@ public class RolloutStatusCache {
public RolloutStatusCache(final TenantAware tenantAware, final long size) {
this.tenantAware = tenantAware;
final CacheBuilder<Object, Object> cacheBuilder = CacheBuilder.newBuilder().maximumSize(size);
final GuavaCacheManager guavaCacheManager = new GuavaCacheManager();
guavaCacheManager.setCacheBuilder(cacheBuilder);
final Caffeine<Object, Object> cacheBuilder = Caffeine.newBuilder().maximumSize(size);
final CaffeineCacheManager caffeineCacheManager = new CaffeineCacheManager();
caffeineCacheManager.setCaffeine(cacheBuilder);
this.cacheManager = new TenantAwareCacheManager(guavaCacheManager, tenantAware);
this.cacheManager = new TenantAwareCacheManager(caffeineCacheManager, tenantAware);
}
/**
@@ -174,8 +174,8 @@ public class RolloutStatusCache {
}
private Map<Long, List<TotalTargetCountActionStatus>> retrieveFromCache(final List<Long> ids, final Cache cache) {
return ids.stream().map(rolloutId -> cache.get(rolloutId, CachedTotalTargetCountActionStatus.class))
.filter(Objects::nonNull).collect(Collectors.toMap(CachedTotalTargetCountActionStatus::getRolloutId,
return ids.stream().map(id -> cache.get(id, CachedTotalTargetCountActionStatus.class)).filter(Objects::nonNull)
.collect(Collectors.toMap(CachedTotalTargetCountActionStatus::getId,
CachedTotalTargetCountActionStatus::getStatus));
}
@@ -189,9 +189,8 @@ public class RolloutStatusCache {
return cacheItem.getStatus();
}
private void putIntoCache(final Long rolloutId, final List<TotalTargetCountActionStatus> status,
final Cache cache) {
cache.put(rolloutId, new CachedTotalTargetCountActionStatus(rolloutId, status));
private void putIntoCache(final Long id, final List<TotalTargetCountActionStatus> status, final Cache cache) {
cache.put(id, new CachedTotalTargetCountActionStatus(id, status));
}
private void putIntoCache(final Map<Long, List<TotalTargetCountActionStatus>> put, final Cache cache) {
@@ -201,19 +200,15 @@ public class RolloutStatusCache {
@EventListener(classes = AbstractActionEvent.class)
void invalidateCachedTotalTargetCountActionStatus(final AbstractActionEvent event) {
if (event.getRolloutId() == null) {
return;
if (event.getRolloutId() != null) {
final Cache cache = tenantAware.runAsTenant(event.getTenant(), () -> cacheManager.getCache(CACHE_RO_NAME));
cache.evict(event.getRolloutId());
}
Cache cache = tenantAware.runAsTenant(event.getTenant(), () -> cacheManager.getCache(CACHE_RO_NAME));
cache.evict(event.getRolloutId());
if (event.getRolloutGroupId() == null) {
return;
if (event.getRolloutGroupId() != null) {
final Cache cache = tenantAware.runAsTenant(event.getTenant(), () -> cacheManager.getCache(CACHE_GR_NAME));
cache.evict(event.getRolloutGroupId());
}
cache = tenantAware.runAsTenant(event.getTenant(), () -> cacheManager.getCache(CACHE_GR_NAME));
cache.evict(event.getRolloutGroupId());
}
@EventListener(classes = RolloutDeletedEvent.class)
@@ -228,18 +223,28 @@ public class RolloutStatusCache {
cache.evict(event.getEntityId());
}
/**
* Evicts all caches for a given tenant. All caches under a certain tenant
* gets evicted.
*
* @param tenant
* the tenant to evict caches
*/
public void evictCaches(final String tenant) {
cacheManager.evictCaches(tenant);
}
private static final class CachedTotalTargetCountActionStatus {
private final long rolloutId;
private final long id;
private final List<TotalTargetCountActionStatus> status;
private CachedTotalTargetCountActionStatus(final long rolloutId,
final List<TotalTargetCountActionStatus> status) {
this.rolloutId = rolloutId;
private CachedTotalTargetCountActionStatus(final long id, final List<TotalTargetCountActionStatus> status) {
this.id = id;
this.status = status;
}
public long getRolloutId() {
return rolloutId;
public long getId() {
return id;
}
public List<TotalTargetCountActionStatus> getStatus() {

View File

@@ -352,7 +352,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
final DistributionSetFilter distributionSetFilter) {
final List<Specification<JpaDistributionSet>> specList = buildDistributionSetSpecifications(
distributionSetFilter);
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return null;
}
return distributionSetRepository.findOne(SpecificationsBuilder.combineWithAnd(specList));
@@ -667,7 +667,7 @@ public class JpaDistributionSetManagement implements DistributionSetManagement {
private Page<JpaDistributionSet> findByCriteriaAPI(final Pageable pageable,
final List<Specification<JpaDistributionSet>> specList) {
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return distributionSetRepository.findAll(pageable);
}

View File

@@ -53,6 +53,7 @@ import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
import org.springframework.validation.annotation.Validated;
/**
@@ -166,7 +167,7 @@ public class JpaRolloutGroupManagement implements RolloutGroupManagement {
List<TotalTargetCountActionStatus> rolloutStatusCountItems = rolloutStatusCache
.getRolloutGroupStatus(rolloutGroupId);
if (rolloutStatusCountItems.isEmpty()) {
if (CollectionUtils.isEmpty(rolloutStatusCountItems)) {
rolloutStatusCountItems = actionRepository.getStatusCountByRolloutGroupId(rolloutGroupId);
rolloutStatusCache.putRolloutGroupStatus(rolloutGroupId, rolloutStatusCountItems);
}

View File

@@ -93,6 +93,7 @@ import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.TransactionException;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.util.concurrent.ListenableFuture;
import org.springframework.validation.annotation.Validated;
@@ -177,7 +178,7 @@ public class JpaRolloutManagement extends AbstractRolloutManagement {
*/
private Page<JpaRollout> findByCriteriaAPI(final Pageable pageable,
final List<Specification<JpaRollout>> specList) {
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return rolloutRepository.findAll(pageable);
}
@@ -992,7 +993,7 @@ public class JpaRolloutManagement extends AbstractRolloutManagement {
List<TotalTargetCountActionStatus> rolloutStatusCountItems = rolloutStatusCache.getRolloutStatus(rolloutId);
if (rolloutStatusCountItems.isEmpty()) {
if (CollectionUtils.isEmpty(rolloutStatusCountItems)) {
rolloutStatusCountItems = actionRepository.getStatusCountByRolloutId(rolloutId);
rolloutStatusCache.putRolloutStatus(rolloutId, rolloutStatusCountItems);
}

View File

@@ -17,6 +17,7 @@ import java.util.stream.Collectors;
import javax.persistence.EntityManager;
import org.eclipse.hawkbit.cache.TenancyCacheManager;
import org.eclipse.hawkbit.repository.RolloutStatusCache;
import org.eclipse.hawkbit.repository.SystemManagement;
import org.eclipse.hawkbit.repository.TenantStatsManagement;
import org.eclipse.hawkbit.repository.jpa.configuration.Constants;
@@ -112,6 +113,9 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst
@Autowired
private PlatformTransactionManager txManager;
@Autowired
private RolloutStatusCache rolloutStatusCache;
@Override
public SystemUsageReport getSystemUsageStatistics() {
@@ -219,6 +223,7 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst
public void deleteTenant(final String t) {
final String tenant = t.toUpperCase();
cacheManager.evictCaches(tenant);
rolloutStatusCache.evictCaches(tenant);
tenantAware.runAsTenant(tenant, () -> {
entityManager.setProperty(PersistenceUnitProperties.MULTITENANT_PROPERTY_DEFAULT, tenant);
tenantMetaDataRepository.deleteByTenantIgnoreCase(tenant);

View File

@@ -41,6 +41,7 @@ import org.springframework.data.jpa.domain.Specifications;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Retryable;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.validation.annotation.Validated;
@@ -157,7 +158,7 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme
private Page<JpaTargetFilterQuery> findTargetFilterQueryByCriteriaAPI(final Pageable pageable,
final List<Specification<JpaTargetFilterQuery>> specList) {
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return targetFilterQueryRepository.findAll(pageable);
}

View File

@@ -67,6 +67,7 @@ import org.springframework.data.jpa.domain.Specification;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Retryable;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.validation.annotation.Validated;
@@ -317,7 +318,7 @@ public class JpaTargetManagement implements TargetManagement {
}
private Slice<Target> findByCriteriaAPI(final Pageable pageable, final List<Specification<JpaTarget>> specList) {
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return convertPage(criteriaNoCountDao.findAll(pageable, JpaTarget.class), pageable);
}
return convertPage(
@@ -326,7 +327,7 @@ public class JpaTargetManagement implements TargetManagement {
}
private Long countByCriteriaAPI(final List<Specification<JpaTarget>> specList) {
if (specList == null || specList.isEmpty()) {
if (CollectionUtils.isEmpty(specList)) {
return targetRepository.count();
}

View File

@@ -43,6 +43,7 @@ import org.slf4j.LoggerFactory;
import org.springframework.beans.SimpleTypeConverter;
import org.springframework.beans.TypeMismatchException;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.util.CollectionUtils;
import cz.jirutka.rsql.parser.RSQLParser;
import cz.jirutka.rsql.parser.RSQLParserException;
@@ -169,7 +170,7 @@ public final class RSQLUtility {
virtualPropertyReplacer);
final List<Predicate> accept = rootNode.<List<Predicate>, String> accept(jpqQueryRSQLVisitor);
if (accept != null && !accept.isEmpty()) {
if (!CollectionUtils.isEmpty(accept)) {
return cb.and(accept.toArray(new Predicate[accept.size()]));
}
return cb.conjunction();
@@ -652,7 +653,7 @@ public final class RSQLUtility {
final List<Predicate> childs = new ArrayList<>();
for (final Node node2 : children) {
final List<Predicate> accept = node2.accept(this);
if (accept != null && !accept.isEmpty()) {
if (!CollectionUtils.isEmpty(accept)) {
childs.addAll(accept);
} else {
LOGGER.debug("visit logical node children but could not parse it, ignoring {}", node2);

View File

@@ -34,6 +34,7 @@ import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.PageRequest;
import org.springframework.orm.jpa.JpaSystemException;
import org.springframework.util.CollectionUtils;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;
@@ -149,7 +150,7 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle {
final int currentTokenEndColumn, final int[] is) {
for (final int i : is) {
final Collection<String> tokenImage = TokenDescription.getTokenImage(i);
if (tokenImage != null && !tokenImage.isEmpty()) {
if (!CollectionUtils.isEmpty(tokenImage)) {
tokenImage.forEach(image -> listTokens.add(new SuggestToken(currentTokenEndColumn + 1,
nextTokenBeginColumn + image.length(), null, image)));
}
@@ -164,8 +165,8 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle {
.of(FieldNameDescription.toTopSuggestToken(nextTokenBeginColumn - currentTokenImageName.length(),
nextTokenBeginColumn + currentTokenImageName.length(), currentTokenImageName));
} else if (shouldSuggestDotToken(currentTokenImageName, containsDot)) {
return Optional.of(
Arrays.asList(new SuggestToken(currentTokenEndColumn, nextTokenBeginColumn + 1, null, ".")));
return Optional
.of(Arrays.asList(new SuggestToken(currentTokenEndColumn, nextTokenBeginColumn + 1, null, ".")));
} else if (shouldSuggestSubTokenFieldNames(currentTokenImageName, containsDot)) {
return handleSubtokenSuggestion(currentTokenImageName, nextTokenBeginColumn);
}
@@ -222,7 +223,7 @@ public class RsqlParserValidationOracle implements RsqlValidationOracle {
builder = builder.substring(0, builder.lastIndexOf("Was expecting"));
}
if (expectedTokens != null && !expectedTokens.isEmpty()) {
if (!CollectionUtils.isEmpty(expectedTokens)) {
final StringBuilder tokens = new StringBuilder();
expectedTokens.stream().forEach(value -> tokens.append(value.getSuggestion() + ","));
builder = builder.concat("Was expecting :" + tokens.toString().substring(0, tokens.length() - 1));