SAK-29584 Ensure that a UUID is passed through when updating a popup.

Previously, modifications to an existing popup would not persist.

Modified the call to .getUUID to throw an exception if a UUID value was
missing where one was expected, and modified callers to deal with this.
The tool should fail more loudly in the future, while the code that
integrates with the portal should swallow errors and log to avoid
interrupting the user's session.

pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Banner.java

@@ -34,7 +34,6 @@
  * A data object representing a banner.
  */
 public class Banner implements Comparable<Banner> {
-    @Getter
     private final String uuid;
     @Getter
     private final String message;
@@ -104,7 +103,11 @@ public boolean equals(Object obj) {
             return false;
         }
 
-        return uuid.equals(((Banner)obj).getUuid());
+        try {
+            return uuid.equals(((Banner)obj).getUuid());
+        } catch (MissingUuidException e) {
+            return false;
+        }
     }
 
     public int hashCode() {
@@ -118,6 +121,14 @@ public int getSeverityScore() {
         return type.ordinal();
     }
 
+    public String getUuid() throws MissingUuidException {
+        if (this.uuid == null) {
+            throw new MissingUuidException("No UUID has been set for this banner");
+        }
+
+        return this.uuid;
+    }
+
     /**
      * Whether or not this banner should be displayed at this moment in time.
      */

pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/MissingUuidException.java

@@ -0,0 +1,31 @@
+/**********************************************************************************
+ *
+ * Copyright (c) 2015 The Sakai Foundation
+ *
+ * Original developers:
+ *
+ *   New York University
+ *   Payten Giles
+ *   Mark Triggs
+ *
+ * Licensed under the Educational Community License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *       http://www.osedu.org/licenses/ECL-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ **********************************************************************************/
+
+package org.sakaiproject.pasystem.api;
+
+public class MissingUuidException extends Exception {
+    public MissingUuidException(String msg) {
+        super(msg);
+    }
+}

pasystem/pasystem-api/api/src/java/org/sakaiproject/pasystem/api/Popup.java

@@ -32,8 +32,6 @@
  * A data object representing a popup.
  */
 public class Popup {
-
-    @Getter
     private final String uuid;
     @Getter
     private String descriptor;
@@ -87,6 +85,15 @@ public static Popup create(String uuid, String descriptor, long startTime, long
         return new Popup(uuid, descriptor, startTime, endTime, isOpenCampaign, template);
     }
 
+
+    public String getUuid() throws MissingUuidException {
+        if (this.uuid == null) {
+            throw new MissingUuidException("No UUID has been set for this popup");
+        }
+
+        return this.uuid;
+    }
+
     /**
      * Whether a given popup could be displayed right now.
      */

pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/PASystemImpl.java

@@ -47,6 +47,7 @@
 import org.sakaiproject.pasystem.api.I18n;
 import org.sakaiproject.pasystem.api.PASystem;
 import org.sakaiproject.pasystem.api.PASystemException;
+import org.sakaiproject.pasystem.api.MissingUuidException;
 import org.sakaiproject.pasystem.api.Popup;
 import org.sakaiproject.pasystem.api.Popups;
 import org.sakaiproject.pasystem.impl.banners.BannerStorage;
@@ -214,13 +215,17 @@ private String getActiveBannersJSON() {
 
         if (currentUser != null && currentUser.getEid() != null) {
             for (Banner banner : getBanners().getRelevantBanners(serverId, currentUser.getEid())) {
-                JSONObject bannerData = new JSONObject();
-                bannerData.put("id", banner.getUuid());
-                bannerData.put("message", banner.getMessage());
-                bannerData.put("dismissible", banner.isDismissible());
-                bannerData.put("dismissed", banner.isDismissed());
-                bannerData.put("type", banner.getType());
-                banners.add(bannerData);
+                try {
+                    JSONObject bannerData = new JSONObject();
+                    bannerData.put("id", banner.getUuid());
+                    bannerData.put("message", banner.getMessage());
+                    bannerData.put("dismissible", banner.isDismissible());
+                    bannerData.put("dismissed", banner.isDismissed());
+                    bannerData.put("type", banner.getType());
+                    banners.add(bannerData);
+                } catch (Exception e) {
+                    LOG.warn("Error processing banner: " + banner, e);
+                }
             }
         }
 
@@ -235,24 +240,25 @@ private String getPopupsFooter(Handlebars handlebars, Map<String, Object> contex
             return "";
         }
 
-        if (session.getAttribute(POPUP_SCREEN_SHOWN) == null) {
-            Popup popup = new PopupForUser(currentUser).getPopup();
-            if (popup.isActiveNow()) {
-                context.put("popupTemplate", popup.getTemplate());
-                context.put("popupUuid", popup.getUuid());
-                context.put("popup", true);
-
-                if (currentUser.getEid() != null) {
-                    // Delivered!
-                    session.setAttribute(POPUP_SCREEN_SHOWN, "true");
+        try {
+            if (session.getAttribute(POPUP_SCREEN_SHOWN) == null) {
+                Popup popup = new PopupForUser(currentUser).getPopup();
+                if (popup.isActiveNow()) {
+                    context.put("popupTemplate", popup.getTemplate());
+                    context.put("popupUuid", popup.getUuid());
+                    context.put("popup", true);
+
+                    if (currentUser.getEid() != null) {
+                        // Delivered!
+                        session.setAttribute(POPUP_SCREEN_SHOWN, "true");
+                    }
                 }
             }
-        }
 
-        try {
+
             Template template = handlebars.compile("templates/popup_footer");
             return template.apply(context);
-        } catch (IOException e) {
+        } catch (IOException | MissingUuidException e) {
             LOG.warn("IOException while getting popups footer", e);
             return "";
         }

pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/banners/BannerStorage.java

@@ -36,6 +36,7 @@
 import org.sakaiproject.pasystem.api.AcknowledgementType;
 import org.sakaiproject.pasystem.api.Banner;
 import org.sakaiproject.pasystem.api.Banners;
+import org.sakaiproject.pasystem.api.MissingUuidException;
 import org.sakaiproject.pasystem.api.PASystemException;
 import org.sakaiproject.pasystem.impl.acknowledgements.AcknowledgementStorage;
 import org.sakaiproject.pasystem.impl.common.DB;
@@ -193,26 +194,32 @@ public String call(DBConnection db) throws SQLException {
 
     @Override
     public void updateBanner(Banner banner) {
-        DB.transaction("Update banner with uuid " + banner.getUuid(),
-                new DBAction<Void>() {
-                    @Override
-                    public Void call(DBConnection db) throws SQLException {
-                        db.run("UPDATE pasystem_banner_alert SET message = ?, hosts = ?, active = ?, start_time = ?, end_time = ?, banner_type = ? WHERE uuid = ?")
-                                .param(banner.getMessage())
-                                .param(banner.getHosts())
-                                .param(Integer.valueOf(banner.isActive() ? 1 : 0))
-                                .param(banner.getStartTime())
-                                .param(banner.getEndTime())
-                                .param(banner.getType())
-                                .param(banner.getUuid())
-                                .executeUpdate();
-
-                        db.commit();
-
-                        return null;
+        try {
+            final String uuid = banner.getUuid();
+
+            DB.transaction("Update banner with uuid " + uuid,
+                    new DBAction<Void>() {
+                        @Override
+                        public Void call(DBConnection db) throws SQLException {
+                            db.run("UPDATE pasystem_banner_alert SET message = ?, hosts = ?, active = ?, start_time = ?, end_time = ?, banner_type = ? WHERE uuid = ?")
+                                    .param(banner.getMessage())
+                                    .param(banner.getHosts())
+                                    .param(Integer.valueOf(banner.isActive() ? 1 : 0))
+                                    .param(banner.getStartTime())
+                                    .param(banner.getEndTime())
+                                    .param(banner.getType())
+                                    .param(uuid)
+                                    .executeUpdate();
+
+                            db.commit();
+
+                            return null;
+                        }
                     }
-                }
-        );
+            );
+        } catch (MissingUuidException e) {
+            throw new RuntimeException("Can't update a banner with no UUID specified", e);
+        }
     }
 
     @Override

pasystem/pasystem-impl/impl/src/java/org/sakaiproject/pasystem/impl/popups/PopupStorage.java

@@ -35,6 +35,7 @@
 import java.util.UUID;
 import org.sakaiproject.pasystem.api.AcknowledgementType;
 import org.sakaiproject.pasystem.api.Acknowledger;
+import org.sakaiproject.pasystem.api.MissingUuidException;
 import org.sakaiproject.pasystem.api.Popup;
 import org.sakaiproject.pasystem.api.Popups;
 import org.sakaiproject.pasystem.api.TemplateStream;
@@ -85,36 +86,42 @@ public String call(DBConnection db) throws SQLException {
 
     @Override
     public void updateCampaign(Popup popup,
-                               Optional<TemplateStream> templateInput,
-                               Optional<List<String>> assignToUsers) {
-        DB.transaction
-                ("Update an existing popup campaign",
-                        new DBAction<Void>() {
-                            @Override
-                            public Void call(DBConnection db) throws SQLException {
+        Optional<TemplateStream> templateInput,
+        Optional<List<String>> assignToUsers) {
+        try {
+            final String uuid = popup.getUuid();
+
+            DB.transaction
+                    ("Update an existing popup campaign",
+                            new DBAction<Void>() {
+                                @Override
+                                public Void call(DBConnection db) throws SQLException {
+
+                                    db.run("UPDATE pasystem_popup_screens SET descriptor = ?, start_time = ?, end_time = ?, open_campaign = ? WHERE uuid = ?")
+                                            .param(popup.getDescriptor())
+                                            .param(popup.getStartTime())
+                                            .param(popup.getEndTime())
+                                            .param(popup.isOpenCampaign() ? 1 : 0)
+                                            .param(uuid)
+                                            .executeUpdate();
+
+                                    setPopupAssignees(db, uuid, assignToUsers);
+
+                                    if (templateInput.isPresent()) {
+                                        setPopupContent(db, uuid, templateInput.get());
+                                    }
 
-                                db.run("UPDATE pasystem_popup_screens SET descriptor = ?, start_time = ?, end_time = ?, open_campaign = ? WHERE uuid = ?")
-                                        .param(popup.getDescriptor())
-                                        .param(popup.getStartTime())
-                                        .param(popup.getEndTime())
-                                        .param(popup.isOpenCampaign() ? 1 : 0)
-                                        .param(popup.getUuid())
-                                        .executeUpdate();
+                                    db.commit();
 
-                                setPopupAssignees(db, popup.getUuid(), assignToUsers);
+                                    LOG.info("Update of popup {} completed", uuid);
 
-                                if (templateInput.isPresent()) {
-                                    setPopupContent(db, popup.getUuid(), templateInput.get());
+                                    return null;
                                 }
-
-                                db.commit();
-
-                                LOG.info("Update of popup {} completed", popup.getUuid());
-
-                                return null;
                             }
-                        }
-                );
+                    );
+        } catch (MissingUuidException e) {
+            throw new RuntimeException("Can't update a popup with no UUID specified", e);
+        }
     }
 
     @Override

pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/forms/BannerForm.java

@@ -28,6 +28,7 @@
 import lombok.Data;
 import org.sakaiproject.pasystem.api.Banner;
 import org.sakaiproject.pasystem.api.Errors;
+import org.sakaiproject.pasystem.api.MissingUuidException;
 
 /**
  * Maps to and from the banner HTML form and a banner data object.
@@ -51,15 +52,19 @@ private BannerForm(String uuid, String message, String hosts, long startTime, lo
     }
 
     public static BannerForm fromBanner(Banner existingBanner) {
-        String uuid = existingBanner.getUuid();
-
-        return new BannerForm(uuid,
-                existingBanner.getMessage(),
-                existingBanner.getHosts(),
-                existingBanner.getStartTime(),
-                existingBanner.getEndTime(),
-                existingBanner.isActive(),
-                existingBanner.getType());
+        try {
+            String uuid = existingBanner.getUuid();
+
+            return new BannerForm(uuid,
+                    existingBanner.getMessage(),
+                    existingBanner.getHosts(),
+                    existingBanner.getStartTime(),
+                    existingBanner.getEndTime(),
+                    existingBanner.isActive(),
+                    existingBanner.getType());
+        } catch (MissingUuidException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     public static BannerForm fromRequest(String uuid, HttpServletRequest request) {

pasystem/pasystem-tool/tool/src/java/org/sakaiproject/pasystem/tool/forms/PopupForm.java

@@ -33,6 +33,7 @@
 import org.apache.commons.fileupload.disk.DiskFileItem;
 import org.sakaiproject.pasystem.api.Errors;
 import org.sakaiproject.pasystem.api.PASystem;
+import org.sakaiproject.pasystem.api.MissingUuidException;
 import org.sakaiproject.pasystem.api.Popup;
 import org.sakaiproject.pasystem.api.TemplateStream;
 import org.sakaiproject.pasystem.tool.handlers.CrudHandler;
@@ -68,15 +69,19 @@ private PopupForm(String uuid,
     }
 
     public static PopupForm fromPopup(Popup existingPopup, PASystem paSystem) {
-        String uuid = existingPopup.getUuid();
-        List<String> assignees = paSystem.getPopups().getAssignees(uuid);
-
-        return new PopupForm(uuid, existingPopup.getDescriptor(),
-                existingPopup.getStartTime(),
-                existingPopup.getEndTime(),
-                existingPopup.isOpenCampaign(),
-                assignees,
-                Optional.empty());
+        try {
+            String uuid = existingPopup.getUuid();
+            List<String> assignees = paSystem.getPopups().getAssignees(uuid);
+
+            return new PopupForm(uuid, existingPopup.getDescriptor(),
+                    existingPopup.getStartTime(),
+                    existingPopup.getEndTime(),
+                    existingPopup.isOpenCampaign(),
+                    assignees,
+                    Optional.empty());
+        } catch (MissingUuidException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     public static PopupForm fromRequest(String uuid, HttpServletRequest request) {
@@ -141,6 +146,6 @@ public Optional<TemplateStream> getTemplateStream() {
     }
 
     public Popup toPopup() {
-        return Popup.create(descriptor, startTime, endTime, isOpenCampaign);
+        return Popup.create(uuid, descriptor, startTime, endTime, isOpenCampaign);
     }
 }