From 4b3930e124460fb8f574cbb7b57571fc79460d4f Mon Sep 17 00:00:00 2001 From: Steven Glover Date: Mon, 31 Oct 2016 18:22:36 +0000 Subject: [PATCH 01/11] MNT-16541 "Cache update for custom workflow model fails" remove CMISDictionaryReload workaround, fix formatting, remove commented out code, fix write locks (locks were too wide) --- .../CMISAbstractDictionaryService.java | 180 +++++++++--------- .../dictionary/CMISDictionaryReload.java | 77 -------- .../CoreDictionaryRegistryImpl.java | 26 --- .../repo/dictionary/DictionaryDAOImpl.java | 6 - 4 files changed, 89 insertions(+), 200 deletions(-) delete mode 100644 src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryReload.java diff --git a/src/main/java/org/alfresco/opencmis/dictionary/CMISAbstractDictionaryService.java b/src/main/java/org/alfresco/opencmis/dictionary/CMISAbstractDictionaryService.java index a7c12510b7..89e329704c 100644 --- a/src/main/java/org/alfresco/opencmis/dictionary/CMISAbstractDictionaryService.java +++ b/src/main/java/org/alfresco/opencmis/dictionary/CMISAbstractDictionaryService.java @@ -73,20 +73,21 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea private final ReentrantReadWriteLock registryLock = new ReentrantReadWriteLock(); private final WriteLock registryWriteLock = registryLock.writeLock(); private final ReadLock registryReadLock = registryLock.readLock(); - + // note: cache is tenant-aware (if using TransctionalCache impl) - private SimpleCache singletonCache; // eg. for openCmisDictionaryRegistry + private SimpleCache cmisRegistryCache; private final String KEY_OPENCMIS_DICTIONARY_REGISTRY = "key.openCmisDictionaryRegistry"; public void setTenantService(TenantService tenantService) { - this.tenantService = tenantService; - } + this.tenantService = tenantService; + } - /** + /** * Set the mapping service * - * @param cmisMapping CMISMapping + * @param cmisMapping + * CMISMapping */ public void setCmisMapping(CMISMapping cmisMapping) { @@ -96,7 +97,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea /** * Set the property accessor mapping service * - * @param accessorMapping mapping + * @param accessorMapping + * mapping */ public void setPropertyAccessorMapping(PropertyAccessorMapping accessorMapping) { @@ -106,7 +108,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea /** * Set the property lucene mapping service * - * @param luceneBuilderMapping mapping + * @param luceneBuilderMapping + * mapping */ public void setPropertyLuceneBuilderMapping(PropertyLuceneBuilderMapping luceneBuilderMapping) { @@ -116,7 +119,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea /** * Set the dictionary Service * - * @param dictionaryService DictionaryService + * @param dictionaryService + * DictionaryService */ public void setDictionaryService(DictionaryService dictionaryService) { @@ -126,7 +130,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea /** * Set the dictionary DAO * - * @param dictionaryDAO DictionaryDAO + * @param dictionaryDAO + * DictionaryDAO */ public void setDictionaryDAO(DictionaryDAO dictionaryDAO) { @@ -135,63 +140,57 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea public void setSingletonCache(SimpleCache singletonCache) { - this.singletonCache = singletonCache; + this.cmisRegistryCache = singletonCache; } protected interface DictionaryInitializer { - Collection createDefinitions(CMISDictionaryRegistry cmisRegistry); - Collection createDefinitions(CMISDictionaryRegistry cmisRegistry, CompiledModel model); + Collection createDefinitions(CMISDictionaryRegistry cmisRegistry); + + Collection createDefinitions(CMISDictionaryRegistry cmisRegistry, + CompiledModel model); } protected abstract DictionaryInitializer getCoreDictionaryInitializer(); + protected abstract DictionaryInitializer getTenantDictionaryInitializer(); protected CMISDictionaryRegistry getRegistry() { - String tenant = TenantUtil.getCurrentDomain(); - return getRegistry(tenant); + String tenant = TenantUtil.getCurrentDomain(); + return getRegistry(tenant); } CMISDictionaryRegistry getRegistry(String tenant) { - CMISDictionaryRegistry cmisRegistry = null; - boolean readLockReleased = false; - //Make sure that DictionaryRegistry exist - dictionaryDAO.getDictionaryRegistry(tenant); + CMISDictionaryRegistry cmisRegistry = null; - registryReadLock.lock(); - try - { - String cacheKey = getCacheKey(tenant); - cmisRegistry = singletonCache.get(cacheKey); - if(cmisRegistry == null) - { - registryReadLock.unlock(); - readLockReleased = true; + String cacheKey = getCacheKey(tenant); - registryWriteLock.lock(); - try - { - cmisRegistry = singletonCache.get(cacheKey); - if(cmisRegistry == null) - { - cmisRegistry = createDictionaryRegistry(tenant); - } - } - finally - { - registryWriteLock.unlock(); - } - } - } - finally - { - if(!readLockReleased) - { - registryReadLock.unlock(); - } - } + registryReadLock.lock(); + try + { + cmisRegistry = cmisRegistryCache.get(cacheKey); + } + finally + { + registryReadLock.unlock(); + } + + if (cmisRegistry == null) + { + cmisRegistry = createDictionaryRegistry(tenant); + + registryWriteLock.lock(); + try + { + cmisRegistryCache.put(cacheKey, cmisRegistry); + } + finally + { + registryWriteLock.unlock(); + } + } return cmisRegistry; } @@ -199,9 +198,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea /* * (non-Javadoc) * - * @see - * org.springframework.extensions.surf.util.AbstractLifecycleBean#onBootstrap - * (org.springframework.context.ApplicationEvent) + * @see org.springframework.extensions.surf.util.AbstractLifecycleBean# + * onBootstrap (org.springframework.context.ApplicationEvent) */ @Override protected void onBootstrap(ApplicationEvent event) @@ -233,42 +231,45 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea private String getCacheKey() { - String tenant = tenantService.getCurrentUserDomain(); - return getCacheKey(tenant); + String tenant = tenantService.getCurrentUserDomain(); + return getCacheKey(tenant); } private String getCacheKey(String tenant) { - String cacheKey = KEY_OPENCMIS_DICTIONARY_REGISTRY + "." + tenant + "." + cmisMapping.getCmisVersion().toString(); - return cacheKey; + String cacheKey = KEY_OPENCMIS_DICTIONARY_REGISTRY + "." + tenant + "." + + cmisMapping.getCmisVersion().toString(); + return cacheKey; } protected CMISDictionaryRegistry createCoreDictionaryRegistry() { - CMISDictionaryRegistryImpl cmisRegistry = new CMISDictionaryRegistryImpl(this, cmisMapping, dictionaryService, - getCoreDictionaryInitializer()); + CMISDictionaryRegistryImpl cmisRegistry = new CMISDictionaryRegistryImpl(this, cmisMapping, dictionaryService, + getCoreDictionaryInitializer()); cmisRegistry.init(); return cmisRegistry; } protected CMISDictionaryRegistry createTenantDictionaryRegistry(String tenant) { - CMISDictionaryRegistryImpl cmisRegistry = new CMISDictionaryRegistryImpl(this, tenant, "", - cmisMapping, dictionaryService, getTenantDictionaryInitializer()); + CMISDictionaryRegistryImpl cmisRegistry = new CMISDictionaryRegistryImpl(this, tenant, "", cmisMapping, + dictionaryService, getTenantDictionaryInitializer()); cmisRegistry.init(); return cmisRegistry; } - + protected CMISDictionaryRegistry createDictionaryRegistryWithWriteLock() { - CMISDictionaryRegistry cmisRegistry = null; - - String tenant = TenantUtil.getCurrentDomain(); + String tenant = TenantUtil.getCurrentDomain(); + CMISDictionaryRegistry cmisRegistry = createDictionaryRegistry(tenant); + String cacheKey = getCacheKey(tenant); registryWriteLock.lock(); try { - cmisRegistry = createDictionaryRegistry(tenant); + // publish new registry + cmisRegistryCache.put(cacheKey, cmisRegistry); + } finally { @@ -280,20 +281,16 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea protected CMISDictionaryRegistry createDictionaryRegistry(String tenant) { - CMISDictionaryRegistry cmisRegistry = null; - String cacheKey = getCacheKey(tenant); + CMISDictionaryRegistry cmisRegistry = null; - if(tenant.equals(TenantService.DEFAULT_DOMAIN)) - { - cmisRegistry = createCoreDictionaryRegistry(); - } - else - { - cmisRegistry = createTenantDictionaryRegistry(tenant); - } - - // publish new registry - singletonCache.put(cacheKey, cmisRegistry); + if (tenant.equals(TenantService.DEFAULT_DOMAIN)) + { + cmisRegistry = createCoreDictionaryRegistry(); + } + else + { + cmisRegistry = createTenantDictionaryRegistry(tenant); + } return cmisRegistry; } @@ -373,7 +370,7 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea { return getRegistry().getAssocDefByQName(cmisMapping.getCmisType(clazz)); } - + @Override public TypeDefinitionWrapper findTypeByQueryName(String queryName) { @@ -418,15 +415,16 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea @Override public List getAllTypes() { - // TODO is there a way of not having to reconstruct this every time? - return Collections.unmodifiableList(new ArrayList(getRegistry().getTypeDefs())); + // TODO is there a way of not having to reconstruct this every time? + return Collections.unmodifiableList(new ArrayList(getRegistry().getTypeDefs())); } @Override public List getAllTypes(boolean includeParent) { - // TODO is there a way of not having to reconstruct this every time? - return Collections.unmodifiableList(new ArrayList(getRegistry().getTypeDefs(includeParent))); + // TODO is there a way of not having to reconstruct this every time? + return Collections + .unmodifiableList(new ArrayList(getRegistry().getTypeDefs(includeParent))); } @Override @@ -454,7 +452,7 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea @Override public void modelAdded(CompiledModel model, String tenantDomain) { - getRegistry(tenantDomain).addModel(model); + getRegistry(tenantDomain).addModel(model); } /* @@ -465,7 +463,7 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea @Override public void afterDictionaryInit() { - createDictionaryRegistryWithWriteLock(); + createDictionaryRegistryWithWriteLock(); } /* @@ -480,8 +478,8 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea registryWriteLock.lock(); try { - String cacheKey = getCacheKey(); - singletonCache.remove(cacheKey); + String cacheKey = getCacheKey(); + cmisRegistryCache.remove(cacheKey); } finally { @@ -493,7 +491,7 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea public List getChildren(String typeId) { List children = getRegistry().getChildren(typeId); - + for (TypeDefinitionWrapper child : children) { if (child != null && child.getTypeDefinition(false).getDisplayName() == null) @@ -501,7 +499,7 @@ public abstract class CMISAbstractDictionaryService extends AbstractLifecycleBea child.updateDefinition(dictionaryService); } } - - return children; + + return children; } } diff --git a/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryReload.java b/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryReload.java deleted file mode 100644 index cdaa202552..0000000000 --- a/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryReload.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * #%L - * Alfresco Data model classes - * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ -package org.alfresco.opencmis.dictionary; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.context.ApplicationEvent; -import org.springframework.extensions.surf.util.AbstractLifecycleBean; - -/** - * Temporary workaround for: - *
    - *
  • ACE-5041: CLONE - google docs content cannot be accessed via cmis
  • - *
