From 30084dde24f185c9f19680ba447a9651bbd53040 Mon Sep 17 00:00:00 2001 From: Neil McErlean Date: Tue, 4 May 2010 15:31:23 +0000 Subject: [PATCH] Merge from V3.3 to HEAD 20069 Fix for unreported issue where the Rendition Service's JavaScript API cannot be used to execute ad hoc rendition definitions. Rendition Definitions can be created and executed programmatically. This works fine via the Java Foundation API, but is broken in the JavaScript API. The rendition nodes are not correctly created for ad hoc rendition definitions created in JavaScript. Note that the built-in rendition definitions work fine in JavaScript. This fix: - ScriptRenditionDefinition now extends ScriptAction in order to reuse its parameter handling and execution code. - ScriptRenditionDefinition now mimics ScriptAction so you can call renditionDef.execute(testSourceNode); in JavaScript. - more test coverage in the test_renditionService.js to reproduce the issue. - adds debug logging in various places in the rendition service. - changes ScriptAction to be a non-final class so that it can be extended by ScriptRenditionDefinition. - fixes some fragile asserts in test code. - changes a few fields to protected visibility and provides an extension point so that the rendition service can execute its "actions" as renditions rather than simple actions. - trivial. tidied up some unused imports in ScriptNode. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@20070 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../alfresco/repo/jscript/ScriptAction.java | 17 ++++--- .../org/alfresco/repo/jscript/ScriptNode.java | 9 ++-- .../PerformRenditionActionExecuter.java | 29 ++++++++++-- .../rendition/RenditionDefinitionImpl.java | 7 ++- .../executer/AbstractRenderingEngine.java | 14 +++++- .../script/ScriptRenditionDefinition.java | 46 +++++++++---------- .../script/ScriptRenditionService.java | 34 ++++++++++++-- .../rendition/script/test_renditionService.js | 31 +++++++++++-- 8 files changed, 137 insertions(+), 50 deletions(-) diff --git a/source/java/org/alfresco/repo/jscript/ScriptAction.java b/source/java/org/alfresco/repo/jscript/ScriptAction.java index 6064aea6b9..f3e357efd4 100644 --- a/source/java/org/alfresco/repo/jscript/ScriptAction.java +++ b/source/java/org/alfresco/repo/jscript/ScriptAction.java @@ -41,7 +41,7 @@ import org.mozilla.javascript.Wrapper; * * @author davidc */ -public final class ScriptAction implements Serializable, Scopeable +public class ScriptAction implements Serializable, Scopeable { private static final long serialVersionUID = 5794161358406531996L; @@ -52,11 +52,11 @@ public final class ScriptAction implements Serializable, Scopeable private ActionValueConverter converter; /** Action state */ - private Action action; + protected Action action; - private ActionDefinition actionDef; + protected ActionDefinition actionDef; - private ServiceRegistry services; + protected ServiceRegistry services; private ActionService actionService; private NamespaceService namespaceService; private TransactionService transactionService; @@ -145,7 +145,7 @@ public final class ScriptAction implements Serializable, Scopeable actionParams.put(name, value); } } - actionService.executeAction(action, node.getNodeRef()); + executeImpl(node); // Parameters may have been updated by action execution, so reset cache this.parameters = null; @@ -153,6 +153,11 @@ public final class ScriptAction implements Serializable, Scopeable // Reset the actioned upon node node.reset(); } + + protected void executeImpl(ScriptNode node) + { + actionService.executeAction(action, node.getNodeRef()); + } /** * Execute action, optionally starting a new, potentially read-only transaction. @@ -184,7 +189,7 @@ public final class ScriptAction implements Serializable, Scopeable { public Object execute() throws Throwable { - actionService.executeAction(action, node.getNodeRef()); + executeImpl(node); return null; } }; diff --git a/source/java/org/alfresco/repo/jscript/ScriptNode.java b/source/java/org/alfresco/repo/jscript/ScriptNode.java index 5774e315da..3d043d7318 100644 --- a/source/java/org/alfresco/repo/jscript/ScriptNode.java +++ b/source/java/org/alfresco/repo/jscript/ScriptNode.java @@ -28,7 +28,6 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -54,10 +53,8 @@ import org.alfresco.repo.workflow.jscript.JscriptWorkflowInstance; import org.alfresco.scripts.ScriptException; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.action.Action; -import org.alfresco.service.cmr.dictionary.ClassDefinition; import org.alfresco.service.cmr.dictionary.DataTypeDefinition; import org.alfresco.service.cmr.dictionary.DictionaryService; -import org.alfresco.service.cmr.dictionary.PropertyDefinition; import org.alfresco.service.cmr.lock.LockStatus; import org.alfresco.service.cmr.model.FileInfo; import org.alfresco.service.cmr.model.FileNotFoundException; @@ -86,10 +83,7 @@ import org.alfresco.service.namespace.NamespacePrefixResolverProvider; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; -import org.springframework.extensions.surf.util.Content; import org.alfresco.util.GUID; -import org.springframework.extensions.surf.util.ParameterCheck; -import org.springframework.extensions.surf.util.URLEncoder; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.json.JSONException; @@ -99,6 +93,9 @@ import org.mozilla.javascript.Scriptable; import org.mozilla.javascript.ScriptableObject; import org.mozilla.javascript.UniqueTag; import org.mozilla.javascript.Wrapper; +import org.springframework.extensions.surf.util.Content; +import org.springframework.extensions.surf.util.ParameterCheck; +import org.springframework.extensions.surf.util.URLEncoder; /** * Script Node class implementation, specific for use by ScriptService as part of the object model. diff --git a/source/java/org/alfresco/repo/rendition/PerformRenditionActionExecuter.java b/source/java/org/alfresco/repo/rendition/PerformRenditionActionExecuter.java index f4d47679b6..90307b4677 100644 --- a/source/java/org/alfresco/repo/rendition/PerformRenditionActionExecuter.java +++ b/source/java/org/alfresco/repo/rendition/PerformRenditionActionExecuter.java @@ -162,6 +162,18 @@ public class PerformRenditionActionExecuter extends ActionExecuterAbstractBase StringBuilder msg = new StringBuilder(); msg.append("Rendering node ").append(actionedUponNodeRef).append(" with rendition definition ").append( renditionDefinition.getRenditionName()); + msg.append("\n").append(" parameters:").append("\n"); + if (renditionDefinition.getParameterValues().isEmpty() == false) + { + for (String paramKey : renditionDefinition.getParameterValues().keySet()) + { + msg.append(" ").append(paramKey).append("=").append(renditionDefinition.getParameterValue(paramKey)).append("\n"); + } + } + else + { + msg.append(" [None]"); + } log.debug(msg.toString()); } @@ -331,10 +343,19 @@ public class PerformRenditionActionExecuter extends ActionExecuterAbstractBase { QName renditionName=renditionDefinition.getRenditionName(); ChildAssociationRef renditionAssoc = renditionService.getRenditionByName(sourceNode, renditionName); - if(renditionAssoc ==null) - return null; - else - return renditionAssoc.getChildRef(); + + NodeRef result = (renditionAssoc == null) ? null : renditionAssoc.getChildRef(); + if (log.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Existing rendition with name ") + .append(renditionName) + .append(": ") + .append(result); + log.debug(msg.toString()); + } + + return result; } private void manageRenditionAspects(NodeRef sourceNode, ChildAssociationRef renditionParentAssoc) diff --git a/source/java/org/alfresco/repo/rendition/RenditionDefinitionImpl.java b/source/java/org/alfresco/repo/rendition/RenditionDefinitionImpl.java index bab894e564..435117d872 100644 --- a/source/java/org/alfresco/repo/rendition/RenditionDefinitionImpl.java +++ b/source/java/org/alfresco/repo/rendition/RenditionDefinitionImpl.java @@ -19,6 +19,8 @@ package org.alfresco.repo.rendition; +import java.io.Serializable; + import org.alfresco.repo.action.ActionImpl; import org.alfresco.service.cmr.action.Action; import org.alfresco.service.cmr.rendition.RenderCallback; @@ -38,7 +40,7 @@ public class RenditionDefinitionImpl extends ActionImpl implements RenditionDefi */ private static final long serialVersionUID = 4336392868488634875L; - protected static final String RENDITION_DEFINITION_NAME = "renderingActionName"; + public static final String RENDITION_DEFINITION_NAME = "renderingActionName"; public NodeRef renditionParent; public QName renditionAssociationType; @@ -74,7 +76,8 @@ public class RenditionDefinitionImpl extends ActionImpl implements RenditionDefi */ public QName getRenditionName() { - return (QName) getParameterValue(RENDITION_DEFINITION_NAME); + Serializable parameterValue = getParameterValue(RENDITION_DEFINITION_NAME); + return (QName) parameterValue; } /* diff --git a/source/java/org/alfresco/repo/rendition/executer/AbstractRenderingEngine.java b/source/java/org/alfresco/repo/rendition/executer/AbstractRenderingEngine.java index 1dc7e1e7da..8f2ae9500c 100644 --- a/source/java/org/alfresco/repo/rendition/executer/AbstractRenderingEngine.java +++ b/source/java/org/alfresco/repo/rendition/executer/AbstractRenderingEngine.java @@ -120,7 +120,6 @@ public abstract class AbstractRenderingEngine extends ActionExecuterAbstractBase protected NodeService nodeService; /* Parameter names common to all Rendering Actions */ - //TODO javadoc these /** * This optional {@link String} parameter specifies the location of a * classpath resource which can be used as a placeholder while a rendition @@ -297,6 +296,15 @@ public abstract class AbstractRenderingEngine extends ActionExecuterAbstractBase @Override protected void executeImpl(Action action, NodeRef sourceNode) { + if (logger.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Executing rendering engine; name:") + .append(this.name).append(", class:") + .append(this.getClass().getName()); + logger.debug(msg.toString()); + } + checkParameterValues(action); RenditionDefinition renditionDefinition = checkActionIsRenditionDefinition(action); checkSourceNodeExists(sourceNode); @@ -413,6 +421,10 @@ public abstract class AbstractRenderingEngine extends ActionExecuterAbstractBase protected Collection getParameterDefinitions() { List paramList = new ArrayList(); + + paramList.add(new ParameterDefinitionImpl(RenditionDefinitionImpl.RENDITION_DEFINITION_NAME, DataTypeDefinition.QNAME, true, + getParamDisplayLabel(RenditionDefinitionImpl.RENDITION_DEFINITION_NAME))); + paramList.add(new ParameterDefinitionImpl(PARAM_RUN_AS, DataTypeDefinition.TEXT, false, getParamDisplayLabel(PARAM_RUN_AS))); diff --git a/source/java/org/alfresco/repo/rendition/script/ScriptRenditionDefinition.java b/source/java/org/alfresco/repo/rendition/script/ScriptRenditionDefinition.java index ab41055d0e..ec55c1a2e7 100644 --- a/source/java/org/alfresco/repo/rendition/script/ScriptRenditionDefinition.java +++ b/source/java/org/alfresco/repo/rendition/script/ScriptRenditionDefinition.java @@ -18,10 +18,8 @@ */ package org.alfresco.repo.rendition.script; -import java.io.Serializable; -import java.util.Map; - import org.alfresco.repo.jscript.ScriptAction; +import org.alfresco.repo.jscript.ScriptNode; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.rendition.RenderingEngineDefinition; import org.alfresco.service.cmr.rendition.RenditionDefinition; @@ -35,23 +33,14 @@ import org.mozilla.javascript.Scriptable; * @author Neil McErlean * @see org.alfresco.service.cmr.rendition.RenditionDefinition */ -public final class ScriptRenditionDefinition implements Serializable +public final class ScriptRenditionDefinition extends ScriptAction { private static final long serialVersionUID = 8132935577891455490L; - private final RenderingEngineDefinition engineDefinition; - private final RenditionDefinition renditionDefinition; - private final ServiceRegistry serviceRegistry; - - private final ScriptAction scriptAction; public ScriptRenditionDefinition(ServiceRegistry serviceRegistry, Scriptable scope, RenderingEngineDefinition engineDefinition, RenditionDefinition renditionDefinition) { - this.serviceRegistry = serviceRegistry; - this.engineDefinition = engineDefinition; - this.renditionDefinition = renditionDefinition; - - this.scriptAction = new ScriptAction(serviceRegistry, renditionDefinition, engineDefinition); + super(serviceRegistry, renditionDefinition, engineDefinition); } /** @@ -61,18 +50,17 @@ public final class ScriptRenditionDefinition implements Serializable */ public String getRenditionName() { - QName qname = this.renditionDefinition.getRenditionName(); - return qname.toPrefixString(serviceRegistry.getNamespaceService()); + QName qname = getRenditionDefinition().getRenditionName(); + return qname.toPrefixString(services.getNamespaceService()); } + /** + * Returns the name of the Rendering Engine used by this definition. + * @return + */ public String getRenderingEngineName() { - return this.engineDefinition.getName(); - } - - public Map getParameters() - { - return this.scriptAction.getParameters(); + return getRenderingEngineDefinition().getName(); } @Override @@ -87,6 +75,18 @@ public final class ScriptRenditionDefinition implements Serializable RenditionDefinition getRenditionDefinition() { - return this.renditionDefinition; + return (RenditionDefinition)action; + } + + RenderingEngineDefinition getRenderingEngineDefinition() + { + return (RenderingEngineDefinition)actionDef; + } + + @Override + protected void executeImpl(ScriptNode node) + { + RenditionDefinition renditionDefinition = getRenditionDefinition(); + this.services.getRenditionService().render(node.getNodeRef(), renditionDefinition); } } diff --git a/source/java/org/alfresco/repo/rendition/script/ScriptRenditionService.java b/source/java/org/alfresco/repo/rendition/script/ScriptRenditionService.java index fbb1520a98..04dfa42bcf 100644 --- a/source/java/org/alfresco/repo/rendition/script/ScriptRenditionService.java +++ b/source/java/org/alfresco/repo/rendition/script/ScriptRenditionService.java @@ -31,8 +31,6 @@ import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.mozilla.javascript.Context; -import org.mozilla.javascript.Scriptable; /** * Script object representing the rendition service. @@ -78,7 +76,16 @@ public class ScriptRenditionService extends BaseScopableProcessorExtension */ public ScriptRenditionDefinition createRenditionDefinition(String renditionName, String renderingEngineName) { - QName renditionQName = createQName(renditionName); + QName renditionQName = createQName(renditionName); + + if (logger.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Creating ScriptRenditionDefinition [") + .append(renditionName).append(", ") + .append(renderingEngineName).append("]"); + logger.debug(msg.toString()); + } RenderingEngineDefinition engineDefinition = renditionService.getRenderingEngineDefinition(renderingEngineName); RenditionDefinition rendDef = renditionService.createRenditionDefinition(renditionQName, renderingEngineName); @@ -123,9 +130,28 @@ public class ScriptRenditionService extends BaseScopableProcessorExtension public ScriptNode render(ScriptNode sourceNode, ScriptRenditionDefinition renditionDefQName) { + if (logger.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Rendering source node '") + .append(sourceNode) + .append("' with renditionDefQName '").append(renditionDefQName) + .append("'"); + logger.debug(msg.toString()); + } + ChildAssociationRef chAssRef = this.renditionService.render(sourceNode.getNodeRef(), renditionDefQName.getRenditionDefinition()); - return new ScriptNode(chAssRef.getChildRef(), serviceRegistry); + + NodeRef renditionNode = chAssRef.getChildRef(); + if (logger.isDebugEnabled()) + { + StringBuilder msg = new StringBuilder(); + msg.append("Rendition: ").append(renditionNode); + logger.debug(msg.toString()); + } + + return new ScriptNode(renditionNode, serviceRegistry); } diff --git a/source/java/org/alfresco/repo/rendition/script/test_renditionService.js b/source/java/org/alfresco/repo/rendition/script/test_renditionService.js index 4ef1bf92ad..74236b8bf1 100644 --- a/source/java/org/alfresco/repo/rendition/script/test_renditionService.js +++ b/source/java/org/alfresco/repo/rendition/script/test_renditionService.js @@ -47,6 +47,7 @@ function testCreateRenditionDefinitionAndRender() // As long as we don't save this renditionDefinition, there should be no need to // give it a name which is unique across multiple test executions. var renditionDefName = "cm:adHocRenditionDef"; + var renditionDefNameLong = "{http://www.alfresco.org/model/content/1.0}adHocRenditionDef"; var renderingEngineName = "imageRenderingEngine"; var renditionDef = renditionService.createRenditionDefinition(renditionDefName, renderingEngineName); @@ -55,20 +56,42 @@ function testCreateRenditionDefinitionAndRender() test.assertEquals(renditionDefName, renditionDef.renditionName); test.assertEquals(renderingEngineName, renditionDef.renderingEngineName); - // Set some parameters. - renditionDef.parameters['rendition-nodetype'] = "cm:content"; - renditionDef.parameters['xsize'] = 99; + renditionDef.parameters["rendition-nodetype"] = "cm:content"; + renditionDef.parameters["xsize"] = 99; // Read them back to check + test.assertNotNull(renditionDef.parameters, "renditionDef.parameters was null"); test.assertEquals("cm:content", renditionDef.parameters['rendition-nodetype']); test.assertEquals(99, renditionDef.parameters['xsize']); // Now execute this rendition definition - var rendition = renditionService.render(testSourceNode, renditionDef); + renditionDef.execute(testSourceNode); + + // Alternate, equivalent call: + // renditionService.render(testSourceNode, renditionDef); + + var children = testSourceNode.children; + + // Find the child that is the ad hoc rendition + var rendition; + for (var i = 0; i < children.length; i++) + { + var nextChild = children[i]; + var assocName = nextChild.primaryParentAssoc.getQName(); + if (assocName == renditionDefNameLong) + { + rendition = nextChild; + } + } test.assertNotNull(rendition, "rendition was null."); + test.assertTrue(rendition.hasAspect("rn:hiddenRendition")); + test.assertEquals("{http://www.alfresco.org/model/content/1.0}content", rendition.type); + + test.assertNotNull(rendition.mimetype, "rendition mimetype was null."); + test.assertEquals("image/png", rendition.mimetype); } // Execute tests