Fixes #662

Removed ActivityService methods with object parameters in favour of key parameter methods

schedule/schedule-appengine/src/main/java/com/jasify/schedule/appengine/model/activity/ActivityService.java

@@ -41,12 +41,13 @@ public interface ActivityService {
      * Subscribe a user for an activity
      *
      * @param user     to subscribe
-     * @param activity to subscribe to
+     * @param activityId to subscribe to
      * @return a newly created Subscription for this user to this activity
      * @throws OperationException        if activity is fully subscribed
+     * @throws EntityNotFoundException
      */
     @Nonnull
-    Subscription subscribe(User user, Activity activity) throws OperationException;
+    Subscription subscribe(User user, Key activityId) throws OperationException, EntityNotFoundException;
 
     /**
      * @param user            who is subscribing

schedule/schedule-appengine/src/main/java/com/jasify/schedule/appengine/model/activity/DefaultActivityService.java

@@ -63,22 +63,22 @@ static ActivityService instance() {
 
     @Nonnull
     @Override
-    public Subscription subscribe(final User user, final Activity activity) throws OperationException {
-        if (activity.getMaxSubscriptions() > 0 && activity.getSubscriptionCount() >= activity.getMaxSubscriptions()) {
-            throw new OperationException("Activity fully subscribed");
-        }
+    public Subscription subscribe(final User user, final Key activityId) throws OperationException, EntityNotFoundException {
 
         try {
             return TransactionOperator.execute(new ModelOperation<Subscription>() {
                 @Override
                 public Subscription execute(Transaction tx) throws ModelException {
+                    Activity activity = activityDao.get(activityId);
+                    if (activity.getMaxSubscriptions() > 0 && activity.getSubscriptionCount() >= activity.getMaxSubscriptions()) {
+                        throw new OperationException("Activity fully subscribed");
+                    }
                     Subscription subscription = new Subscription();
 
                     subscription.getActivityRef().setKey(activity.getId());
                     subscription.getUserRef().setKey(user.getId());
 
                     activity.setSubscriptionCount(activity.getSubscriptionCount() + 1);
-                    activity.getSubscriptionListRef().getModelList().add(subscription);
 
                     activityDao.save(activity);
                     subscriptionDao.save(subscription, user.getId());
@@ -88,6 +88,8 @@ public Subscription execute(Transaction tx) throws ModelException {
                     return subscription;
                 }
             });
+        } catch (EntityNotFoundException | OperationException e) {
+            throw e;
         } catch (ModelException e) {
             throw Throwables.propagate(e);
         }

schedule/schedule-appengine/src/main/java/com/jasify/schedule/appengine/model/payment/workflow/ActivityPaymentWorkflow.java

@@ -56,8 +56,7 @@ public void onCreated() throws PaymentWorkflowException {
         try {
             ActivityService activityService = ActivityServiceFactory.getActivityService();
             User user = UserServiceFactory.getUserService().getUser(payment.getUserRef().getKey());
-            Activity activity = activityDao.get(activityId);
-            Subscription subscribe = activityService.subscribe(user, activity);
+            Subscription subscribe = activityService.subscribe(user, activityId);
             subscriptionId = subscribe.getId();
         } catch (EntityNotFoundException | OperationException e) {
             throw new PaymentWorkflowException(e);

schedule/schedule-appengine/src/main/java/com/jasify/schedule/appengine/spi/ActivityEndpoint.java

@@ -310,8 +310,7 @@ public Subscription addSubscription(User caller, @Named("userId") Key userId, @N
         mustBeSameUserOrAdminOrOrgMember(caller, userId, OrgMemberChecker.createFromActivityId(activityId));
         try {
             com.jasify.schedule.appengine.model.users.User user = userDao.get(userId);
-            Activity activity = activityDao.get(activityId);
-            return ActivityServiceFactory.getActivityService().subscribe(user, activity);
+            return ActivityServiceFactory.getActivityService().subscribe(user, activityId);
         } catch (EntityNotFoundException e) {
             throw new NotFoundException(e.getMessage());
         } catch (OperationException e) {

schedule/schedule-appengine/src/test/java/com/jasify/schedule/appengine/model/activity/ActivityServiceTest.java

@@ -107,7 +107,7 @@ public void cleanupDatastore() {
     @Test
     public void testSubscribe() throws Exception {
         Activity activity = TestHelper.createActivity(true);
-        Subscription subscription = activityService.subscribe(testUser1, activity);
+        Subscription subscription = activityService.subscribe(testUser1, activity.getId());
         assertNotNull(subscription);
         assertEquals(testUser1.getId(), subscription.getUserRef().getKey());
         assertEquals(activity.getId(), subscription.getActivityRef().getKey());
@@ -124,7 +124,7 @@ public void testSubscribeNotifiesIfUserNameIsNull() throws Exception {
         service.clearSentMessages();
         User user = new User();
         Datastore.put(user);
-        activityService.subscribe(user, activity);
+        activityService.subscribe(user, activity.getId());
         List<MailServicePb.MailMessage> sentMessages = service.getSentMessages();
         assertNotNull(sentMessages);
         assertEquals(0, sentMessages.size());
@@ -137,7 +137,7 @@ public void testSubscribeNotifiesIfActivityNameIsNull() throws Exception {
         Datastore.put(activity);
         LocalMailService service = LocalMailServiceTestConfig.getLocalMailService();
         service.clearSentMessages();
-        activityService.subscribe(testUser1, activity);
+        activityService.subscribe(testUser1, activity.getId());
         List<MailServicePb.MailMessage> sentMessages = service.getSentMessages();
         assertNotNull(sentMessages);
         assertEquals(0, sentMessages.size());
@@ -150,24 +150,24 @@ public void testOversubscribe() throws Exception {
         Activity activity = TestHelper.createActivity(true);
         activity.setMaxSubscriptions(1);
         Datastore.put(activity);
-        activityService.subscribe(testUser1, activity);
-        activityService.subscribe(testUser2, activity);
+        activityService.subscribe(testUser1, activity.getId());
+        activityService.subscribe(testUser2, activity.getId());
     }
 
     @Test
     public void testSubscribeForZeroMaxSubscriptions() throws Exception {
         Activity activity = TestHelper.createActivity(true);
         activity.setMaxSubscriptions(0);
         Datastore.put(activity);
-        assertNotNull(activityService.subscribe(testUser1, activity));
-        assertNotNull(activityService.subscribe(testUser2, activity));
+        assertNotNull(activityService.subscribe(testUser1, activity.getId()));
+        assertNotNull(activityService.subscribe(testUser2, activity.getId()));
         assertEquals(2, Datastore.get(ActivityMeta.get(), activity.getId()).getSubscriptionCount());
     }
 
     @Test
     public void testCancel() throws Exception {
         Activity activity = TestHelper.createActivity(true);
-        Subscription subscription = activityService.subscribe(testUser1, activity);
+        Subscription subscription = activityService.subscribe(testUser1, activity.getId());
 
         // cache it in
         activity1Organization1.getSubscriptionListRef().getModelList();

schedule/schedule-appengine/src/test/java/com/jasify/schedule/appengine/model/payment/workflow/ActivityPaymentWorkflowTest.java

@@ -1,5 +1,6 @@
 package com.jasify.schedule.appengine.model.payment.workflow;
 
+import com.google.appengine.api.datastore.Key;
 import com.jasify.schedule.appengine.TestHelper;
 import com.jasify.schedule.appengine.meta.activity.ActivityMeta;
 import com.jasify.schedule.appengine.meta.payment.workflow.ActivityPaymentWorkflowMeta;
@@ -83,7 +84,7 @@ public void testOnCreated() throws Exception {
             ActivityService activityService = testActivityServiceFactory.getActivityServiceMock();
             // TODO: This is wrong
            // expect(activityService.subscribe(user, activity)).andReturn(subscription);
-            expect(activityService.subscribe(anyObject(User.class), anyObject(Activity.class))).andReturn(subscription);
+            expect(activityService.subscribe(anyObject(User.class), anyObject(Key.class))).andReturn(subscription);
             testActivityServiceFactory.replay();
 
             ActivityPaymentWorkflow transition = PaymentWorkflowEngine.transition(activityPaymentWorkflow.getId(), PaymentStateEnum.Created);