[CST-7756] refactoring & added missing JavaDoc

dspace-api/src/main/java/org/dspace/content/factory/ContentServiceFactory.java

@@ -117,10 +117,7 @@ public abstract class ContentServiceFactory {
     }
 
     public <T extends DSpaceObject> DSpaceObjectService<T> getDSpaceObjectService(T dso) {
-        // No need to worry when supressing, as long as our "getDSpaceObjectManager" method is properly implemented
+        return getDSpaceObjectService(dso.getType());
-        // no casting issues should occur
-        DSpaceObjectService<T> manager = getDSpaceObjectService(dso.getType());
-        return manager;
     }
 
     @SuppressWarnings("unchecked")

dspace-api/src/main/java/org/dspace/eperson/SubscribeParameterServiceImpl.java

@@ -1,76 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.eperson;
-
-import java.sql.SQLException;
-import java.util.List;
-import java.util.Objects;
-
-import org.dspace.core.Context;
-import org.dspace.eperson.dao.SubscriptionParameterDAO;
-import org.dspace.eperson.service.SubscriptionParameterService;
-import org.springframework.beans.factory.annotation.Autowired;
-
-/**
- * Class implemennting method for service layer of SubscriptionParameter entity
- *
- * @author Alba Aliu at atis.al
- */
-public class SubscribeParameterServiceImpl implements SubscriptionParameterService {
-
-    @Autowired(required = true)
-    protected SubscriptionParameterDAO subscriptionParameterDAO;
-
-    protected SubscribeParameterServiceImpl() {}
-
-    @Override
-    public List<SubscriptionParameter> findAll(Context context) throws SQLException {
-        return subscriptionParameterDAO.findAll(context, SubscriptionParameter.class);
-    }
-
-    @Override
-    public SubscriptionParameter add(Context context, String name, String value, Subscription subscription)
-            throws SQLException {
-        SubscriptionParameter subscriptionParameter =
-                subscriptionParameterDAO.create(context, new SubscriptionParameter());
-        subscriptionParameter.setName(name);
-        subscriptionParameter.setSubscription(subscription);
-        subscriptionParameter.setValue(value);
-        return subscriptionParameter;
-    }
-
-    @Override
-    public SubscriptionParameter edit(Context context, Integer id, String value, String name, Subscription subscription)
-            throws SQLException {
-        SubscriptionParameter subscriptionParameter =
-                    subscriptionParameterDAO.findByID(context, SubscriptionParameter.class, id);
-        subscriptionParameter.setName(name);
-        subscriptionParameter.setSubscription(subscription);
-        subscriptionParameter.setValue(value);
-        subscriptionParameterDAO.save(context, subscriptionParameter);
-        return subscriptionParameter;
-    }
-
-    @Override
-    public SubscriptionParameter findById(Context context, int id) throws SQLException {
-        return subscriptionParameterDAO.findByID(context, SubscriptionParameter.class, id);
-    }
-
-    @Override
-    public void delete(Context context, Integer id) throws SQLException {
-        SubscriptionParameter subscriptionParameter =
-                subscriptionParameterDAO.findByID(context, SubscriptionParameter.class, id);
-        if (Objects.nonNull(subscriptionParameter)) {
-            subscriptionParameter.setSubscription(null);
-            subscriptionParameterDAO.delete(context, subscriptionParameter);
-        } else {
-            throw new SQLException("Subscription parameter with id" + id + "do not exists");
-        }
-    }
-
-}

dspace-api/src/main/java/org/dspace/eperson/SubscribeServiceImpl.java

@@ -72,7 +72,7 @@ public class SubscribeServiceImpl implements SubscribeService {
                     newSubscription.addParameter(subscriptionParameter));
             newSubscription.setePerson(eperson);
             newSubscription.setdSpaceObject(dSpaceObject);
