diff --git a/config/alfresco/content-services-context.xml b/config/alfresco/content-services-context.xml index b3b2210e73..5dcf26bf59 100644 --- a/config/alfresco/content-services-context.xml +++ b/config/alfresco/content-services-context.xml @@ -294,8 +294,15 @@ + + + - + + + + + diff --git a/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistry.java b/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistry.java index 205b7f13fb..66ea3e89cd 100644 --- a/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistry.java +++ b/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistry.java @@ -35,7 +35,10 @@ import org.apache.commons.logging.LogFactory; *

* The transformers themselves are used to determine the applicability * of a particular transformation. - * + *

+ * The actual selection of a transformer is done by the injected + * {@link TransformerSelector}. + * * @see org.alfresco.repo.content.transform.ContentTransformer * * @author Derek Hulley @@ -46,11 +49,14 @@ public class ContentTransformerRegistry private List transformers; + private final TransformerSelector transformerSelector; + /** * @param mimetypeMap all the mimetypes available to the system */ - public ContentTransformerRegistry() + public ContentTransformerRegistry(TransformerSelector transformerSelector) { + this.transformerSelector = transformerSelector; this.transformers = new ArrayList(10); } @@ -108,67 +114,7 @@ public class ContentTransformerRegistry public List getActiveTransformers(String sourceMimetype, long sourceSize, String targetMimetype, TransformationOptions options) { // Get the list of transformers - List transformers = findTransformers(sourceMimetype, sourceSize, targetMimetype, options); - final Map activeTransformers = new HashMap(); - - // identify the performance of all the transformers - for (ContentTransformer transformer : transformers) - { - long transformationTime = transformer.getTransformationTime(); - activeTransformers.put(transformer, transformationTime); - } - - // sort by performance (quicker is "better") - List sorted = new ArrayList(activeTransformers.keySet()); - Collections.sort(sorted, new Comparator() { - - @Override - public int compare(ContentTransformer a, ContentTransformer b) - { - return activeTransformers.get(a).compareTo(activeTransformers.get(b)); - } - - }); - - // All done - return sorted; - } - - /** - * Gets all transformers, of equal reliability, that can perform the requested transformation. - * - * @return Returns best transformer for the translation - null if all - * score 0.0 on reliability - */ - private List findTransformers(String sourceMimetype, long sourceSize, String targetMimetype, TransformationOptions options) - { - List transformers = new ArrayList(2); - boolean foundExplicit = false; - - // loop through transformers - for (ContentTransformer transformer : this.transformers) - { - if (transformer.isTransformable(sourceMimetype, sourceSize, targetMimetype, options) == true) - { - if (transformer.isExplicitTransformation(sourceMimetype, targetMimetype, options) == true) - { - if (foundExplicit == false) - { - transformers.clear(); - foundExplicit = true; - } - transformers.add(transformer); - } - else - { - if (foundExplicit == false) - { - transformers.add(transformer); - } - } - } - } - // done + List transformers = transformerSelector.selectTransformers(this.transformers, sourceMimetype, sourceSize, targetMimetype, options); if (logger.isDebugEnabled()) { logger.debug("Searched for transformer: \n" + diff --git a/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistryTest.java b/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistryTest.java index 05a4de2942..c3bd6b9b61 100644 --- a/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistryTest.java +++ b/source/java/org/alfresco/repo/content/transform/ContentTransformerRegistryTest.java @@ -70,7 +70,7 @@ public class ContentTransformerRegistryTest extends AbstractContentTransformerTe bytes[i] = (byte)i; } // create the dummyRegistry - dummyRegistry = new ContentTransformerRegistry(); + dummyRegistry = new ContentTransformerRegistry(new TransformerSelectorImpl()); // create some dummy transformers for reliability tests new DummyTransformer(mimetypeService, transformerDebug, dummyRegistry, A, B, 10L); new DummyTransformer(mimetypeService, transformerDebug, dummyRegistry, A, B, 10L); diff --git a/source/java/org/alfresco/repo/content/transform/TransformerDebug.java b/source/java/org/alfresco/repo/content/transform/TransformerDebug.java index 269a96e514..1c6b2104fa 100644 --- a/source/java/org/alfresco/repo/content/transform/TransformerDebug.java +++ b/source/java/org/alfresco/repo/content/transform/TransformerDebug.java @@ -27,7 +27,6 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import org.alfresco.model.ContentModel; -import org.alfresco.service.cmr.repository.InvalidNodeRefException; import org.alfresco.service.cmr.repository.MimetypeService; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; @@ -57,6 +56,8 @@ import org.apache.commons.logging.LogFactory; public class TransformerDebug { private static final Log logger = LogFactory.getLog(TransformerDebug.class); + private static final TransformerLog info = new TransformerLog(); + private static final String NO_TRANSFORMERS = "No transformers"; private enum Call { @@ -119,16 +120,19 @@ public class TransformerDebug private Call callType; private int childId; private Set unavailableTransformers; -// See debug(String, Throwable) as to why this is commented out -// private Throwable lastThrowable; - - private Frame(Frame parent, String fromUrl, String sourceMimetype, String targetMimetype, - TransformationOptions options, Call pushCall, boolean origDebugOutput) + private String failureReason; + private long sourceSize; + private String transformerName; + + private Frame(Frame parent, String transformerName, String fromUrl, String sourceMimetype, String targetMimetype, + long sourceSize, TransformationOptions options, Call pushCall, boolean origDebugOutput) { this.id = parent == null ? -1 : ++parent.childId; this.fromUrl = fromUrl; + this.transformerName = transformerName; this.sourceMimetype = sourceMimetype; this.targetMimetype = targetMimetype; + this.sourceSize = sourceSize; this.options = options; this.callType = pushCall; this.origDebugOutput = origDebugOutput; @@ -143,18 +147,48 @@ public class TransformerDebug } return id; } + + private void setFailureReason(String failureReason) + { + this.failureReason = failureReason; + } + + private String getFailureReason() + { + return failureReason; + } + + private void setSourceSize(long sourceSize) + { + this.sourceSize = sourceSize; + } + + public long getSourceSize() + { + return sourceSize; + } + + private void setTransformerName(String transformerName) + { + this.transformerName = transformerName; + } + + public String getTransformerName() + { + return transformerName; + } } private class UnavailableTransformer { private final String name; - private final String reason; + private final long maxSourceSizeKBytes; private final transient boolean debug; - UnavailableTransformer(String name, String reason, boolean debug) + UnavailableTransformer(String name, long maxSourceSizeKBytes, boolean debug) { this.name = name; - this.reason = reason; + this.maxSourceSizeKBytes = maxSourceSizeKBytes; this.debug = debug; } @@ -162,7 +196,7 @@ public class TransformerDebug public int hashCode() { int hashCode = 37 * name.hashCode(); - hashCode += 37 * reason.hashCode(); + hashCode += 37 * maxSourceSizeKBytes; return hashCode; } @@ -178,7 +212,7 @@ public class TransformerDebug UnavailableTransformer that = (UnavailableTransformer) obj; return EqualsHelper.nullSafeEquals(name, that.name) && - EqualsHelper.nullSafeEquals(reason, that.reason); + maxSourceSizeKBytes == that.maxSourceSizeKBytes; } else { @@ -248,7 +282,7 @@ public class TransformerDebug } } - private void push(String name, String fromUrl, String sourceMimetype, String targetMimetype, + private void push(String transformerName, String fromUrl, String sourceMimetype, String targetMimetype, long sourceSize, TransformationOptions options, Call callType) { Deque ourStack = ThreadInfo.getStack(); @@ -256,18 +290,20 @@ public class TransformerDebug if (callType == Call.TRANSFORM && frame != null && frame.callType == Call.AVAILABLE) { + frame.setTransformerName(transformerName); + frame.setSourceSize(sourceSize); frame.callType = Call.AVAILABLE_AND_TRANSFORM; } // Create a new frame. Logging level is set to trace if the file size is 0 boolean origDebugOutput = ThreadInfo.setDebugOutput(ThreadInfo.getDebugOutput() && sourceSize != 0); - frame = new Frame(frame, fromUrl, sourceMimetype, targetMimetype, options, callType, origDebugOutput); + frame = new Frame(frame, transformerName, fromUrl, sourceMimetype, targetMimetype, sourceSize, options, callType, origDebugOutput); ourStack.push(frame); if (callType == Call.TRANSFORM) { // Log the basic info about this transformation - logBasicDetails(frame, sourceSize, name, (ourStack.size() == 1)); + logBasicDetails(frame, sourceSize, transformerName, (ourStack.size() == 1)); } } @@ -288,20 +324,12 @@ public class TransformerDebug String name = (!isTransformableStack.isEmpty()) ? isTransformableStack.getFirst() : getName(transformer); - String reason = "> "+fileSize(maxSourceSizeKBytes*1024); boolean debug = (maxSourceSizeKBytes != 0); - if (ourStack.size() == 1) + if (frame.unavailableTransformers == null) { - if (frame.unavailableTransformers == null) - { - frame.unavailableTransformers = new HashSet(); - } - frame.unavailableTransformers.add(new UnavailableTransformer(name, reason, debug)); - } - else - { - log("-- " + name + ' ' + reason, debug); + frame.unavailableTransformers = new HashSet(); } + frame.unavailableTransformers.add(new UnavailableTransformer(name, maxSourceSizeKBytes, debug)); } } } @@ -315,18 +343,25 @@ public class TransformerDebug { Deque ourStack = ThreadInfo.getStack(); Frame frame = ourStack.peek(); + boolean firstLevel = ourStack.size() == 1; // Override setDebugOutput(false) to allow debug when there are transformers but they are all unavailable // Note once turned on we don't turn it off again. - if (transformers.size() == 0 && - frame.unavailableTransformers != null && - frame.unavailableTransformers.size() != 0) { - ThreadInfo.setDebugOutput(true); + if (transformers.size() == 0) + { + frame.setFailureReason(NO_TRANSFORMERS); + if (frame.unavailableTransformers != null && + frame.unavailableTransformers.size() != 0) + { + ThreadInfo.setDebugOutput(true); + } } + frame.setSourceSize(sourceSize); + // Log the basic info about this transformation logBasicDetails(frame, sourceSize, calledFrom + ((transformers.size() == 0) ? " NO transformers" : ""), - (ourStack.size() == 1)); + firstLevel); // Report available and unavailable transformers char c = 'a'; @@ -346,8 +381,8 @@ public class TransformerDebug for (UnavailableTransformer unavailable: frame.unavailableTransformers) { int pad = longestNameLength - unavailable.name.length(); - log("--" + (c++) + ") " + unavailable.name + spaces(pad+1) + unavailable.reason, - unavailable.debug); + String reason = "> "+fileSize(unavailable.maxSourceSizeKBytes*1024); + log("--" + (c++) + ") " + unavailable.name + spaces(pad+1) + reason, unavailable.debug); } } } @@ -476,38 +511,100 @@ public class TransformerDebug if (!ourStack.isEmpty()) { Frame frame = ourStack.peek(); + if ((frame.callType == callType) || (frame.callType == Call.AVAILABLE_AND_TRANSFORM && callType == Call.AVAILABLE)) { - if (!suppressFinish && (ourStack.size() == 1 || logger.isTraceEnabled())) + int size = ourStack.size(); + String ms = ms(System.currentTimeMillis() - frame.start); + + logInfo(frame, size, ms); + + boolean firstLevel = size == 1; + if (!suppressFinish && (firstLevel || logger.isTraceEnabled())) { - boolean topFrame = ourStack.size() == 1; - log("Finished in " + - ms(System.currentTimeMillis() - frame.start) + + log("Finished in " + ms + (frame.callType == Call.AVAILABLE ? " Transformer NOT called" : "") + - (topFrame ? "\n" : ""), - topFrame); + (firstLevel ? "\n" : ""), + firstLevel); } setDebugOutput(frame.origDebugOutput); ourStack.pop(); - -// See debug(String, Throwable) as to why this is commented out -// if (ourStack.size() >= 1) -// { -// ourStack.peek().lastThrowable = frame.lastThrowable; -// } } } } + private void logInfo(Frame frame, int size, String ms) + { + if (info.isDebugEnabled()) + { + String failureReason = frame.getFailureReason(); + boolean firstLevel = size == 1; + String sourceExt = getMimetypeExt(frame.sourceMimetype); + String targetExt = getMimetypeExt(frame.targetMimetype); + String fileName = getFileName(frame.options, firstLevel, frame.sourceSize); + long sourceSize = frame.getSourceSize(); + String transformerName = frame.getTransformerName(); + String level = null; + boolean debug = false; + if (NO_TRANSFORMERS.equals(failureReason)) + { + debug = firstLevel; + level = "INFO"; + failureReason = NO_TRANSFORMERS; + if (frame.unavailableTransformers != null) + { + level = "WARN"; + long smallestMaxSourceSizeKBytes = Long.MAX_VALUE; + for (UnavailableTransformer unavailable: frame.unavailableTransformers) + { + if (smallestMaxSourceSizeKBytes > unavailable.maxSourceSizeKBytes) + { + smallestMaxSourceSizeKBytes = unavailable.maxSourceSizeKBytes; + } + } + failureReason = "No transformers as file is > "+fileSize(smallestMaxSourceSizeKBytes*1024); + } + } + else if (frame.callType == Call.TRANSFORM) + { + level = failureReason == null || failureReason.length() == 0 ? "INFO" : "ERROR"; + + // Use TRACE logging for all but the first TRANSFORM + debug = size == 1 || (size == 2 && ThreadInfo.getStack().peekLast().callType != Call.TRANSFORM); + } + + if (level != null) + { + infoLog(getReference(debug), sourceExt, targetExt, level, fileName, sourceSize, transformerName, failureReason, ms, debug); + } + } + } + + private void infoLog(String reference, String sourceExt, String targetExt, String level, String fileName, + long sourceSize, String transformerName, String failureReason, String ms, boolean debug) + { + String message = + " "+reference + + sourceExt + + targetExt + + (level == null ? "" : level+' ') + + (fileName == null ? "" : fileName) + + (sourceSize >= 0 ? ' '+fileSize(sourceSize) : "") + + (transformerName == null ? "" : ' '+transformerName) + + (failureReason == null ? "" : ' '+failureReason) + + ' '+ms; + info.log(message, debug); + } + /** * Indicates if any logging is required. */ public boolean isEnabled() { // Don't check ThreadInfo.getDebugOutput() as availableTransformers() may upgrade from trace to debug. - return logger.isDebugEnabled(); + return logger.isDebugEnabled() || info.isDebugEnabled(); } /** @@ -545,40 +642,32 @@ public class TransformerDebug if (isEnabled()) { log(message + ' ' + t.getMessage()); - -// // Generally the full stack is not needed as transformer -// // Exceptions get logged as a Error higher up, so including -// // the stack trace has been found not to be needed. Keeping -// // the following code and code that sets lastThrowable just -// // in case we need it after all. -// -// Frame frame = ThreadInfo.getStack().peek(); -// boolean newThrowable = isNewThrowable(frame.lastThrowable, t); -// frame.lastThrowable = t; -// -// if (newThrowable) -// { -// log(message, t, true); -// } -// else -// { -// log(message + ' ' + t.getMessage()); -// } + + Deque ourStack = ThreadInfo.getStack(); + if (!ourStack.isEmpty()) + { + Frame frame = ourStack.peek(); + frame.setFailureReason(message +' '+ getRootCauseMessage(t)); + } } } -// private boolean isNewThrowable(Throwable lastThrowable, Throwable t) -// { -// while (t != null) -// { -// if (lastThrowable == t) -// { -// return false; -// } -// t = t.getCause(); -// } -// return true; -// } + private String getRootCauseMessage(Throwable t) + { + Throwable cause = t; + while (cause != null) + { + t = cause; + cause = t.getCause(); + } + + String message = t.getMessage(); + if (message == null || message.length() == 0) + { + message = t.getClass().getSimpleName(); + } + return message; + } private void log(String message) { @@ -592,13 +681,13 @@ public class TransformerDebug private void log(String message, Throwable t, boolean debug) { - if (debug && ThreadInfo.getDebugOutput()) + if (debug && ThreadInfo.getDebugOutput() && logger.isDebugEnabled()) { - logger.debug(getReference()+message, t); + logger.debug(getReference(false)+message, t); } else if (logger.isTraceEnabled()) { - logger.trace(getReference()+message, t); + logger.trace(getReference(false)+message, t); } } @@ -609,19 +698,15 @@ public class TransformerDebug */ public T setCause(T t) { -// See debug(String, Throwable) as to why this is commented out -// if (isEnabled()) -// { -// Deque ourStack = ThreadInfo.getStack(); -// if (!ourStack.isEmpty()) -// { -// ourStack.peek().lastThrowable = t; -// } -// } return t; } - private String getReference() + /** + * Returns a N.N.N style reference to the transformation. + * @param firstLevelOnly indicates if only the top level should be included. + * @return a padded (fixed length) reference. + */ + private String getReference(boolean firstLevelOnly) { StringBuilder sb = new StringBuilder(""); Frame frame = null; @@ -634,6 +719,10 @@ public class TransformerDebug { sb.append(frame.getId()); lengthOfFirstId = sb.length(); + if (firstLevelOnly) + { + break; + } } else { @@ -770,3 +859,30 @@ public class TransformerDebug return sb.toString(); } } + +class TransformerLog +{ + private static final Log logger = LogFactory.getLog(TransformerLog.class); + + void log(String message, boolean debug) + { + if (debug) + { + logger.debug(message); + } + else + { + logger.trace(message); + } + } + + public boolean isDebugEnabled() + { + return logger.isDebugEnabled(); + } + + public boolean isTraceEnabled() + { + return logger.isTraceEnabled(); + } +} diff --git a/source/java/org/alfresco/repo/content/transform/TransformerSelector.java b/source/java/org/alfresco/repo/content/transform/TransformerSelector.java new file mode 100644 index 0000000000..a0279ff75e --- /dev/null +++ b/source/java/org/alfresco/repo/content/transform/TransformerSelector.java @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2005-2012 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * 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 . + */ +package org.alfresco.repo.content.transform; + +import java.util.List; + +import org.alfresco.service.cmr.repository.TransformationOptions; + +/** + * Selects a transformer from a supplied list of transformers that appear + * able to handle a given transformation. + * + * @author Alan Davis + */ +public interface TransformerSelector +{ + /** + * Returns a sorted list of transformers that identifies the order in which transformers + * should be tried. + * @param transformers an unordered list of all transformers. This + * list may be modified. + * @param sourceMimetype + * @param sourceSize + * @param targetMimetype + * @param options transformation options + * @return a sorted list of transformers, with the best one first. + */ + List selectTransformers(List transformers, + String sourceMimetype, long sourceSize, String targetMimetype, + TransformationOptions options); +} diff --git a/source/java/org/alfresco/repo/content/transform/TransformerSelectorImpl.java b/source/java/org/alfresco/repo/content/transform/TransformerSelectorImpl.java new file mode 100644 index 0000000000..3a9d8a36b8 --- /dev/null +++ b/source/java/org/alfresco/repo/content/transform/TransformerSelectorImpl.java @@ -0,0 +1,121 @@ +/* + * Copyright (C) 2005-2012 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * 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 . + */ +package org.alfresco.repo.content.transform; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.alfresco.service.cmr.repository.TransformationOptions; + +/** + * Implementation of a transformer selector that matches the code that was in place + * before a selector was introduced. It is expected that this code will be replaced. + * + * @author Alan Davis + */ +public class TransformerSelectorImpl implements TransformerSelector +{ + + @Override + public List selectTransformers(List transformers, + String sourceMimetype, long sourceSize, String targetMimetype, + TransformationOptions options) + { + transformers = findTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options); + transformers = discardNonExplicitTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options); + transformers = sortTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options); + return transformers; + } + + /** + * Reduces the list of transformers down to only those capable of doing the transformation. + */ + private List findTransformers(List allTransformers, String sourceMimetype, + long sourceSize, String targetMimetype, TransformationOptions options) + { + List transformers = new ArrayList(2); + + for (ContentTransformer transformer : allTransformers) + { + if (transformer.isTransformable(sourceMimetype, sourceSize, targetMimetype, options) == true) + { + transformers.add(transformer); + } + } + return transformers; + } + + /** + * Discards non explicit transformers if there are any explicit ones. + */ + private List discardNonExplicitTransformers(List allTransformers, String sourceMimetype, + long sourceSize, String targetMimetype, TransformationOptions options) + { + List transformers = new ArrayList(2); + boolean foundExplicit = false; + + for (ContentTransformer transformer : allTransformers) + { + if (transformer.isExplicitTransformation(sourceMimetype, targetMimetype, options) == true) + { + if (foundExplicit == false) + { + transformers.clear(); + foundExplicit = true; + } + transformers.add(transformer); + } + else + { + if (foundExplicit == false) + { + transformers.add(transformer); + } + } + } + return transformers; + } + + // sort by performance (quicker is "better") + private List sortTransformers(List transformers, + String sourceMimetype, long sourceSize, String targetMimetype, + TransformationOptions options) + { + final Map activeTransformers = new HashMap(); + for (ContentTransformer transformer : transformers) + { + long transformationTime = transformer.getTransformationTime(); + activeTransformers.put(transformer, transformationTime); + } + + List sorted = new ArrayList(activeTransformers.keySet()); + Collections.sort(sorted, new Comparator() { + @Override + public int compare(ContentTransformer a, ContentTransformer b) + { + return activeTransformers.get(a).compareTo(activeTransformers.get(b)); + } + }); + return sorted; + } +} diff --git a/source/java/org/alfresco/repo/discussion/DiscussionServiceImpl.java b/source/java/org/alfresco/repo/discussion/DiscussionServiceImpl.java index 6996244fcd..3de78a8d14 100644 --- a/source/java/org/alfresco/repo/discussion/DiscussionServiceImpl.java +++ b/source/java/org/alfresco/repo/discussion/DiscussionServiceImpl.java @@ -584,7 +584,7 @@ public class DiscussionServiceImpl implements DiscussionService NodeBackedEntity node = results.getPage().get(0); // Wrap and return - return buildPost(node.getNodeRef(), topic, node.getName(), null); + return buildPost(tenantService.getBaseName(node.getNodeRef()), topic, node.getName(), null); } @@ -820,7 +820,7 @@ public class DiscussionServiceImpl implements DiscussionService Map idToNodeRef = new HashMap(); for(NodeWithTargetsEntity e : results.getPage()) { - idToNodeRef.put(e.getId(), e.getNodeRef()); + idToNodeRef.put(e.getId(), tenantService.getBaseName(e.getNodeRef())); } Map> idToReplies = new HashMap>(); @@ -869,7 +869,7 @@ public class DiscussionServiceImpl implements DiscussionService { for (NodeWithTargetsEntity entity : replies) { - calculateRepliesPreLoad(entity.getNodeRef(), preLoad, idToReplies, levels-1); + calculateRepliesPreLoad(tenantService.getBaseName(entity.getNodeRef()), preLoad, idToReplies, levels-1); } } } @@ -885,7 +885,7 @@ public class DiscussionServiceImpl implements DiscussionService { for (NodeWithTargetsEntity entity : replyEntities) { - PostInfo replyPost = buildPost(entity.getNodeRef(), post.getTopic(), entity.getName(), null); + PostInfo replyPost = buildPost(tenantService.getBaseName(entity.getNodeRef()), post.getTopic(), entity.getName(), null); replies.add(wrap(replyPost, idToReplies, levels-1)); } } diff --git a/source/java/org/alfresco/repo/discussion/DiscussionServiceImplTest.java b/source/java/org/alfresco/repo/discussion/DiscussionServiceImplTest.java index a92dd017cc..434b35e019 100644 --- a/source/java/org/alfresco/repo/discussion/DiscussionServiceImplTest.java +++ b/source/java/org/alfresco/repo/discussion/DiscussionServiceImplTest.java @@ -94,16 +94,13 @@ public class DiscussionServiceImplTest private static TaggingService TAGGING_SERVICE; private static TenantAdminService TENANT_ADMIN_SERVICE; - private static final String TEST_USER = DiscussionServiceImplTest.class.getSimpleName() + "_testuser"; - private static final String ADMIN_USER = AuthenticationUtil.getAdminUserName(); - private static final String TENANT_DOMAIN = (DiscussionServiceImplTest.class.getSimpleName() + "Tenant").toLowerCase(); - private static final String TENANT_ADMIN_USER = ADMIN_USER + "@" + TENANT_DOMAIN; - private static final String TENANT_TEST_USER = TEST_USER + "@" + TENANT_DOMAIN; + + private static final String TEST_USER = DiscussionServiceImplTest.class.getSimpleName() + "_testuser@" + TENANT_DOMAIN; + private static final String ADMIN_USER = AuthenticationUtil.getAdminUserName() + "@" + TENANT_DOMAIN; private static SiteInfo DISCUSSION_SITE; private static SiteInfo ALTERNATE_DISCUSSION_SITE; - private static SiteInfo TENANT_DISCUSSION_SITE; private static NodeRef FORUM_NODE; /** @@ -130,8 +127,8 @@ public class DiscussionServiceImplTest TAGGING_SERVICE = (TaggingService)testContext.getBean("TaggingService"); TENANT_ADMIN_SERVICE = testContext.getBean("tenantAdminService", TenantAdminService.class); - createTenantAndTestSites(); - + createTenant(); + // Do the setup as admin AuthenticationUtil.setFullyAuthenticatedUser(ADMIN_USER); createUser(TEST_USER); @@ -391,99 +388,7 @@ public class DiscussionServiceImplTest } } - @Test - public void tenantCreateNewTopicAndPostAndReply() throws Exception - { - TenantUtil.runAsUserTenant(new TenantRunAsWork(){ - @Override - public Object doWork() throws Exception - { - TopicInfo siteTopic; - PostInfo post; - PostInfo reply1; - - // Nothing to start with - PagingResults results = DISCUSSION_SERVICE.listTopics(TENANT_DISCUSSION_SITE.getShortName(), true, new PagingRequest(10)); - assertEquals(0, results.getPage().size()); - - // Create two topics, one node and one site based - siteTopic = DISCUSSION_SERVICE.createTopic(TENANT_DISCUSSION_SITE.getShortName(), - "Site Title"); - - testNodesToTidy.add(siteTopic.getNodeRef()); - - // There are no posts to start with - PagingResults posts = DISCUSSION_SERVICE.listPosts(siteTopic, new PagingRequest(10)); - assertEquals(0, posts.getPage().size()); - - // The topic has no primary post - assertEquals(null, DISCUSSION_SERVICE.getPrimaryPost(siteTopic)); - // Which means no recent post - assertEquals(null, DISCUSSION_SERVICE.getMostRecentPost(siteTopic)); - - // Create the first post - String contents = "This Is Some Content"; - post = DISCUSSION_SERVICE.createPost(siteTopic, contents); - - // Ensure it got a NodeRef, a Name and the Topic - assertNotNull(post.getNodeRef()); - assertNotNull(post.getSystemName()); - assertEquals(siteTopic, post.getTopic()); - - // Ensure it shares a name with the topic - assertEquals(siteTopic.getSystemName(), post.getSystemName()); - - // As this is the primary post, it'll share the topic title - assertEquals(siteTopic.getTitle(), post.getTitle()); - - // It will have contents and a creator - assertEquals(contents, post.getContents()); - - // Fetch and check - post = DISCUSSION_SERVICE.getPost(siteTopic, post.getSystemName()); - assertNotNull(post.getNodeRef()); - assertNotNull(post.getSystemName()); - assertEquals(siteTopic, post.getTopic()); - assertEquals(siteTopic.getTitle(), post.getTitle()); - assertEquals(contents, post.getContents()); - - // Topic will now have a primary post - assertNotNull(DISCUSSION_SERVICE.getPrimaryPost(siteTopic)); - assertEquals(post.getNodeRef(), DISCUSSION_SERVICE.getPrimaryPost(siteTopic).getNodeRef()); - - // Topic will now have one post listed - posts = DISCUSSION_SERVICE.listPosts(siteTopic, new PagingRequest(10)); - assertEquals(1, posts.getPage().size()); - - // Add a reply - String reply1Contents = "Reply Contents"; - reply1 = DISCUSSION_SERVICE.createReply(post, reply1Contents); - assertNotNull(reply1.getNodeRef()); - assertNotNull(reply1.getSystemName()); - assertEquals(siteTopic, reply1.getTopic()); - assertEquals(null, reply1.getTitle()); // No title by default for - // replies - assertEquals(reply1Contents, reply1.getContents()); - - // Fetch and check - PostWithReplies postWithReplies = DISCUSSION_SERVICE.listPostReplies(siteTopic, 1); - assertNotNull(postWithReplies); - assertEquals(1, postWithReplies.getReplies().size()); - PostInfo reply2 = postWithReplies.getReplies().get(0).getPost(); - assertNotNull(reply2.getNodeRef()); - assertNotNull(reply2.getSystemName()); - assertEquals(siteTopic, reply2.getTopic()); - assertEquals(null, reply2.getTitle()); // No title by default for - // replies - assertEquals(reply1Contents, reply1.getContents()); - return null; - } - - }, TENANT_TEST_USER, TENANT_DOMAIN); - } - - @Test public void createUpdateDeleteEntries() throws Exception { TopicInfo siteTopic; @@ -1951,53 +1856,6 @@ public class DiscussionServiceImplTest AuthenticationUtil.setFullyAuthenticatedUser(TEST_USER); } - private static void createTenantAndTestSites() - { - AuthenticationUtil.runAs(new RunAsWork(){ - - @Override - public Object doWork() throws Exception - { - createTenant(); - return null; - } - - }, ADMIN_USER); - - createTenantUser(); - - TenantUtil.runAsUserTenant(new TenantRunAsWork() { - - @Override - public Object doWork() throws Exception - { - createTenantTestSites(); - return null; - } - }, TENANT_TEST_USER, TENANT_DOMAIN); - } - - private static void createTenantTestSites() throws Exception - { - final DiscussionServiceImpl privateDiscussionService = (DiscussionServiceImpl)testContext.getBean("discussionService"); - - TENANT_DISCUSSION_SITE = TRANSACTION_HELPER.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback() - { - @Override - public SiteInfo execute() throws Throwable - { - SiteInfo site = SITE_SERVICE.createSite( - TEST_SITE_PREFIX, - DiscussionServiceImplTest.class.getSimpleName() + "_testSite" + System.currentTimeMillis(), - "test site title", "test site description", - SiteVisibility.PUBLIC); - privateDiscussionService.getSiteDiscussionsContainer(site.getShortName(), true); - CLASS_TEST_NODES_TO_TIDY.add(site.getNodeRef()); - return site; - } - }); - } - private static void createTenant() { @@ -2015,20 +1873,6 @@ public class DiscussionServiceImplTest }); } - private static void createTenantUser() - { - TenantUtil.runAsUserTenant(new TenantRunAsWork(){ - - @Override - public Object doWork() throws Exception - { - createUser(TENANT_TEST_USER); - return null; - } - - }, TENANT_ADMIN_USER, TENANT_DOMAIN); - } - /** * By default, all tests are run as the admin user. */ @@ -2046,7 +1890,6 @@ public class DiscussionServiceImplTest { performDeletionOfNodes(CLASS_TEST_NODES_TO_TIDY); deleteUser(TEST_USER); - deleteUser(TENANT_TEST_USER); deleteTenant(); }