Missing commit broke tests. Tighten up code, #2129

dspace-api/src/main/java/org/dspace/app/requestitem/RequestItem.java

@@ -33,7 +33,6 @@ import org.dspace.core.ReloadableEntity;
 @Table(name = "requestitem")
 public class RequestItem implements ReloadableEntity<Integer> {
 
-
     @Id
     @Column(name = "requestitem_id")
     @GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "requestitem_seq")
@@ -54,8 +53,6 @@ public class RequestItem implements ReloadableEntity<Integer> {
     @Column(name = "request_name", length = 64)
     private String reqName;
 
-    //    @Column(name = "request_message")
-//    @Lob
     @Column(name = "request_message", columnDefinition = "text")
     private String reqMessage;
 
@@ -82,8 +79,8 @@ public class RequestItem implements ReloadableEntity<Integer> {
 
     /**
      * Protected constructor, create object using:
-     * {@link org.dspace.app.requestitem.service.RequestItemService#createRequest(Context, Bitstream, Item,
+     * {@link org.dspace.app.requestitem.service.RequestItemService#createRequest(
-     * boolean, String, String, String)}
+     * Context, Bitstream, Item, boolean, String, String, String)}
      */
     protected RequestItem() {
     }

dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemServiceImpl.java

@@ -9,6 +9,7 @@ package org.dspace.app.requestitem;
 
 import java.sql.SQLException;
 import java.util.Date;
+import java.util.List;
 
 import org.apache.logging.log4j.Logger;
 import org.dspace.app.requestitem.dao.RequestItemDAO;
@@ -16,19 +17,21 @@ import org.dspace.app.requestitem.service.RequestItemService;
 import org.dspace.content.Bitstream;
 import org.dspace.content.Item;
 import org.dspace.core.Context;
+import org.dspace.core.LogManager;
 import org.dspace.core.Utils;
 import org.springframework.beans.factory.annotation.Autowired;
 
 /**
  * Service implementation for the RequestItem object.
- * This class is responsible for all business logic calls for the RequestItem object and is autowired by Spring.
+ * This class is responsible for all business logic calls for the RequestItem
+ * object and is autowired by Spring.
  * This class should never be accessed directly.
  *
  * @author kevinvandevelde at atmire.com
  */
 public class RequestItemServiceImpl implements RequestItemService {
 
-    private final Logger log = org.apache.logging.log4j.LogManager.getLogger(RequestItemServiceImpl.class);
+    private final Logger log = org.apache.logging.log4j.LogManager.getLogger();
 
     @Autowired(required = true)
     protected RequestItemDAO requestItemDAO;
@@ -58,6 +61,12 @@ public class RequestItemServiceImpl implements RequestItemService {
         return requestItem.getToken();
     }
 
+    @Override
+    public List<RequestItem> findAll(Context context)
+            throws SQLException {
+        return requestItemDAO.findAll(context, RequestItem.class);
+    }
+
     @Override
     public RequestItem findByToken(Context context, String token) {
         try {
@@ -79,6 +88,8 @@ public class RequestItemServiceImpl implements RequestItemService {
 
     @Override
     public void delete(Context context, RequestItem requestItem) {
+        log.debug(LogManager.getHeader(context, "delete_itemrequest", "request_id={}"),
+                requestItem.getID());
         try {
             requestItemDAO.delete(context, requestItem);
         } catch (SQLException e) {

dspace-api/src/main/java/org/dspace/app/requestitem/service/RequestItemService.java

@@ -8,6 +8,7 @@
 package org.dspace.app.requestitem.service;
 
 import java.sql.SQLException;
+import java.util.List;
 
 import org.dspace.app.requestitem.RequestItem;
 import org.dspace.content.Bitstream;
@@ -41,6 +42,16 @@ public interface RequestItemService {
             boolean allFiles, String reqEmail, String reqName, String reqMessage)
         throws SQLException;
 
+    /**
+     * Fetch all item requests.
+     *
+     * @param context current DSpace session.
+     * @return all item requests.
+     * @throws java.sql.SQLException passed through.
+     */
+    public List<RequestItem> findAll(Context context)
+            throws SQLException;
+
     /**
      * Retrieve a request by its token.
      *

dspace-api/src/test/java/org/dspace/builder/RequestItemBuilder.java

@@ -42,11 +42,16 @@ public class RequestItemBuilder
     public void cleanup()
             throws Exception {
         LOG.debug("cleanup()");
-        if (null != requestItem) {
+        try ( Context ctx = new Context(); ) {
-            delete(context, requestItem);
+            ctx.turnOffAuthorisationSystem();
-            requestItem = null;
+            requestItem = ctx.reloadEntity(requestItem);
-        } else {
+            if (null != requestItem) {
-            LOG.debug("nothing to clean up.");
+                delete(ctx, requestItem);
+                ctx.complete();
+                requestItem = null;
+            } else {
+                LOG.debug("nothing to clean up.");
+            }
         }
     }
 
@@ -72,6 +77,13 @@ public class RequestItemBuilder
 
     @Override
     public RequestItem build() {
+        LOG.atDebug()
+                .withLocation()
+                .log("Building request with item ID {}",
+                        () -> requestItem.getItem().getID().toString());
+        System.out.format("Building request with item ID %s%n",
+                requestItem.getItem().getID().toString());
+        new Throwable().printStackTrace(System.out);
         // Nothing to build.
         return requestItem;
     }
@@ -85,12 +97,22 @@ public class RequestItemBuilder
     /**
      * Delete a request identified by its token.  If no such token is known,
      * simply return.
+     *
      * @param token the token identifying the request.
+     * @throws java.sql.SQLException passed through
      */
-    static public void deleteRequestItem(String token) {
+    static public void deleteRequestItem(String token)
+            throws SQLException {
+        LOG.atDebug()
+                .withLocation()
+                .log("Delete RequestItem with token {}", token);
         try (Context context = new Context()) {
             RequestItem request = requestItemService.findByToken(context, token);
+            if (null == request) {
+                return;
+            }
             requestItemService.delete(context, request);
+            context.complete();
         }
     }
 

dspace-server-webapp/src/test/java/org/dspace/app/rest/RequestItemRepositoryIT.java

@@ -27,6 +27,7 @@ import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.http.Cookie;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -68,11 +69,39 @@ public class RequestItemRepositoryIT
     @Autowired(required = true)
     RequestItemService requestItemService;
 
+    private Collection collection;
+
+    private Item item;
+
+    private Bitstream bitstream;
+
     @Before
-    public void init() {
+    public void init()
+            throws SQLException, AuthorizeException, IOException {
         context.turnOffAuthorisationSystem();
-        parentCommunity = CommunityBuilder.createCommunity(context).withName(
+        context.setCurrentUser(eperson);
-                "Parent Community").build();
+
+        parentCommunity = CommunityBuilder
+                .createCommunity(context)
+                .withName("Community")
+                .build();
+
+        collection = CollectionBuilder
+                .createCollection(context, parentCommunity)
+                .withName("Collection")
+                .build();
+
+        item = ItemBuilder
+                .createItem(context, collection)
+                .withTitle("Item")
+                .build();
+
+        InputStream is = new ByteArrayInputStream(new byte[0]);
+        bitstream = BitstreamBuilder
+                .createBitstream(context, item, is)
+                .withName("Bitstream")
+                .build();
+
         context.restoreAuthSystemState();
     }
 
@@ -86,24 +115,11 @@ public class RequestItemRepositoryIT
             throws Exception {
         System.out.println("findOne (authenticated)");
 
-        context.turnOffAuthorisationSystem();
-
-        // Create necessary supporting objects.
-        Collection collection = CollectionBuilder.createCollection(context, parentCommunity)
-                .build();
-        Item item = ItemBuilder.createItem(context, collection)
-                .build();
-        InputStream is = new ByteArrayInputStream(new byte[0]);
-        Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, is)
-                .build();
-
         // Create a request.
         RequestItem request = RequestItemBuilder
                 .createRequestItem(context, item, bitstream)
                 .build();
 
-        context.restoreAuthSystemState();
-
         // Test:  can we find it?
         String authToken = getAuthToken(admin.getEmail(), password);
         final String uri = URI_ROOT + '/' + request.getToken();
@@ -124,24 +140,11 @@ public class RequestItemRepositoryIT
             throws Exception {
         System.out.println("findOne (not authenticated)");
 
-        context.turnOffAuthorisationSystem();
-
-        // Create necessary supporting objects.
-        Collection collection = CollectionBuilder.createCollection(context, parentCommunity)
-                .build();
-        Item item = ItemBuilder.createItem(context, collection)
-                .build();
-        InputStream is = new ByteArrayInputStream(new byte[0]);
-        Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, is)
-                .build();
-
         // Create a request.
         RequestItem request = RequestItemBuilder
                 .createRequestItem(context, item, bitstream)
                 .build();
 
-        context.restoreAuthSystemState();
-
         // Test:  can we find it?
         final String uri = URI_ROOT + '/' + request.getToken();
         getClient().perform(get(uri))
@@ -161,21 +164,7 @@ public class RequestItemRepositoryIT
     @Test
     public void testCreateAndReturnAuthenticated()
             throws SQLException, AuthorizeException, IOException, Exception {
-        System.out.println("createAndReturn");
+        System.out.println("createAndReturn (authenticated)");
-
-        // Create some necessary objects.
-        context.turnOffAuthorisationSystem();
-
-        Collection col = CollectionBuilder.createCollection(context,
-                parentCommunity).build();
-        Item item = ItemBuilder.createItem(context, col).build();
-        InputStream is = new ByteArrayInputStream(new byte[0]);
-        Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, is)
-                .withName("/dev/null")
-                .withMimeType("text/plain")
-                .build();
-
-        context.restoreAuthSystemState();
 
         // Fake up a request in REST form.
         RequestItemRest rir = new RequestItemRest();
@@ -189,28 +178,34 @@ public class RequestItemRepositoryIT
         // Create it and see if it was created correctly.
         ObjectMapper mapper = new ObjectMapper();
         String authToken = getAuthToken(admin.getEmail(), password);
-        getClient(authToken)
+        AtomicReference<String> requestTokenRef = new AtomicReference<>();
-                .perform(post(URI_ROOT)
+        try {
-                        .content(mapper.writeValueAsBytes(rir))
+            getClient(authToken)
-                        .contentType(contentType))
+                    .perform(post(URI_ROOT)
-                .andExpect(status().isCreated())
+                            .content(mapper.writeValueAsBytes(rir))
-                .andExpect(content().contentType(contentType))
+                            .contentType(contentType))
-                .andExpect(jsonPath("$", Matchers.allOf(
+                    .andExpect(status().isCreated())
-                        hasJsonPath("$.id", not(is(emptyOrNullString()))),
+                    .andExpect(content().contentType(contentType))
-                        hasJsonPath("$.type", is(RequestItemRest.NAME)),
+                    .andExpect(jsonPath("$", Matchers.allOf(
-                        hasJsonPath("$.token", not(is(emptyOrNullString()))),
+                            hasJsonPath("$.id", not(is(emptyOrNullString()))),
-                        hasJsonPath("$.requestEmail", is(RequestItemBuilder.REQ_EMAIL)),
+                            hasJsonPath("$.type", is(RequestItemRest.NAME)),
-                        hasJsonPath("$.requestMessage", is(RequestItemBuilder.REQ_MESSAGE)),
+                            hasJsonPath("$.token", not(is(emptyOrNullString()))),
-                        hasJsonPath("$.requestName", is(RequestItemBuilder.REQ_NAME)),
+                            hasJsonPath("$.requestEmail", is(RequestItemBuilder.REQ_EMAIL)),
-                        hasJsonPath("$.allfiles", is(false)),
+                            hasJsonPath("$.requestMessage", is(RequestItemBuilder.REQ_MESSAGE)),
-                        hasJsonPath("$.requestDate", not(is(emptyOrNullString()))), // TODO should be an ISO datetime
+                            hasJsonPath("$.requestName", is(RequestItemBuilder.REQ_NAME)),
-                        hasJsonPath("$._links.self.href", not(is(emptyOrNullString())))
+                            hasJsonPath("$.allfiles", is(false)),
-                )))
+                            // TODO should be an ISO datetime
-                .andDo((var result) -> saveToken(read(result.getResponse().getContentAsString(), "token")))
+                            hasJsonPath("$.requestDate", not(is(emptyOrNullString()))),
-                .andReturn();
+                            hasJsonPath("$._links.self.href", not(is(emptyOrNullString())))
+                    )))
+                    .andDo((var result) -> requestTokenRef.set(
+                            read(result.getResponse().getContentAsString(), "token")))
+                    .andReturn();
 
-        // Clean up the created request.
+        } finally {
-        RequestItemBuilder.deleteRequestItem(requestToken);
+            // Clean up the created request.
+            RequestItemBuilder.deleteRequestItem(requestTokenRef.get());
+        }
     }
 
     /**
@@ -224,21 +219,7 @@ public class RequestItemRepositoryIT
     @Test
     public void testCreateAndReturnNotAuthenticated()
             throws SQLException, AuthorizeException, IOException, Exception {
-        System.out.println("createAndReturn");
+        System.out.println("createAndReturn (not authenticated)");
-
-        // Create some necessary objects.
-        context.turnOffAuthorisationSystem();
-
-        Collection col = CollectionBuilder.createCollection(context,
-                parentCommunity).build();
-        Item item = ItemBuilder.createItem(context, col).build();
-        InputStream is = new ByteArrayInputStream(new byte[0]);
-        Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, is)
-                .withName("/dev/null")
-                .withMimeType("text/plain")
-                .build();
-
-        context.restoreAuthSystemState();
 
         // Fake up a request in REST form.
         RequestItemRest rir = new RequestItemRest();
@@ -251,27 +232,32 @@ public class RequestItemRepositoryIT
 
         // Create it and see if it was created correctly.
         ObjectMapper mapper = new ObjectMapper();
-        getClient().perform(post(URI_ROOT)
+        AtomicReference<String> requestTokenRef = new AtomicReference<>();
-                        .content(mapper.writeValueAsBytes(rir))
+        try {
-                        .contentType(contentType))
+            getClient().perform(post(URI_ROOT)
-                .andExpect(status().isCreated())
+                            .content(mapper.writeValueAsBytes(rir))
-                .andExpect(content().contentType(contentType))
+                            .contentType(contentType))
-                .andExpect(jsonPath("$", Matchers.allOf(
+                    .andExpect(status().isCreated())
-                        hasJsonPath("$.id", not(is(emptyOrNullString()))),
+                    .andExpect(content().contentType(contentType))
-                        hasJsonPath("$.type", is(RequestItemRest.NAME)),
+                    .andExpect(jsonPath("$", Matchers.allOf(
-                        hasJsonPath("$.token", not(is(emptyOrNullString()))),
+                            hasJsonPath("$.id", not(is(emptyOrNullString()))),
-                        hasJsonPath("$.requestEmail", is(RequestItemBuilder.REQ_EMAIL)),
+                            hasJsonPath("$.type", is(RequestItemRest.NAME)),
-                        hasJsonPath("$.requestMessage", is(RequestItemBuilder.REQ_MESSAGE)),
+                            hasJsonPath("$.token", not(is(emptyOrNullString()))),
-                        hasJsonPath("$.requestName", is(RequestItemBuilder.REQ_NAME)),
+                            hasJsonPath("$.requestEmail", is(RequestItemBuilder.REQ_EMAIL)),
-                        hasJsonPath("$.allfiles", is(false)),
+                            hasJsonPath("$.requestMessage", is(RequestItemBuilder.REQ_MESSAGE)),
-                        hasJsonPath("$.requestDate", not(is(emptyOrNullString()))), // TODO should be an ISO datetime
+                            hasJsonPath("$.requestName", is(RequestItemBuilder.REQ_NAME)),
-                        hasJsonPath("$._links.self.href", not(is(emptyOrNullString())))
+                            hasJsonPath("$.allfiles", is(false)),
-                )))
+                            // TODO should be an ISO datetime
-                .andDo((var result) -> saveToken(read(result.getResponse().getContentAsString(), "token")))
+                            hasJsonPath("$.requestDate", not(is(emptyOrNullString()))),
-                .andReturn();
+                            hasJsonPath("$._links.self.href", not(is(emptyOrNullString())))
-
+                    )))
-        // Clean up the created request.
+                    .andDo((var result) -> requestTokenRef.set(
-        RequestItemBuilder.deleteRequestItem(requestToken);
+                            read(result.getResponse().getContentAsString(), "token")))
+                    .andReturn();
+        } finally {
+            // Clean up the created request.
+            RequestItemBuilder.deleteRequestItem(requestTokenRef.get());
+        }
     }
 
     /**
@@ -284,6 +270,8 @@ public class RequestItemRepositoryIT
     @Test
     public void testCreateWithInvalidCSRF()
             throws Exception {
+        System.out.println("testCreateWithInvalidCSRF");
+
         // Login via password to retrieve a valid token
         String token = getAuthToken(eperson.getEmail(), password);
 
@@ -294,20 +282,6 @@ public class RequestItemRepositoryIT
         Cookie[] cookies = new Cookie[1];
         cookies[0] = new Cookie(AUTHORIZATION_COOKIE, token);
 
-        // Create some necessary objects.
-        context.turnOffAuthorisationSystem();
-
-        Collection col = CollectionBuilder.createCollection(context,
-                parentCommunity).build();
-        Item item = ItemBuilder.createItem(context, col).build();
-        InputStream is = new ByteArrayInputStream(new byte[0]);
-        Bitstream bitstream = BitstreamBuilder.createBitstream(context, item, is)
-                .withName("/dev/null")
-                .withMimeType("text/plain")
-                .build();
-
-        context.restoreAuthSystemState();
-
         // Fake up a request in REST form.
         RequestItemRest rir = new RequestItemRest();
         rir.setBitstreamId(bitstream.getID().toString());
@@ -346,6 +320,8 @@ public class RequestItemRepositoryIT
     @Test
     public void testUntrustedOrigin()
             throws Exception {
+        System.out.println("testUntrustedOrigin");
+
         // First, get a valid login token
         String token = getAuthToken(eperson.getEmail(), password);
 
@@ -379,15 +355,4 @@ public class RequestItemRepositoryIT
         Class instanceClass = instance.getDomainClass();
         assertEquals("Wrong domain class", RequestItemRest.class, instanceClass);
     }
-
-    /** Saves the request token generated by creating an item request. */
-    private String requestToken;
-
-    /**
-     * Silly work-around because lambdas can't mutate external variables.
-     * @param token a request token to be remembered.
-     */
-    private void saveToken(String token) {
-        requestToken = token;
-    }
 }