-            newSubscription.setType(type);
+            newSubscription.setSubscriptionType(type);
             return newSubscription;
         } else {
             throw new AuthorizeException("Only admin or e-person themselves can subscribe");
@@ -153,7 +153,7 @@ public class SubscribeServiceImpl implements SubscribeService {
                                            throws SQLException {
         Subscription subscriptionDB = subscriptionDAO.findByID(context, Subscription.class, id);
         subscriptionDB.removeParameterList();
-        subscriptionDB.setType(type);
+        subscriptionDB.setSubscriptionType(type);
         subscriptionDB.setdSpaceObject(dSpaceObject);
         subscriptionParameterList.forEach(x -> subscriptionDB.addParameter(x));
         subscriptionDB.setePerson(eperson);
@@ -185,9 +185,10 @@ public class SubscribeServiceImpl implements SubscribeService {
     }
 
     @Override
-    public List<Subscription> findAllSubscriptionsByTypeAndFrequency(Context context,String type, String frequencyValue)
+    public List<Subscription> findAllSubscriptionsBySubscriptionTypeAndFrequency(Context context,
-            throws SQLException {
+            String subscriptionType, String frequencyValue) throws SQLException {
-        return subscriptionDAO.findAllSubscriptionsByTypeAndFrequency(context, type, frequencyValue);
+        return subscriptionDAO.findAllSubscriptionsBySubscriptionTypeAndFrequency(context, subscriptionType,
+                frequencyValue);
     }
 
     @Override

dspace-api/src/main/java/org/dspace/eperson/Subscription.java

@@ -49,8 +49,13 @@ public class Subscription implements ReloadableEntity<Integer> {
     @JoinColumn(name = "eperson_id")
     private EPerson ePerson;
 
+    /**
+     * Represent subscription type, for example, "content" or  "statistics".
+     * 
+     * NOTE: Currently, in DSpace we use only one "content"
+     */
     @Column(name = "type")
-    private String type;
+    private String subscriptionType;
 
     @OneToMany(fetch = FetchType.LAZY, mappedBy = "subscription", cascade = CascadeType.ALL, orphanRemoval = true)
     private List<SubscriptionParameter> subscriptionParameterList = new ArrayList<>();
@@ -86,12 +91,12 @@ public class Subscription implements ReloadableEntity<Integer> {
         this.dSpaceObject = dSpaceObject;
     }
 
-    public String getType() {
+    public String getSubscriptionType() {
-        return type;
+        return subscriptionType;
     }
 
-    public void setType(String type) {
+    public void setSubscriptionType(String subscriptionType) {
-        this.type = type;
+        this.subscriptionType = subscriptionType;
     }
 
     public List<SubscriptionParameter> getSubscriptionParameterList() {

dspace-api/src/main/java/org/dspace/eperson/SubscriptionParameter.java

@@ -21,6 +21,7 @@ import org.dspace.core.ReloadableEntity;
 
 /**
  * Database entity representation of the subscription_parameter table
+ * SubscriptionParameter represents a frequency with which an user wants to be notified.
  *
  * @author Alba Aliu at atis.al
  */
@@ -39,9 +40,16 @@ public class SubscriptionParameter implements ReloadableEntity<Integer> {
     @JoinColumn(name = "subscription_id", nullable = false)
     private Subscription subscription;
 
+    /*
+     * Currently, we have only one use case for this attribute: "frequency"
+     */
     @Column
     private String name;
 
+    /*
+     * Currently, we use this attribute only with following values: "D", "W", "M".
+     * Where D stand for Day, W stand for Week and M stand for Month
+     */
     @Column
     private String value;
 

dspace-api/src/main/java/org/dspace/eperson/dao/SubscriptionDAO.java

@@ -26,31 +26,125 @@ import org.dspace.eperson.Subscription;
  */
 public interface SubscriptionDAO extends GenericDAO<Subscription> {
 
+    /**
+     * Delete all subscription of provided dSpaceObject
+     * 
+     * @param context         DSpace context object
+     * @param dSpaceObject    DSpace resource
+     * @throws SQLException   If database error
+     */
     public void deleteByDspaceObject(Context context, DSpaceObject dSpaceObject) throws SQLException;
 
+    /**
+     * Return a paginated list of all subscriptions of the eperson
+     * 
+     * @param context        DSpace context object
+     * @param eperson        ePerson whose subscriptions want to find
+     * @param limit          Paging limit
+     * @param offset         The position of the first result to return
+     * @return
+     * @throws SQLException  If database error
+     */
     public List<Subscription> findByEPerson(Context context, EPerson eperson, Integer limit, Integer offset)
             throws SQLException;
 
-    public List<Subscription> findByEPersonAndDso(Context context,EPerson eperson, DSpaceObject dSpaceObject,
+    /**
+     * Return a paginated list of subscriptions related to a DSpaceObject belong to an ePerson
+     * 
+     * @param context        DSpace context object
+     * @param eperson        ePerson whose subscriptions want to find
+     * @param dSpaceObject   DSpaceObject of whom subscriptions want to find
+     * @param limit          Paging limit
+     * @param offset         The position of the first result to return
+     * @return
+     * @throws SQLException  If database error
+     */
+    public List<Subscription> findByEPersonAndDso(Context context, EPerson eperson, DSpaceObject dSpaceObject,
                                                   Integer limit, Integer offset) throws SQLException;
 
+    /**
+     * Delete all subscription of provided ePerson
+     * 
+     * @param context        DSpace context object
+     * @param eperson        ePerson whose subscriptions want to delete
+     * @throws SQLException  If database error
+     */
     public void deleteByEPerson(Context context, EPerson eperson) throws SQLException;
 
-    public void deleteByDSOAndEPerson(Context context, DSpaceObject dSpaceObject, EPerson eperson)
+    /**
-            throws SQLException;
+     * Delete all subscriptions related to a DSpaceObject belong to an ePerson
+     * 
+     * @param context        DSpace context object
+     * @param dSpaceObject   DSpaceObject of whom subscriptions want to delete
+     * @param eperson        ePerson whose subscriptions want to delete
+     * @throws SQLException  If database error
+     */
+    public void deleteByDSOAndEPerson(Context context, DSpaceObject dSpaceObject, EPerson eperson) throws SQLException;
 
+    /**
+     * Return a paginated list of all subscriptions ordered by ID and resourceType
+     * 
+     * @param context        DSpace context object
+     * @param resourceType   Could be Item, Collection or Community
+     * @param limit          Paging limit
+     * @param offset         The position of the first result to return
+     * @return
+     * @throws SQLException  If database error
+     */
     public List<Subscription> findAllOrderedByIDAndResourceType(Context context, String resourceType,
                                                                 Integer limit, Integer offset) throws SQLException;
 
+    /**
+     * Return a paginated list of subscriptions ordered by DSpaceObject
+     * 
+     * @param context        DSpace context object
+     * @param limit          Paging limit
+     * @param offset         The position of the first result to return
+     * @return
+     * @throws SQLException  If database error
+     */
     public List<Subscription> findAllOrderedByDSO(Context context, Integer limit, Integer offset) throws SQLException;
 
-    public List<Subscription> findAllSubscriptionsByTypeAndFrequency(Context context, String type,String frequencyValue)
+    /**
-            throws SQLException;
+     * Return a list of all subscriptions by subscriptionType and frequency
+     * 
+     * @param context           DSpace context object
+     * @param subscriptionType  Could be "content" or "statistics". NOTE: in DSpace we have only "content"
+     * @param frequencyValue    Could be "D" stand for Day, "W" stand for Week, and "M" stand for Month
+     * @return
+     * @throws SQLException     If database error
+     */
+    public List<Subscription> findAllSubscriptionsBySubscriptionTypeAndFrequency(Context context,
+            String subscriptionType, String frequencyValue) throws SQLException;
 
+    /**
+     * Count all subscriptions
+     * 
+     * @param context        DSpace context object
+     * @return               Total of all subscriptions
+     * @throws SQLException  If database error
+     */
     public Long countAll(Context context) throws SQLException;
 
+    /**
+     * Count all subscriptions belong to an ePerson
+     * 
+     * @param context        DSpace context object
+     * @param ePerson        ePerson whose subscriptions want count
+     * @return               Total of all subscriptions belong to an ePerson
+     * @throws SQLException  If database error
+     */
     public Long countAllByEPerson(Context context, EPerson ePerson) throws SQLException;
 
+    /**
+     * Count all subscriptions related to a DSpaceObject belong to an ePerson
+     * 
+     * @param context        DSpace context object
+     * @param ePerson        ePerson whose subscriptions want count
+     * @param dSpaceObject   DSpaceObject of whom subscriptions want count
+     * @return
+     * @throws SQLException  If database error
+     */
     public Long countAllByEPersonAndDso(Context context, EPerson ePerson,DSpaceObject dSpaceObject) throws SQLException;
 
 }

dspace-api/src/main/java/org/dspace/eperson/dao/impl/SubscriptionDAOImpl.java

@@ -132,14 +132,15 @@ public class SubscriptionDAOImpl extends AbstractHibernateDAO<Subscription> impl
     }
 
     @Override
-    public List<Subscription> findAllSubscriptionsByTypeAndFrequency(Context context,
+    public List<Subscription> findAllSubscriptionsBySubscriptionTypeAndFrequency(Context context,
-                        String type, String frequencyValue) throws SQLException {
+                        String subscriptionType, String frequencyValue) throws SQLException {
         CriteriaBuilder criteriaBuilder = getCriteriaBuilder(context);
         CriteriaQuery criteriaQuery = getCriteriaQuery(criteriaBuilder, Subscription.class);
         Root<Subscription> subscriptionRoot = criteriaQuery.from(Subscription.class);
         criteriaQuery.select(subscriptionRoot);
         Join<Subscription, SubscriptionParameter> childJoin = subscriptionRoot.join("subscriptionParameterList");
-        criteriaQuery.where(criteriaBuilder.and(criteriaBuilder.equal(subscriptionRoot.get(Subscription_.TYPE), type),
+        criteriaQuery.where(
+                criteriaBuilder.and(criteriaBuilder.equal(subscriptionRoot.get(Subscription_.TYPE), subscriptionType),
                 criteriaBuilder.equal(childJoin.get(SubscriptionParameter_.name), "frequency"),
                 criteriaBuilder.equal(childJoin.get(SubscriptionParameter_.value), frequencyValue)
                 ));

dspace-api/src/main/java/org/dspace/eperson/service/SubscribeService.java

@@ -200,15 +200,15 @@ public interface SubscribeService {
     public void deleteSubscription(Context context, Subscription subscription) throws SQLException;
 
     /**
-     * Finds all subscriptions having given type and frequency
+     * Finds all subscriptions by subscriptionType and frequency
      *
      * @param context             DSpace context
-     * @param type String type of subscription
+     * @param subscriptionType    Could be "content" or "statistics". NOTE: in DSpace we have only "content"
-     * @param frequencyValue String frequency value of subscription
+     * @param frequencyValue      Could be "D" stand for Day, "W" stand for Week, and "M" stand for Month
      * @throws SQLException       An exception that provides information on a database access error or other errors.
      */
-    public List<Subscription> findAllSubscriptionsByTypeAndFrequency(Context context, String type,
+    public List<Subscription> findAllSubscriptionsBySubscriptionTypeAndFrequency(Context context,
-            String frequencyValue) throws SQLException;
+            String subscriptionType, String frequencyValue) throws SQLException;
 
     /**
      * Counts all subscriptions

dspace-api/src/main/java/org/dspace/eperson/service/SubscriptionParameterService.java

@@ -1,82 +0,0 @@
-/**
- * The contents of this file are subject to the license and copyright
- * detailed in the LICENSE and NOTICE files at the root of the source
- * tree and available online at
- *
- * http://www.dspace.org/license/
- */
-package org.dspace.eperson.service;
-
-import java.sql.SQLException;
-import java.util.List;
-
-import org.dspace.authorize.AuthorizeException;
-import org.dspace.core.Context;
-import org.dspace.eperson.Subscription;
-import org.dspace.eperson.SubscriptionParameter;
-
-/**
- * Service interface class for the SubscriptionParameter object.
- * The implementation of this class is responsible for all business logic calls for
- * the SubscriptionParameter object and is autowired by spring
- * Class defining methods for sending new item e-mail alerts to users
- *
- * @author Alba Aliu @atis.al
- */
-public interface SubscriptionParameterService {
-    /**
-     * Finds all of subscriptions parameter
-     *
-     * @param context DSpace context
-     * @return list of Subscription objects
-     * @throws SQLException An exception that provides information on a database access error or other errors.
-     */
-    public List<SubscriptionParameter> findAll(Context context) throws SQLException;
-
-    /**
-     * Adds a new subscription parameter related with a subscription
-     *
-     * @param context    DSpace context
-     * @param value      String value
-     * @param name       String name
-     * @param subscription    Subscription subscription
-     * @throws SQLException       An exception that provides information on a database access error or other errors.
-     * @throws AuthorizeException Exception indicating the current user of the context does not have permission
-     *                            to perform a particular action.
-     */
-    public SubscriptionParameter add(Context context, String value, String name, Subscription subscription)
-            throws SQLException;
-
-    /**
-     * Updates a  subscription parameter with id
-     *
-     * @param context    DSpace context
-     * @param id    Integer id
-     * @param value      String value
-     * @param name       String name
-     * @param subscription    Subscription subscription
-     * @throws SQLException       An exception that provides information on a database access error or other errors.
-     * @throws AuthorizeException Exception indicating the current user of the context does not have permission
-     *                            to perform a particular action.
-     */
-    public SubscriptionParameter edit(Context context, Integer id, String value, String name, Subscription subscription)
-            throws SQLException;
-
-    /**
-     * Finds a subscriptionParameter by id
-     *
-     * @param context DSpace context
-     * @param id the id of subscriptionParameter to be searched
-     * @throws SQLException An exception that provides information on a database access error or other errors.
-     */
-    public SubscriptionParameter findById(Context context, int id) throws SQLException;
-
-    /**
-     * Deletes a subscriptionParameter with id
-     *
-     * @param context DSpace context
-     * @throws SQLException An exception that provides information on a database access error or other errors.
-     */
-    public void delete(Context context, Integer id) throws SQLException;
-
-}

dspace-api/src/main/java/org/dspace/subscriptions/SubscriptionEmailNotification.java

@@ -7,6 +7,8 @@
  */
 package org.dspace.subscriptions;
 
+import static org.dspace.subscriptions.SubscriptionEmailNotificationService.FREQUENCIES;
+
 import java.sql.SQLException;
 import java.util.Objects;
 import java.util.UUID;
@@ -51,13 +53,13 @@ public class SubscriptionEmailNotification
         if (StringUtils.isBlank(frequencyOption)) {
             throw new IllegalArgumentException("Option frequency f must be set");
         }
-        if (!frequencyOption.equals("D") && !frequencyOption.equals("M") && !frequencyOption.equals("W")) {
+        if (!FREQUENCIES.contains(frequencyOption)) {
-            throw new IllegalArgumentException("Option f must be D, M or W");
+            throw new IllegalArgumentException("Option f must be one of following values:" + FREQUENCIES);
         }
         subscriptionEmailNotificationService.perform(getContext(), handler, "content", frequencyOption);
     }
 
-    protected void assignCurrentUserInContext() throws SQLException {
+    private void assignCurrentUserInContext() throws SQLException {
         context = new Context();
         UUID uuid = getEpersonIdentifier();
         if (Objects.nonNull(uuid)) {

dspace-api/src/main/java/org/dspace/subscriptions/SubscriptionEmailNotificationConfiguration.java

@@ -42,7 +42,8 @@ public class SubscriptionEmailNotificationConfiguration<T
     public Options getOptions() {
         if (Objects.isNull(options)) {
             Options options = new Options();
-            options.addOption("f", "Frequency", true, "Subscription frequency. It can have value D, M or W");
+            options.addOption("f", "Frequency", true, "Subscription frequency."
+                            + " It can have value D stand for 'Day', W stand for 'Week' and M stand for 'Month'");
             options.getOption("f").setRequired(true);
             super.options = options;
         }

dspace-api/src/main/java/org/dspace/subscriptions/SubscriptionEmailNotificationService.java

@@ -61,13 +61,14 @@ public class SubscriptionEmailNotificationService {
     public void perform(Context context, DSpaceRunnableHandler handler, String type, String frequency) {
         try {
             context.turnOffAuthorisationSystem();
-            List<Subscription> subscriptionList = findAllSubscriptionsByTypeAndFrequency(context, type, frequency);
+            List<Subscription> subscriptions = findAllSubscriptionsBySubscriptionTypeAndFrequency(context, type,
+                                               frequency);
             // if content subscription
             // Here is verified if type is "content" Or "statistics" as them are configured
             if (type.equals(generators.keySet().toArray()[0])) {
                 // the list of the person who has subscribed
                 int iterator = 0;
-                for (Subscription subscription : subscriptionList) {
+                for (Subscription subscription : subscriptions) {
                     DSpaceObject dSpaceObject = getdSpaceObject(subscription);
                     if (dSpaceObject instanceof Community) {
                         communities.addAll(contentUpdates.get(Community.class.getSimpleName().toLowerCase(Locale.ROOT))
@@ -80,8 +81,8 @@ public class SubscriptionEmailNotificationService {
                              .findUpdates(context, dSpaceObject, frequency));
                     }
                     var ePerson = subscription.getePerson();
-                    if (iterator < subscriptionList.size() - 1) {
+                    if (iterator < subscriptions.size() - 1) {
-                        if (ePerson.equals(subscriptionList.get(iterator + 1).getePerson())) {
+                        if (ePerson.equals(subscriptions.get(iterator + 1).getePerson())) {
                             iterator++;
                             continue;
                         } else {
@@ -120,14 +121,16 @@ public class SubscriptionEmailNotificationService {
         return dSpaceObject;
     }
 
-    private List<Subscription> findAllSubscriptionsByTypeAndFrequency(Context context, String type, String frequency) {
+    private List<Subscription> findAllSubscriptionsBySubscriptionTypeAndFrequency(Context context,
+             String subscriptionType, String frequency) {
         try {
-            return this.subscribeService.findAllSubscriptionsByTypeAndFrequency(context, type, frequency)
+            return subscribeService.findAllSubscriptionsBySubscriptionTypeAndFrequency(context, subscriptionType,
+                                    frequency)
                                    .stream()
                                    .sorted(Comparator.comparing(s -> s.getePerson().getID()))
                                    .collect(Collectors.toList());
         } catch (SQLException e) {
-            log.error(e.getMessage());
+            log.error(e.getMessage(), e);
         }
         return new ArrayList<Subscription>();
     }

dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/SubscriptionConverter.java

@@ -41,11 +41,10 @@ public class SubscriptionConverter implements DSpaceConverter<Subscription, Subs
             SubscriptionParameterRest subscriptionParameterRest = new SubscriptionParameterRest();
             subscriptionParameterRest.setName(subscriptionParameter.getName());
             subscriptionParameterRest.setValue(subscriptionParameter.getValue());
-            subscriptionParameterRest.setId(subscriptionParameter.getID());
             subscriptionParameterRestList.add(subscriptionParameterRest);
         }
         rest.setSubscriptionParameterList(subscriptionParameterRestList);
-        rest.setSubscriptionType(subscription.getType());
+        rest.setSubscriptionType(subscription.getSubscriptionType());
         return rest;
     }
 

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

@@ -31,10 +31,10 @@ public class SubscriptionMatcher {
         return allOf(
                 hasJsonPath("$.id", is(subscription.getID())),
                 hasJsonPath("$.type", is("subscription")),
-                hasJsonPath("$.subscriptionType", is(subscription.getType())),
+                hasJsonPath("$.subscriptionType", is(subscription.getSubscriptionType())),
                 hasJsonPath("$.subscriptionParameterList", Matchers.containsInAnyOrder(
                     subscription.getSubscriptionParameterList().stream()
-                        .map(x -> SubscriptionMatcher.matchSubscriptionParameter(x.getID(), x.getName(), x.getValue()))
+                                .map(x -> SubscriptionMatcher.matchSubscriptionParameter(x.getName(), x.getValue()))
                                 .collect(Collectors.toList())
                 )),
                 //Check links
@@ -42,9 +42,8 @@ public class SubscriptionMatcher {
                 );
     }
 
-    public static Matcher<? super Object> matchSubscriptionParameter(int id, String name, String value) {
+    public static Matcher<? super Object> matchSubscriptionParameter(String name, String value) {
         return allOf(
-            hasJsonPath("$.id", is(id)),
             hasJsonPath("$.name", is(name)),
             hasJsonPath("$.value", is(value))
         );