- * - * TODO: Remove this bean when rework for MNT-14819 is complete. - * - * @author Matt Ward - */ -public final class CMISDictionaryReload extends AbstractLifecycleBean -{ - private static final Log log = LogFactory.getLog(CMISDictionaryReload.class); - private final CMISAbstractDictionaryService cmisDictService; - private final boolean enabled; - - public CMISDictionaryReload(CMISAbstractDictionaryService cmisDictService, boolean enabled) - { - this.cmisDictService = cmisDictService; - this.enabled = enabled; - } - - public void reload() - { - if (enabled) - { - // Avoid deadlock by making sure we already have a registry present. - cmisDictService.getRegistry(); - log.debug("Reloading CMIS dictionary."); - cmisDictService.afterDictionaryInit(); - } - } - - @Override - protected void onBootstrap(ApplicationEvent event) - { - reload(); - } - - @Override - protected void onShutdown(ApplicationEvent event) - { - // Do nothing. - } -} diff --git a/src/main/java/org/alfresco/repo/dictionary/CoreDictionaryRegistryImpl.java b/src/main/java/org/alfresco/repo/dictionary/CoreDictionaryRegistryImpl.java index b0e88c45e7..7056bbd325 100644 --- a/src/main/java/org/alfresco/repo/dictionary/CoreDictionaryRegistryImpl.java +++ b/src/main/java/org/alfresco/repo/dictionary/CoreDictionaryRegistryImpl.java @@ -87,33 +87,7 @@ public class CoreDictionaryRegistryImpl extends AbstractDictionaryRegistry protected QName putModelImpl(CompiledModel model) { // TODO disallow model overrides for the core dictionary - -// if(compiledModels.get(model.getModelDefinition().getName()) != null) -// { -// throw new AlfrescoRuntimeException("Cannot override existing model " + model.getModelDefinition().getName()); -// } -// -// for(M2Namespace namespace : model.getM2Model().getNamespaces()) -// { -// if(uriToModels.get(namespace.getUri()) != null) -// { -// throw new AlfrescoRuntimeException("Cannot override existing namespace " + namespace.getUri()); -// } -// } - QName qname = super.putModelImpl(model); - -// if(dictionaryDAO.isContextRefreshed()) -// { -// for(DictionaryListener listener : dictionaryDAO.getDictionaryListeners()) -// { -// if(listener instanceof ExtendedDictionaryListener) -// { -// ((ExtendedDictionaryListener)listener).coreModelAdded(model); -// } -// } -// } - return qname; } diff --git a/src/main/java/org/alfresco/repo/dictionary/DictionaryDAOImpl.java b/src/main/java/org/alfresco/repo/dictionary/DictionaryDAOImpl.java index 200eb6b4a5..f55933b5ae 100644 --- a/src/main/java/org/alfresco/repo/dictionary/DictionaryDAOImpl.java +++ b/src/main/java/org/alfresco/repo/dictionary/DictionaryDAOImpl.java @@ -186,9 +186,6 @@ public class DictionaryDAOImpl implements DictionaryDAO, NamespaceDAO, { DictionaryRegistry dictionaryRegistry = new CoreDictionaryRegistryImpl( this); - getThreadLocal().put("", dictionaryRegistry); - dictionaryRegistry.init(); - getThreadLocal().remove(""); return dictionaryRegistry; } @@ -202,9 +199,6 @@ public class DictionaryDAOImpl implements DictionaryDAO, NamespaceDAO, { DictionaryRegistry dictionaryRegistry = new TenantDictionaryRegistryImpl( DictionaryDAOImpl.this, tenant); - getThreadLocal().put(tenant, dictionaryRegistry); - dictionaryRegistry.init(); - getThreadLocal().remove(tenant); return dictionaryRegistry; } }, tenantService.getDomainUser( From a7378ac0fcbbe9c788f29987b50d299a44da92cf Mon Sep 17 00:00:00 2001 From: Steven Glover Date: Wed, 2 Nov 2016 12:41:13 +0000 Subject: [PATCH 02/11] MNT-16541: "Cache update for custom workflow model fails" --- .../CMISDictionaryRegistryImpl.java | 1 - .../repo/dictionary/CompiledModelsCache.java | 48 +++++++++++++++++++ .../CoreDictionaryRegistryImpl.java | 6 --- .../TenantDictionaryRegistryImpl.java | 6 --- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryRegistryImpl.java b/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryRegistryImpl.java index 3db14e8c31..26c4dfc8dd 100644 --- a/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryRegistryImpl.java +++ b/src/main/java/org/alfresco/opencmis/dictionary/CMISDictionaryRegistryImpl.java @@ -33,7 +33,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.opencmis.dictionary.CMISAbstractDictionaryService.DictionaryInitializer; import org.alfresco.opencmis.mapping.CMISMapping; import org.alfresco.repo.dictionary.CompiledModel; diff --git a/src/main/java/org/alfresco/repo/dictionary/CompiledModelsCache.java b/src/main/java/org/alfresco/repo/dictionary/CompiledModelsCache.java index a193cbd0f9..9f5d2c8394 100644 --- a/src/main/java/org/alfresco/repo/dictionary/CompiledModelsCache.java +++ b/src/main/java/org/alfresco/repo/dictionary/CompiledModelsCache.java @@ -29,6 +29,9 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.tenant.TenantService; import org.alfresco.util.cache.AbstractAsynchronouslyRefreshedCache; +import org.alfresco.util.cache.RefreshableCacheEvent; +import org.alfresco.util.cache.RefreshableCacheListener; +import org.alfresco.util.cache.RefreshableCacheRefreshedEvent; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -103,4 +106,49 @@ public class CompiledModelsCache extends AbstractAsynchronouslyRefreshedCache Date: Wed, 2 Nov 2016 12:41:32 +0000 Subject: [PATCH 03/11] MNT-16541: "Cache update for custom workflow model fails" create RC1 for testing --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ec6a6999fb..c9ed09624e 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ alfresco-data-model Alfresco Data Model Alfresco Data Model classes - 6.4-SNAPSHOT + REPO1259-6.4-RC1 scm:git:https://gitlab.alfresco.com/platform/alfresco-data-model.git @@ -145,7 +145,7 @@ org.alfresco alfresco-core - 6.5 + REPO1259-6.7-RC1