ALF-4713 - It is possible to transfer content to the not enabled transfer target

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@22452 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/messages/transfer-service.properties

@@ -9,6 +9,7 @@ transfer_service.comms.unsupported_protocol=Unsupported protocol: {0}
 transfer_service.comms.unsuccessful_response=Received unsuccessful response code from target server: {0}, {1}
 transfer_service.comms.http_request_failed=Failed to execute HTTP request {0} to target: {1} status: {2}
 transfer_service.no_nodes=No nodes to transfer
+transfer_service.target_not_enabled=Transfer target not enabled {0}
 transfer_service.cancelled=Transfer cancelled
 transfer_service.failed_to_get_transfer_status=Failed to retrieve transfer status from target {0}
 transfer_service.target_error=Transfer target failed with {0}

source/java/org/alfresco/repo/transfer/TransferServiceImpl2.java

@@ -123,6 +123,7 @@ public class TransferServiceImpl2 implements TransferService2
     private static final String MSG_FAILED_TO_GET_TRANSFER_STATUS = "transfer_service.failed_to_get_transfer_status";
     private static final String MSG_TARGET_ERROR = "transfer_service.target_error";
     private static final String MSG_UNKNOWN_TARGET_ERROR = "transfer_service.unknown_target_error";
+    private static final String MSG_TARGET_NOT_ENABLED = "transfer_service.target_not_enabled";
     
     private static final String FILE_DIRECTORY = "transfer";
     private static final String FILE_SUFFIX = ".xml";
@@ -574,6 +575,7 @@ public class TransferServiceImpl2 implements TransferService2
                         manifest = createManifest(definition);
                         logger.debug("transfer begin");
                         target = getTransferTarget(targetName);
+                        checkTargetEnabled(target);
                         transfer = transmitter.begin(target);
                         String transferId = transfer.getTransferId();
                         TransferStatus status = new TransferStatus();
@@ -662,6 +664,7 @@ public class TransferServiceImpl2 implements TransferService2
                         if (progress.getCurrentPosition() != pollPosition)
                         {
                             pollPosition = progress.getCurrentPosition();
+                            logger.debug("committing :" + pollPosition);
                             eventProcessor.committing(progress.getEndPosition(), pollPosition);
                         }
 
@@ -772,11 +775,14 @@ public class TransferServiceImpl2 implements TransferService2
 
                             try
                             {
-                                logger.debug("now pull back the destination transfer report");
+                                if(transfer != null)
-                                destinationReport = persistDestinationTransferReport(reportName, transfer, target);
-                                if (destinationReport != null)
                                 {
-                                    eventProcessor.writeReport(destinationReport, TransferEventReport.ReportType.DESTINATION, endEventImpl.getTransferState());
+                                    logger.debug("now pull back the destination transfer report");
+                                    destinationReport = persistDestinationTransferReport(reportName, transfer, target);
+                                    if (destinationReport != null)
+                                    {
+                                        eventProcessor.writeReport(destinationReport, TransferEventReport.ReportType.DESTINATION, endEventImpl.getTransferState());
+                                    }
                                 }
 
                                 logger.debug("now persist the client side transfer report");
@@ -1062,6 +1068,15 @@ public class TransferServiceImpl2 implements TransferService2
             }
         }
     }
+    
+    private void checkTargetEnabled(TransferTarget target) throws TransferException
+    {
+        if(!target.isEnabled())
+        {
+            logger.debug("target is not enabled");
+            throw new TransferException(MSG_TARGET_NOT_ENABLED, new Object[] {target.getName()});
+        }
+    }
 
     public void setNodeService(NodeService nodeService)
     {
@@ -1232,6 +1247,11 @@ public class TransferServiceImpl2 implements TransferService2
         {
             def.setEnabled((Boolean)properties.get(TransferModel.PROP_ENABLED));
         }
+        else
+        {
+            // If the enableable aspect is not present then we don't want transfer failing.
+            def.setEnabled(Boolean.TRUE);
+        }
     }
 
     /* (non-Javadoc)
@@ -1408,8 +1428,5 @@ public class TransferServiceImpl2 implements TransferService2
         boolean cancelMe = false;
         boolean cancelInProgress = false;
     }
-    
+   
-
-
-
 }

source/java/org/alfresco/repo/transfer/TransferServiceImplTest.java

@@ -623,6 +623,9 @@ public class TransferServiceImplTest extends BaseAlfrescoSpringTest
      * Step 6: Negative test : transfer no nodes
      * transfer (should throw exception)
      * 
+     * Step 7: Negative test : transfer to a disabled transfer target
+     * transfer (should throw exception)
+     * 
      * This is a unit test so it does some shenanigans to send to the same instance of alfresco.
      */
     public void testTransferOneNode() throws Exception
@@ -689,6 +692,7 @@ public class TransferServiceImplTest extends BaseAlfrescoSpringTest
             {
                 transferMe = transferService.getTransferTarget(targetName);
             }
+            transferService.enableTransferTarget(targetName, true);
         }
         finally
         {
@@ -905,7 +909,6 @@ public class TransferServiceImplTest extends BaseAlfrescoSpringTest
             endTransaction();
         }
 
-        
         /**
           * Step 6
           * Negative test transfer nothing
@@ -921,7 +924,28 @@ public class TransferServiceImplTest extends BaseAlfrescoSpringTest
         {
             // expect to go here
         }
-        
+       
+        /**
+         * Step 7: Negative test : transfer to a disabled transfer target
+         * transfer (should throw exception)
+         */ 
+        logger.debug("Transfer again - with no content - should throw exception");
+        try
+        {
+                transferService.enableTransferTarget(targetName, false);
+                TransferDefinition definition = new TransferDefinition();
+                
+                Set<NodeRef>nodes = new HashSet<NodeRef>();
+                nodes.add(deletedContentNodeRef);
+                definition.setNodes(nodes);
+                transferService.transfer(targetName, definition);
+                fail("target not enabled exception not thrown");
+        }
+        catch(TransferException te)
+        {
+           // expect to go here
+           assertTrue("check contents of exception message", te.getCause().getMessage().contains("enabled"));
+        }
      }
     
     /**