From 5e3ca9df9eb57c70de81c19e66c2379acd81108c Mon Sep 17 00:00:00 2001 From: alandavis Date: Wed, 19 Dec 2018 16:36:32 +0000 Subject: [PATCH] REPO-4114 POST /nodes/{nodeId}/children api does not use Transform Service for renditions (#114) In addition to using the Transform service for renditions when uploading content via the v1 REST api, this PR also includes changes to allow a client to request multiple renditions when uploading content or to request multiple renditions in a single request. This is now possible as the newer RenditionService2 is async. --- .../org/alfresco/rest/api/Renditions.java | 16 +++ .../org/alfresco/rest/api/impl/NodesImpl.java | 99 +++++++-------- .../rest/api/impl/RenditionsImpl.java | 115 +++++++++++++++++- .../api/nodes/NodeRenditionsRelation.java | 12 +- .../rest/api/tests/RenditionsTest.java | 82 ++++++++----- 5 files changed, 223 insertions(+), 101 deletions(-) diff --git a/src/main/java/org/alfresco/rest/api/Renditions.java b/src/main/java/org/alfresco/rest/api/Renditions.java index 3a92e5e809..c281bc9162 100644 --- a/src/main/java/org/alfresco/rest/api/Renditions.java +++ b/src/main/java/org/alfresco/rest/api/Renditions.java @@ -27,11 +27,15 @@ package org.alfresco.rest.api; import org.alfresco.rest.api.model.Rendition; +import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException; +import org.alfresco.rest.framework.core.exceptions.NotFoundException; import org.alfresco.rest.framework.resource.content.BinaryResource; import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo; import org.alfresco.rest.framework.resource.parameters.Parameters; import org.alfresco.service.cmr.repository.NodeRef; +import java.util.List; + /** * Renditions API * @@ -80,6 +84,18 @@ public interface Renditions */ void createRendition(NodeRef nodeRef, Rendition rendition, boolean executeAsync, Parameters parameters); + /** + * Creates renditions that don't already exist for the given node asynchronously. + * + * @param nodeRef + * @param renditions the {@link Rendition} request + * @param parameters the {@link Parameters} object to get the parameters passed into the request + * @throws NotFoundException if any of the rendition id do not exist. + * @throws ConstraintViolatedException if all of the renditions already exist. + */ + void createRenditions(NodeRef nodeRef, List renditions, Parameters parameters) + throws NotFoundException, ConstraintViolatedException; + /** * Downloads rendition. * diff --git a/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java b/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java index e165dadec0..5673563852 100644 --- a/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java +++ b/src/main/java/org/alfresco/rest/api/impl/NodesImpl.java @@ -64,6 +64,9 @@ import org.alfresco.repo.node.getchildren.FilterPropBoolean; import org.alfresco.repo.node.getchildren.GetChildrenCannedQuery; import org.alfresco.repo.node.integrity.IntegrityException; import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.repo.rendition2.RenditionDefinition2; +import org.alfresco.repo.rendition2.RenditionDefinitionRegistry2; +import org.alfresco.repo.rendition2.RenditionService2; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.security.permissions.AccessDeniedException; @@ -92,7 +95,6 @@ import org.alfresco.rest.api.model.PathInfo.ElementInfo; import org.alfresco.rest.api.model.QuickShareLink; import org.alfresco.rest.api.model.UserInfo; import org.alfresco.rest.api.nodes.NodeAssocService; -import org.alfresco.rest.framework.core.exceptions.ApiException; import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException; import org.alfresco.rest.framework.core.exceptions.DisabledServiceException; import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException; @@ -101,6 +103,7 @@ import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException; import org.alfresco.rest.framework.core.exceptions.NotFoundException; import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException; import org.alfresco.rest.framework.core.exceptions.RequestEntityTooLargeException; +import org.alfresco.rest.framework.core.exceptions.StaleEntityException; import org.alfresco.rest.framework.core.exceptions.UnsupportedMediaTypeException; import org.alfresco.rest.framework.resource.content.BasicContentInfo; import org.alfresco.rest.framework.resource.content.BinaryResource; @@ -207,6 +210,7 @@ public class NodesImpl implements Nodes private OwnableService ownableService; private AuthorityService authorityService; private ThumbnailService thumbnailService; + private RenditionService2 renditionService2; private SiteService siteService; private ActivityPoster poster; private RetryingTransactionHelper retryingTransactionHelper; @@ -261,6 +265,7 @@ public class NodesImpl implements Nodes this.ownableService = sr.getOwnableService(); this.authorityService = sr.getAuthorityService(); this.thumbnailService = sr.getThumbnailService(); + this.renditionService2 = sr.getRenditionService2(); this.siteService = sr.getSiteService(); this.retryingTransactionHelper = sr.getRetryingTransactionHelper(); this.lockService = sr.getLockService(); @@ -2991,21 +2996,8 @@ public class NodesImpl implements Nodes // Create the response final Node fileNode = getFolderOrDocumentFullInfo(nodeRef, parentNodeRef, nodeTypeQName, parameters); - // RA-1052 - try - { - List thumbnailDefs = getThumbnailDefs(renditions); - requestRenditions(thumbnailDefs, fileNode); - } - catch (Exception ex) - { - // Note: The log level is not 'error' as it could easily fill out the log file, especially in the Cloud. - if (logger.isDebugEnabled()) - { - // Don't throw the exception as we don't want the the upload to fail, just log it. - logger.debug("Asynchronous request to create a rendition upon upload failed: " + ex.getMessage()); - } - } + checkRenditionNames(renditions); + requestRenditions(renditions, fileNode); return fileNode; @@ -3073,38 +3065,28 @@ public class NodesImpl implements Nodes return null; } - private List getThumbnailDefs(Set renditionNames) + private void checkRenditionNames(Set renditionNames) { - List thumbnailDefs = null; - if (renditionNames != null) { - // If thumbnail generation has been configured off, then don't bother. - if (!thumbnailService.getThumbnailsEnabled()) + if (!renditionService2.isEnabled()) { throw new DisabledServiceException("Thumbnail generation has been disabled."); } - thumbnailDefs = new ArrayList<>(renditionNames.size()); - ThumbnailRegistry registry = thumbnailService.getThumbnailRegistry(); + RenditionDefinitionRegistry2 renditionDefinitionRegistry2 = renditionService2.getRenditionDefinitionRegistry2(); for (String renditionName : renditionNames) { - // Use the thumbnail registry to get the details of the thumbnail - ThumbnailDefinition thumbnailDef = registry.getThumbnailDefinition(renditionName); - if (thumbnailDef == null) + RenditionDefinition2 renditionDefinition = renditionDefinitionRegistry2.getRenditionDefinition(renditionName); + if (renditionDefinition == null) { throw new NotFoundException(renditionName + " is not registered."); } - - thumbnailDefs.add(thumbnailDef); - } } - - return thumbnailDefs; } - private Set getRequestedRenditions(String renditionsParam) + static Set getRequestedRenditions(String renditionsParam) { if (renditionsParam == null) { @@ -3113,13 +3095,6 @@ public class NodesImpl implements Nodes String[] renditionNames = renditionsParam.split(","); - // Temporary - pending future improvements to thumbnail service to minimise chance of - // missing/failed thumbnails (when requested/generated 'concurrently') - if (renditionNames.length > 1) - { - throw new InvalidArgumentException("Please specify one rendition entity id only"); - } - Set renditions = new LinkedHashSet<>(renditionNames.length); for (String name : renditionNames) { @@ -3132,28 +3107,39 @@ public class NodesImpl implements Nodes return renditions; } - private void requestRenditions(List thumbnailDefs, Node fileNode) + private void requestRenditions(Set renditionNames, Node fileNode) { - if (thumbnailDefs != null) + if (renditionNames != null) { - ThumbnailRegistry registry = thumbnailService.getThumbnailRegistry(); - for (ThumbnailDefinition thumbnailDef : thumbnailDefs) + NodeRef sourceNodeRef = fileNode.getNodeRef(); + String mimeType = fileNode.getContent().getMimeType(); + long size = fileNode.getContent().getSizeInBytes(); + RenditionDefinitionRegistry2 renditionDefinitionRegistry2 = renditionService2.getRenditionDefinitionRegistry2(); + Set availableRenditions = renditionDefinitionRegistry2.getRenditionNamesFrom(mimeType, size); + + for (String renditionName : renditionNames) { - NodeRef sourceNodeRef = fileNode.getNodeRef(); - String mimeType = fileNode.getContent().getMimeType(); - long size = fileNode.getContent().getSizeInBytes(); - - // Check if anything is currently available to generate thumbnails for the specified mimeType - if (! registry.isThumbnailDefinitionAvailable(null, mimeType, size, sourceNodeRef, thumbnailDef)) + // RA-1052 (REPO-47) + try { - throw new InvalidArgumentException("Unable to create thumbnail '" + thumbnailDef.getName() + "' for " + - mimeType + " as no transformer is currently available."); + // File may be to big + if (!availableRenditions.contains(renditionName)) + { + throw new InvalidArgumentException("Unable to create thumbnail '" + renditionName + "' for " + + mimeType + " as no transformer is currently available."); + } + + renditionService2.render(sourceNodeRef, renditionName); + } + catch (Exception ex) + { + // Note: The log level is not 'error' as it could easily fill out the log file. + if (logger.isDebugEnabled()) + { + // Don't throw the exception as we don't want the the upload to fail, just log it. + logger.debug("Asynchronous request to create a rendition upon upload failed: " + ex.getMessage()); + } } - - Action action = ThumbnailHelper.createCreateThumbnailAction(thumbnailDef, sr); - - // Queue async creation of thumbnail - actionService.executeAction(action, sourceNodeRef, true, true); } } } @@ -3522,6 +3508,7 @@ public class NodesImpl implements Nodes return authorityService; } + @Deprecated protected ThumbnailService getThumbnailService() { return thumbnailService; diff --git a/src/main/java/org/alfresco/rest/api/impl/RenditionsImpl.java b/src/main/java/org/alfresco/rest/api/impl/RenditionsImpl.java index c92df940dc..af0a7f5c6c 100644 --- a/src/main/java/org/alfresco/rest/api/impl/RenditionsImpl.java +++ b/src/main/java/org/alfresco/rest/api/impl/RenditionsImpl.java @@ -76,11 +76,14 @@ import org.springframework.core.io.ResourceLoader; import java.io.File; import java.io.InputStream; import java.io.Serializable; +import java.util.ArrayList; import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.StringJoiner; import java.util.TreeMap; /** @@ -280,7 +283,7 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware final NodeRef renditionNodeRef = getRenditionByName(sourceNodeRef, rendition.getId(), parameters); if (renditionNodeRef != null) { - throw new ConstraintViolatedException(rendition.getId() + " rendition already exists."); + throw new ConstraintViolatedException(rendition.getId() + " rendition already exists."); // 409 } try @@ -301,6 +304,116 @@ public class RenditionsImpl implements Renditions, ResourceLoaderAware } } + @Override + public void createRenditions(NodeRef nodeRef, List renditions, Parameters parameters) + throws NotFoundException, ConstraintViolatedException + { + if (renditions.isEmpty()) + { + return; + } + + if (!renditionService2.isEnabled()) + { + throw new DisabledServiceException("Rendition generation has been disabled."); + } + + final NodeRef sourceNodeRef = validateNode(nodeRef.getStoreRef(), nodeRef.getId()); + RenditionDefinitionRegistry2 renditionDefinitionRegistry2 = renditionService2.getRenditionDefinitionRegistry2(); + + // So that POST /nodes/{nodeId}/renditions can specify rendition names as a comma separated list just like + // POST /nodes/{nodId}/children can specify a comma separated list, the following code checks to see if the + // supplied Rendition names are actually comma separated lists. The following example shows it is possible to + // use both approaches. + // [ + // { "id": "doclib" }, + // { "id": "avatar,avatar32" } + // ] + Set renditionNames = new HashSet<>(); + for (Rendition rendition : renditions) + { + String name = getName(rendition); + Set requestedRenditions = NodesImpl.getRequestedRenditions(name); + if (requestedRenditions == null) + { + renditionNames.add(null); + } + else + { + renditionNames.addAll(requestedRenditions); + } + } + + StringJoiner renditionNamesAlreadyExist = new StringJoiner(","); + StringJoiner renditionNamesNotRegistered = new StringJoiner(","); + List renditionNamesToCreate = new ArrayList<>(); + for (String renditionName : renditionNames) + { + System.err.println("renditionName="+renditionName); + if (renditionName == null) + { + throw new IllegalArgumentException(("Null rendition name supplied")); // 400 + } + + RenditionDefinition2 renditionDefinition = renditionDefinitionRegistry2.getRenditionDefinition(renditionName); + if (renditionDefinition == null) + { + renditionNamesNotRegistered.add(renditionName); + } + + final NodeRef renditionNodeRef = getRenditionByName(sourceNodeRef, renditionName, parameters); + if (renditionNodeRef == null) + { + renditionNamesToCreate.add(renditionName); + } + else + { + renditionNamesAlreadyExist.add(renditionName); + } + } + + if (renditionNamesNotRegistered.length() != 0) + { + throw new NotFoundException("Renditions not registered: " + renditionNamesNotRegistered); // 404 + } + + if (renditionNamesToCreate.size() == 0) + { + throw new ConstraintViolatedException("All renditions requested already exist: " + renditionNamesAlreadyExist); // 409 + } + + for (String renditionName : renditionNamesToCreate) + { + try + { + renditionService2.render(sourceNodeRef, renditionName); + } + catch (UnsupportedOperationException e) + { + throw new IllegalArgumentException((e.getMessage())); // 400 + } + catch (IllegalStateException e) + { + throw new StaleEntityException(e.getMessage()); // 409 + } + } + + } + + private String getName(Rendition rendition) + { + String renditionName = rendition.getId(); + if (renditionName != null) + { + renditionName = renditionName.trim(); + if (renditionName.isEmpty()) + { + renditionName = null; + } + } + return renditionName; + } + @Override public BinaryResource getContent(NodeRef nodeRef, String renditionId, Parameters parameters) { diff --git a/src/main/java/org/alfresco/rest/api/nodes/NodeRenditionsRelation.java b/src/main/java/org/alfresco/rest/api/nodes/NodeRenditionsRelation.java index 6079312d32..4879e90494 100644 --- a/src/main/java/org/alfresco/rest/api/nodes/NodeRenditionsRelation.java +++ b/src/main/java/org/alfresco/rest/api/nodes/NodeRenditionsRelation.java @@ -90,17 +90,7 @@ public class NodeRenditionsRelation implements RelationshipResourceAction.Read create(String nodeId, List entity, Parameters parameters) { NodeRef nodeRef = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, nodeId); - // Temporary - pending future improvements to thumbnail service to minimise chance of - // missing/failed thumbnails (when requested/generated 'concurrently') - if (entity.size() > 1) - { - throw new InvalidArgumentException("Please specify one rendition entity id only"); - } - - for (Rendition rendition : entity) - { - renditions.createRendition(nodeRef, rendition, parameters); - } + renditions.createRenditions(nodeRef, entity, parameters); return null; } diff --git a/src/test/java/org/alfresco/rest/api/tests/RenditionsTest.java b/src/test/java/org/alfresco/rest/api/tests/RenditionsTest.java index fb43dc6cf7..e00acfd653 100644 --- a/src/test/java/org/alfresco/rest/api/tests/RenditionsTest.java +++ b/src/test/java/org/alfresco/rest/api/tests/RenditionsTest.java @@ -323,8 +323,8 @@ public class RenditionsTest extends AbstractBaseApiTest // Create a node without any content. Test only if OpenOffice is available if(isOpenOfficeAvailable()) { - String emptyContentNodeId = addToDocumentLibrary(userOneN1Site, "emptyDoc.txt", TYPE_CM_CONTENT, userOneN1.getId()); - getSingle(getNodeRenditionsUrl(emptyContentNodeId), "doclib", 200); + String emptyContentNodeId = addToDocumentLibrary(userOneN1Site, "emptyDoc.txt", TYPE_CM_CONTENT, userOneN1.getId()); + getSingle(getNodeRenditionsUrl(emptyContentNodeId), "doclib", 200); } // Create multipart request @@ -464,14 +464,31 @@ public class RenditionsTest extends AbstractBaseApiTest // renditionId is empty post(getNodeRenditionsUrl(contentNodeId), toJsonAsString(new Rendition().setId("")), 400); - // -ve test - we do not currently accept multiple create entities + // Multiple rendition request (as of ACS 6.1) + + // Multiple rendition request, including an empty id List multipleRenditionRequest = new ArrayList<>(2); - multipleRenditionRequest.add(new Rendition().setId("doclib")); - multipleRenditionRequest.add(new Rendition().setId("imgpreview")); + multipleRenditionRequest.add(new Rendition().setId("")); + multipleRenditionRequest.add(new Rendition().setId("medium")); post(getNodeRenditionsUrl(contentNodeId), toJsonAsString(multipleRenditionRequest), 400); - RenditionService2Impl renditionService2 = applicationContext.getBean("renditionService2", RenditionService2Impl.class); + // Multiple rendition request. doclib has already been done + multipleRenditionRequest = new ArrayList<>(3); + multipleRenditionRequest.add(new Rendition().setId("doclib")); + multipleRenditionRequest.add(new Rendition().setId("medium")); + multipleRenditionRequest.add(new Rendition().setId("avatar,avatar32")); + post(getNodeRenditionsUrl(contentNodeId), toJsonAsString(multipleRenditionRequest), 202); + assertRenditionCreatedWithWait(contentNodeId, "doclib", "medium", "avatar", "avatar32"); + + // Multiple rendition request. All have already been done. + multipleRenditionRequest = new ArrayList<>(2); + multipleRenditionRequest.add(new Rendition().setId("doclib")); + multipleRenditionRequest.add(new Rendition().setId("medium")); + multipleRenditionRequest.add(new Rendition().setId("avatar")); + post(getNodeRenditionsUrl(contentNodeId), toJsonAsString(multipleRenditionRequest), 409); + // Disable thumbnail generation + RenditionService2Impl renditionService2 = applicationContext.getBean("renditionService2", RenditionService2Impl.class); renditionService2.setThumbnailsEnabled(false); try { @@ -541,24 +558,21 @@ public class RenditionsTest extends AbstractBaseApiTest if(isOpenOfficeAvailable()) { - // Create multipart request - Word doc file - renditionName = "doclib"; - fileName = "farmers_markets_list_2003.doc"; - file = getResourceFile(fileName); - reqBody = MultiPartBuilder.create() - .setFileData(new FileData(fileName, file)) - .setRenditions(Collections.singletonList(renditionName)) - .build(); + // Create multipart request - Word doc file + renditionName = "doclib"; + fileName = "farmers_markets_list_2003.doc"; + file = getResourceFile(fileName); + reqBody = MultiPartBuilder.create() + .setFileData(new FileData(fileName, file)) + .setRenditions(Collections.singletonList(renditionName)) + .build(); - // Upload file into 'folder' - including request to create 'doclib' thumbnail - response = post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 201); - document = RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class); - contentNodeId = document.getId(); + // Upload file into 'folder' - including request to create 'doclib' thumbnail + response = post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 201); + document = RestApiUtil.parseRestApiEntry(response.getJsonResponse(), Document.class); + contentNodeId = document.getId(); - // wait and check that rendition is created ... - rendition = waitAndGetRendition(contentNodeId, renditionName); - assertNotNull(rendition); - assertEquals(RenditionStatus.CREATED, rendition.getStatus()); + assertRenditionCreatedWithWait(contentNodeId, renditionName); } /* RA-834: commented-out since not currently applicable for empty file @@ -581,20 +595,13 @@ public class RenditionsTest extends AbstractBaseApiTest assertEquals(RenditionStatus.CREATED, rendition.getStatus()); */ - - /* - * Per RA-1052, the failure of the async request to create a rendition - * should NOT fail the upload. - */ - - // Currently we do not support multiple rendition requests on create + // Multiple renditions requested reqBody = MultiPartBuilder.create() .setFileData(new FileData(fileName, file)) .setAutoRename(true) .setRenditions(Arrays.asList(new String[]{"doclib,imgpreview"})) .build(); - - post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 400); + post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 201); // Unknown rendition reqBody = MultiPartBuilder.create() @@ -602,8 +609,7 @@ public class RenditionsTest extends AbstractBaseApiTest .setAutoRename(true) .setRenditions(Arrays.asList(new String[]{"unknown"})) .build(); - - post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 201); + post(getNodeChildrenUrl(folder_Id), reqBody.getBody(), null, reqBody.getContentType(), 404); // ThumbnailService is disabled ThumbnailService thumbnailService = applicationContext.getBean("thumbnailService", ThumbnailService.class); @@ -626,6 +632,16 @@ public class RenditionsTest extends AbstractBaseApiTest } } + protected void assertRenditionCreatedWithWait(String contentNodeId, String... renditionNames) throws Exception + { + for (String renditionName : renditionNames) + { + Rendition rendition = waitAndGetRendition(contentNodeId, renditionName); + assertNotNull(rendition); + assertEquals(RenditionStatus.CREATED, rendition.getStatus()); + } + } + /** * Tests create rendition after uploading new version(s) *