Skip to content

Commit

Permalink
Add cache to contentversioninfo (#26968)
Browse files Browse the repository at this point in the history 
* chore: add cache to contentletVersionInfo ref: #26931

* test: add test ref: #26931

* feedback

* feedback

---------

Co-authored-by: Will Ezell <will@dotcms.com>
  • Loading branch information
@erickgonzalez @wezell
erickgonzalez and wezell committed Dec 27, 2023
1 parent 7b858a3 commit 48caf15
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 57 deletions.
82 changes: 79 additions & 3 deletions dotCMS/src/integration-test/java/com/dotmarketing/business/IdentifierCacheImplTest.java
View file
@@ -1,19 +1,22 @@
package com.dotmarketing.business;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;

import com.dotcms.datagen.LanguageDataGen;
import com.dotcms.datagen.VariantDataGen;
import com.dotcms.util.IntegrationTestInitService;
import com.dotcms.variant.VariantAPI;
import com.dotcms.variant.model.Variant;
import com.dotmarketing.beans.Identifier;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
import com.dotmarketing.portlets.languagesmanager.model.Language;
import com.dotmarketing.util.UUIDGenerator;
import org.junit.BeforeClass;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;

import static org.junit.Assert.*;

public class IdentifierCacheImplTest {

@BeforeClass
Expand Down Expand Up @@ -73,4 +76,77 @@ public void addIntoCacheWithVariant(){
assertNotNull(contentletVersionInfoFromCache);
assertEquals(contentletVersionInfo, contentletVersionInfoFromCache);
}

/**
* Method to test: {@link IdentifierCacheImpl#putContentVersionInfos(String, List)} and {@link IdentifierCacheImpl#getContentVersionInfos(String)}
* When: Add a list of {@link ContentletVersionInfo} is added to cache
* Should: get it with {@link IdentifierCacheImpl#getContentVersionInfos(String)}
*/
@Test
public void putContentVersionInfos_and_getContentVersionInfos_successfully(){
final String id = UUIDGenerator.generateUuid();
final Language language1 = new LanguageDataGen().nextPersisted();
final Language language2 = new LanguageDataGen().nextPersisted();
final List<ContentletVersionInfo> listContentletVersionInfo = new ArrayList<>();

ContentletVersionInfo contentletVersionInfo = new ContentletVersionInfo();
contentletVersionInfo.setIdentifier(id);
contentletVersionInfo.setLang(language1.getId());
contentletVersionInfo.setVariant(VariantAPI.DEFAULT_VARIANT.name());
listContentletVersionInfo.add(contentletVersionInfo);

contentletVersionInfo = new ContentletVersionInfo();
contentletVersionInfo.setIdentifier(id);
contentletVersionInfo.setLang(language2.getId());
contentletVersionInfo.setVariant(VariantAPI.DEFAULT_VARIANT.name());
listContentletVersionInfo.add(contentletVersionInfo);

CacheLocator.getIdentifierCache().putContentVersionInfos(id,listContentletVersionInfo);

final List<ContentletVersionInfo> listContentletVersionInfoFromCache = CacheLocator.getIdentifierCache()
.getContentVersionInfos(id);

assertNotNull(listContentletVersionInfoFromCache);
assertEquals(listContentletVersionInfo.size(), listContentletVersionInfoFromCache.size());
}


/**
* Method to test: {@link IdentifierCacheImpl#removeFromCacheByIdentifier(Identifier)}
* When: Add a list of {@link ContentletVersionInfo} is added to cache and removed by the identifier
* Should: all versions should be removed.
*/
@Test
public void removeFromCacheByIdentifier_successfully(){
final String id = UUIDGenerator.generateUuid();
final Language language1 = new LanguageDataGen().nextPersisted();
final Language language2 = new LanguageDataGen().nextPersisted();
final List<ContentletVersionInfo> listContentletVersionInfo = new ArrayList<>();

ContentletVersionInfo contentletVersionInfo = new ContentletVersionInfo();
contentletVersionInfo.setIdentifier(id);
contentletVersionInfo.setLang(language1.getId());
contentletVersionInfo.setVariant(VariantAPI.DEFAULT_VARIANT.name());
listContentletVersionInfo.add(contentletVersionInfo);

contentletVersionInfo = new ContentletVersionInfo();
contentletVersionInfo.setIdentifier(id);
contentletVersionInfo.setLang(language2.getId());
contentletVersionInfo.setVariant(VariantAPI.DEFAULT_VARIANT.name());
listContentletVersionInfo.add(contentletVersionInfo);

CacheLocator.getIdentifierCache().putContentVersionInfos(id,listContentletVersionInfo);

List<ContentletVersionInfo> listContentletVersionInfoFromCache = CacheLocator.getIdentifierCache()
.getContentVersionInfos(id);

assertNotNull(listContentletVersionInfoFromCache);
assertEquals(listContentletVersionInfo.size(), listContentletVersionInfoFromCache.size());

CacheLocator.getIdentifierCache().removeFromCacheByIdentifier(id);

listContentletVersionInfoFromCache = CacheLocator.getIdentifierCache()
.getContentVersionInfos(id);
assertNull(listContentletVersionInfoFromCache);
}
}
7 changes: 7 additions & 0 deletions dotCMS/src/main/java/com/dotmarketing/business/IdentifierCache.java
View file
Expand Up @@ -5,6 +5,8 @@
import com.dotmarketing.beans.VersionInfo;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;

import java.util.List;

public abstract class IdentifierCache implements Cachable {

abstract protected void addIdentifierToCache(Identifier id);
Expand Down Expand Up @@ -70,4 +72,9 @@ public String get404Group() {
return "Identifier404Cache";
}

public abstract void putContentVersionInfos(String identifier,
List<ContentletVersionInfo> cvis);

public abstract List<ContentletVersionInfo> getContentVersionInfos(String identifier);

}
35 changes: 32 additions & 3 deletions dotCMS/src/main/java/com/dotmarketing/business/IdentifierCacheImpl.java
View file
Expand Up @@ -11,6 +11,7 @@
import com.dotmarketing.beans.Host;
import com.dotmarketing.beans.Identifier;
import com.dotmarketing.beans.VersionInfo;
import com.dotmarketing.db.DbConnectionFactory;
import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo;
import com.dotmarketing.util.InodeUtils;
import com.dotmarketing.util.Logger;
Expand Down Expand Up @@ -159,15 +160,16 @@ protected void removeFromCacheByIdentifier(final Identifier id) {
if(id==null) {
return;
}
if(InodeUtils.isSet(id.getId())) {

if(UtilMethods.isSet(id::getId)) {
final String key = getPrimaryGroup() + id.getId();
cache.remove(key, getPrimaryGroup());
cache.remove(key, get404Group());
cache.remove(allInfosKey(id.getId()), getVersionInfoGroup());
}

final String uri = id.getURI();
if(UtilMethods.isSet(id.getHostId()) && UtilMethods.isSet(uri)) {
if(UtilMethods.isSet(id::getHostId) && UtilMethods.isSet(uri)) {
final String key = getPrimaryGroup() + id.getHostId() + "-" + uri;
cache.remove(key, getPrimaryGroup());
cache.remove(key, get404Group());
Expand Down Expand Up @@ -226,6 +228,7 @@ public void removeFromCacheByVersionable(Versionable versionable) {

public void removeFromCacheByInode(String inode) {
cache.remove(getVersionGroup() + inode, getVersionGroup());
cache.remove(allInfosKey(inode), getVersionInfoGroup());
}


Expand Down Expand Up @@ -295,16 +298,42 @@ public VersionInfo getVersionInfo(String identifier) {
public void removeContentletVersionInfoToCache(String identifier, long lang) {
String key = getKey(identifier, lang);
cache.remove(getVersionInfoGroup()+key, getVersionInfoGroup());
cache.remove( allInfosKey(identifier) ,getVersionInfoGroup());
}

@Override
public void removeContentletVersionInfoToCache(String identifier, long lang, final String variantId) {
String key = getKey(identifier, lang, variantId);
cache.remove(getVersionInfoGroup() + key , getVersionInfoGroup());
cache.remove( allInfosKey(identifier) ,getVersionInfoGroup());
}

@Override
protected void removeVersionInfoFromCache(String identifier) {
cache.remove(getVersionInfoGroup()+identifier, getVersionInfoGroup());
cache.remove( allInfosKey(identifier) ,getVersionInfoGroup());
}

private String allInfosKey(String... vals){
return "all-"+ String.join("-", vals);
}


@Override
public List<ContentletVersionInfo> getContentVersionInfos(final String identifier){
// we don't read from cache in transactions
if(DbConnectionFactory.inTransaction()){
return null;
}
return (List<ContentletVersionInfo>) cache.getNoThrow(allInfosKey(identifier) , getVersionInfoGroup());
}

@Override
public void putContentVersionInfos(final String identifier, final List<ContentletVersionInfo> versionInfos){
// we don't write cache in transactions
if(DbConnectionFactory.inTransaction()){
return;
}
cache.put(allInfosKey(identifier) ,versionInfos, getVersionInfoGroup());
}
}
105 changes: 54 additions & 51 deletions dotCMS/src/main/java/com/dotmarketing/business/VersionableFactoryImpl.java
View file
Expand Up @@ -3,6 +3,8 @@
import static com.dotcms.util.CollectionsUtils.set;
import static com.dotcms.variant.VariantAPI.DEFAULT_VARIANT;

import com.dotcms.concurrent.DotConcurrentFactory;
import com.dotcms.concurrent.lock.IdentifierStripedLock;
import com.dotcms.repackage.com.google.common.annotations.VisibleForTesting;
import com.dotcms.util.transform.TransformerLocator;
import com.dotcms.variant.model.Variant;
Expand All @@ -13,6 +15,7 @@
import com.dotmarketing.db.DbConnectionFactory;
import com.dotmarketing.db.HibernateUtil;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotRuntimeException;
import com.dotmarketing.exception.DotSecurityException;
import com.dotmarketing.portlets.containers.business.ContainerAPI;
import com.dotmarketing.portlets.containers.model.Container;
Expand All @@ -31,6 +34,9 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;

import org.apache.commons.beanutils.BeanUtils;

/**
Expand All @@ -44,6 +50,7 @@
public class VersionableFactoryImpl extends VersionableFactory {

private final String fourOhFour = "NOTFOUND";
private IdentifierStripedLock lockManager;

IdentifierAPI iapi = null;
IdentifierCache icache = null;
Expand All @@ -61,6 +68,7 @@ public class VersionableFactoryImpl extends VersionableFactory {
public VersionableFactoryImpl() {
this(APILocator.getIdentifierAPI(), CacheLocator.getIdentifierCache(), APILocator.getUserAPI(),
APILocator.getContainerAPI(), APILocator.getTemplateAPI());
lockManager = DotConcurrentFactory.getInstance().getIdentifierStripedLock();
}

@VisibleForTesting
Expand All @@ -71,6 +79,7 @@ public VersionableFactoryImpl(IdentifierAPI identifierApi, IdentifierCache ident
this.userApi = userApi;
this.containerApi = containerApi;
this.templateApi = templateApi;
lockManager = DotConcurrentFactory.getInstance().getIdentifierStripedLock();
}

@Override
Expand Down Expand Up @@ -389,56 +398,20 @@ public Optional<ContentletVersionInfo> findAnyContentletVersionInfo(final String
@Override
public Optional<ContentletVersionInfo> findAnyContentletVersionInfo(final String identifier, final boolean deleted)
throws DotDataException {
final DotConnect dotConnect = new DotConnect()
.setSQL("SELECT * FROM contentlet_version_info WHERE identifier= ? AND deleted = ? "
+ "AND variant_id = '"+DEFAULT_VARIANT.name()+"'")
.addParam(identifier)
.addParam(deleted)
.setMaxRows(1);

final List<ContentletVersionInfo> versionInfos = TransformerLocator
.createContentletVersionInfoTransformer(dotConnect.loadObjectResults()).asList();

return versionInfos == null || versionInfos.isEmpty()
? Optional.empty()
: Optional.of(versionInfos.get(0));
return findContentletVersionInfos(identifier, DEFAULT_VARIANT.name(), 0).stream().filter(cvi->!cvi.isDeleted() || deleted ).findAny();
}

@Override
public Optional<ContentletVersionInfo> findAnyContentletVersionInfoAnyVariant(final String identifier, final boolean deleted)
throws DotDataException {
final DotConnect dotConnect = new DotConnect()
.setSQL("SELECT * FROM contentlet_version_info WHERE identifier= ? AND deleted = ?")
.addParam(identifier)
.addParam(deleted)
.setMaxRows(1);

final List<ContentletVersionInfo> versionInfos = TransformerLocator
.createContentletVersionInfoTransformer(dotConnect.loadObjectResults()).asList();

return versionInfos == null || versionInfos.isEmpty()
? Optional.empty()
: Optional.of(versionInfos.get(0));
return findContentletVersionInfos(identifier, null, 0).stream().filter(cvi->!cvi.isDeleted() || deleted ).findAny();
}

@Override
public Optional<ContentletVersionInfo> findAnyContentletVersionInfo(final String identifier,
final String variant, final boolean deleted)
throws DotDataException {
final DotConnect dotConnect = new DotConnect()
.setSQL("SELECT * FROM contentlet_version_info WHERE identifier= ? AND deleted = ? "
+ "AND variant_id = ?")
.addParam(identifier)
.addParam(deleted)
.addParam(variant)
.setMaxRows(1);

final List<ContentletVersionInfo> versionInfos = TransformerLocator
.createContentletVersionInfoTransformer(dotConnect.loadObjectResults()).asList();

return versionInfos == null || versionInfos.isEmpty()
? Optional.empty()
: Optional.of(versionInfos.get(0));
return findContentletVersionInfos(identifier, variant, 0).stream().filter(cvi->!cvi.isDeleted() || deleted ).findAny();
}

private List<ContentletVersionInfo> findContentletVersionInfos(final String identifier,
Expand All @@ -448,22 +421,31 @@ private List<ContentletVersionInfo> findContentletVersionInfos(final String iden

private List<ContentletVersionInfo> findContentletVersionInfos(final String identifier, final String variantName,
final int maxResults) throws DotDataException, DotStateException {
final DotConnect dotConnect = new DotConnect();
List<ContentletVersionInfo> infos = findAllContentletVersionInfos(identifier);

if (UtilMethods.isSet(variantName)) {
dotConnect.setSQL(
"SELECT * FROM contentlet_version_info WHERE identifier=? AND variant_id = ?")
.addParam(identifier)
.addParam(variantName);
} else {
dotConnect.setSQL(
"SELECT * FROM contentlet_version_info WHERE identifier=?")
.addParam(identifier);
infos = infos.stream().filter(i-> variantName.equals(i.getVariant())).collect(Collectors.toList());
}

if (maxResults > 0) {
dotConnect.setMaxRows(maxResults);
if (maxResults > 0 && infos.size() > maxResults) {
infos = infos.subList(0, maxResults);
}
return infos;

}

/**
* this will return ALL CVIs from the database
* @param identifier
* @return
* @throws DotDataException
* @throws DotStateException
*/
private List<ContentletVersionInfo> findContentletVersionInfosInDB(final String identifier) throws DotDataException, DotStateException {

DotConnect dotConnect = new DotConnect().setSQL(
"SELECT * FROM contentlet_version_info WHERE identifier=?")
.addParam(identifier);

final List<ContentletVersionInfo> versionInfos = TransformerLocator
.createContentletVersionInfoTransformer(dotConnect.loadObjectResults()).asList();
Expand All @@ -476,7 +458,28 @@ private List<ContentletVersionInfo> findContentletVersionInfos(final String iden
@Override
protected List<ContentletVersionInfo> findAllContentletVersionInfos(final String identifier)
throws DotDataException, DotStateException {
return findContentletVersionInfos(identifier, -1);
if(identifier==null){
throw new DotRuntimeException("identifier cannot be null");
}

final List<ContentletVersionInfo> infos = icache.getContentVersionInfos(identifier);
if (infos != null) {
return infos;
}

try{
return lockManager.tryLock(identifier,()->{
final List<ContentletVersionInfo> infos2 = icache.getContentVersionInfos(identifier);
if (infos2 != null) {
return infos2;
}
final List<ContentletVersionInfo> infos3 = findContentletVersionInfosInDB(identifier);
icache.putContentVersionInfos(identifier, infos3);
return infos3;
});
} catch (Throwable e) {
throw new RuntimeException(e);
}
}

@Override
Expand Down

0 comments on commit 48caf15

Please sign in to comment.