SAK-31878 still send the mail even if ical4j cant create an ics file (s…

…akaiproject#3506)

* SAK-31878 still send the mail even if ical4j cant create an ics file

* SAK-31878 use a Set instead of a List for unique users

* SAK-31878 still send the mail even if ical4j cant create an ics file

external-calendaring-service/api/src/java/org/sakaiproject/calendaring/api/ExternalCalendaringService.java

@@ -20,6 +20,7 @@
 package org.sakaiproject.calendaring.api;
 
 import java.util.List;
+import java.util.Set;
 
 import net.fortuna.ical4j.model.Calendar;
 import net.fortuna.ical4j.model.component.VEvent;
@@ -61,30 +62,30 @@ public interface ExternalCalendaringService {
 	 * <br>If the CalendarEvent has the field 'vevent_url', that will be added to the URL property of the VEvent.
 	 * 
 	 * @param event Sakai CalendarEvent
-	 * @param attendees list of Users that have been invited to the event
+	 * @param attendees set of Users that have been invited to the event
 	 * @return the VEvent for the given event or null if there was an error
 	 */
-	public VEvent createEvent(CalendarEvent event, List<User> attendees);
+	public VEvent createEvent(CalendarEvent event, Set<User> attendees);
 
 	/**
 	 * Adds a list of attendees to an existing VEvent.
 	 * This must then be turned into a Calendar before it can be turned into an ICS file. 
 	 * 
 	 * @param vevent  The VEvent to add the attendess too
-	 * @param attendees list of Users that have been invited to the event
+	 * @param attendees set of Users that have been invited to the event
 	 * @return the VEvent for the given event or null if there was an error
 	 */
-	public VEvent addAttendeesToEvent(VEvent vevent, List<User> attendees);
+	public VEvent addAttendeesToEvent(VEvent vevent, Set<User> attendees);
 
 	/**
 	 * Adds a list of attendees to an existing VEvent with the chair role.
 	 * This must then be turned into a Calendar before it can be turned into an ICS file.
 	 *
 	 * @param vevent  The VEvent to add the attendess too
-	 * @param attendees list of Users that will chair the event
+	 * @param attendees set of Users that will chair the event
 	 * @return the VEvent for the given event or null if there was an error
 	 */
-	public VEvent addChairAttendeesToEvent(VEvent vevent, List<User> attendees);
+	public VEvent addChairAttendeesToEvent(VEvent vevent, Set<User> attendees);
 
 	/**
 	 * Set the status of an existing VEvent to cancelled.

external-calendaring-service/impl/src/java/org/sakaiproject/calendaring/api/ExternalCalendaringServiceImpl.java

@@ -27,6 +27,7 @@
 import java.net.URISyntaxException;
 import java.util.GregorianCalendar;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 
 import lombok.Setter;
@@ -73,7 +74,7 @@ public VEvent createEvent(CalendarEvent event) {
 	/**
 	 * {@inheritDoc}
 	 */
-	public VEvent createEvent(CalendarEvent event, List<User> attendees) {
+	public VEvent createEvent(CalendarEvent event, Set<User> attendees) {
 
 		if(!isIcsEnabled()) {
 			log.debug("ExternalCalendaringService is disabled. Enable via calendar.ics.generation.enabled=true in sakai.properties");
@@ -158,15 +159,15 @@ public VEvent createEvent(CalendarEvent event, List<User> attendees) {
 	/**
 	 * {@inheritDoc}
 	 */
-	public VEvent addAttendeesToEvent(VEvent vevent, List<User> attendees) {
+	public VEvent addAttendeesToEvent(VEvent vevent, Set<User> attendees) {
 		return addAttendeesToEventWithRole(vevent, attendees, Role.REQ_PARTICIPANT);
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	@Override
-	public VEvent addChairAttendeesToEvent(VEvent vevent, List<User> attendees) {
+	public VEvent addChairAttendeesToEvent(VEvent vevent, Set<User> attendees) {
 		return addAttendeesToEventWithRole(vevent, attendees, Role.CHAIR);
 	}
 
@@ -179,7 +180,7 @@ public VEvent addChairAttendeesToEvent(VEvent vevent, List<User> attendees) {
 	 * @param role      the role with which to add each user
 	 * @return          the VEvent for the given event or null if there was an error
 	 */
-	protected VEvent addAttendeesToEventWithRole(VEvent vevent, List<User> attendees, Role role) {
+	protected VEvent addAttendeesToEventWithRole(VEvent vevent, Set<User> attendees, Role role) {
 
 		if(!isIcsEnabled()) {
 			log.debug("ExternalCalendaringService is disabled. Enable via calendar.ics.generation.enabled=true in sakai.properties");
@@ -266,7 +267,7 @@ public Calendar createCalendar(List<VEvent> events, String method) {
 		try {
 			calendar.validate(true);
 		} catch (ValidationException e) {
-			e.printStackTrace();
+			log.error("createCalendar failed validation", e);
 			return null;
 		}
 

external-calendaring-service/impl/src/test/org/sakaiproject/calendaring/ExternalCalendaringServiceTest.java

@@ -22,7 +22,9 @@
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.UUID;
 
 import javax.annotation.Resource;
@@ -80,7 +82,7 @@ public class ExternalCalendaringServiceTest {
 	private ApplicationContext applicationContext;
 
 
-	private List<User> users;
+	private Set<User> users;
 
 
 	@Before
@@ -502,8 +504,8 @@ private CalendarEventEdit generateEvent() {
 	 * @throws UserAlreadyDefinedException 
 	 * @throws UserIdInvalidException 
 	 */
-	private List<User> generateUsers(){
-		List<User> users = new ArrayList<User>();
+	private Set<User> generateUsers(){
+		Set<User> users = new HashSet<User>();
 
 		for(int i=0;i<5;i++) {
 			User u = mock(User.class);

signup/api/src/java/org/sakaiproject/signup/logic/SignupCalendarHelper.java

@@ -20,6 +20,7 @@
 package org.sakaiproject.signup.logic;
 
 import java.util.List;
+import java.util.Set;
 
 import net.fortuna.ical4j.model.component.VEvent;
 
@@ -95,15 +96,15 @@ public interface SignupCalendarHelper {
 	 * @param users		List of Users to add
 	 * @return
 	 */
-	public VEvent addUsersToVEvent(VEvent vevent, List<User> users);
+	public VEvent addUsersToVEvent(VEvent vevent, Set<User> users);
 
 	/**
 	 * Add the list of SignupAttendees to the VEvent as attendees
 	 * @param vevent	VEvent to modify
 	 * @param attendees	List of Attendees to add
 	 * @return
 	 */
-	public VEvent addAttendeesToVEvent(VEvent vevent, List<SignupAttendee> attendees);
+	public VEvent addAttendeesToVEvent(VEvent vevent, Set<SignupAttendee> attendees);
 
 	/**
 	 * Is ICS calendar generation enabled in the external calendaring service?

signup/impl/src/java/org/sakaiproject/signup/logic/SignupCalendarHelperImpl.java

@@ -19,9 +19,10 @@
 
 package org.sakaiproject.signup.logic;
 
-import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
@@ -163,14 +164,14 @@ public VEvent generateVEventForMeeting(SignupMeeting meeting) {
 	}
 
 	/**
-	 * Helper to get a list of Users who are coordinates for a given meeting
+	 * Helper to get a set of Users who are coordinates for a given meeting
 	 *     meeting.coordinatorIds.map(userDirectoryService.getUser)
 	 *
 	 * @param meeting  the meeting in question
 	 * @return the list of coordinator Users
 	 */
-	private List<User> getCoordinators(SignupMeeting meeting) {
-		List<User> users = new ArrayList<User>();
+	private Set<User> getCoordinators(SignupMeeting meeting) {
+		Set<User> users = new HashSet<User>();
 		List<String> ids = meeting.getCoordinatorIdsList();
 		for (String coordinator : ids) {
 			users.add(sakaiFacade.getUserQuietly(coordinator));
@@ -201,12 +202,12 @@ public VEvent cancelVEvent(VEvent vevent) {
 		return externalCalendaringService.cancelEvent(vevent);
 	}
 
-	public VEvent addUsersToVEvent(VEvent vevent, List<User> users) {
+	public VEvent addUsersToVEvent(VEvent vevent, Set<User> users) {
 		return externalCalendaringService.addAttendeesToEvent(vevent, users);
 	}
 
-	public VEvent addAttendeesToVEvent(VEvent vevent, List<SignupAttendee> attendees) {
-        List<User> users = new ArrayList<User>();
+	public VEvent addAttendeesToVEvent(VEvent vevent, Set<SignupAttendee> attendees) {
+        Set<User> users = new HashSet<User>();
         for (SignupAttendee attendee : attendees) {
             User user = sakaiFacade.getUser(attendee.getAttendeeUserId());
             if (user != null) {

signup/impl/src/java/org/sakaiproject/signup/logic/SignupEmailFacadeImpl.java

@@ -543,7 +543,7 @@ private void sendEmailToAllUsers(SignupMeeting meeting, String messageType) thro
 				logger.warn("User is not found for userId: " + meeting.getCreatorUserId());
 			} catch (Exception e) {
 				isException = true;
-				logger.error("Exception: " + e.getClass() + ": " + e.getMessage());
+				logger.error("Exception: " + e.getClass() + ": " + e.getMessage(), e);
 			}
 		}
 
@@ -765,7 +765,10 @@ private List<Attachment> generateICS(SignupEmailNotification email, User user) {
 		final List<VEvent> events = email.generateEvents(user, calendarHelper);
 
 		if (events.size() > 0) {
-			attachments.add(formatICSAttachment(events, method));
+			Attachment a = formatICSAttachment(events, method);
+			if (a != null) {
+				attachments.add(a);
+			}
 		}
 
 		/*
@@ -806,6 +809,10 @@ private SignupMeeting generateVEvents(SignupMeeting meeting) {
 	 */
 	private Attachment formatICSAttachment(List<VEvent> vevents, String method) {
 		String path = calendarHelper.createCalendarFile(vevents, method);
+		// It's possible that ICS creation failed
+		if (StringUtils.isBlank(path)) {
+			return null;
+		}
 
 		// Explicitly define the Content-Type and Content-Diposition headers so the invitation appears inline
 		String filename = StringUtils.substringAfterLast(path, File.separator);

signup/impl/src/java/org/sakaiproject/signup/logic/messages/AttendeeEmailBase.java

@@ -23,7 +23,7 @@ public List<VEvent> generateEvents(User user, SignupCalendarHelper calendarHelpe
         events.addAll(eventsWhichUserIsAttending(user));
 
         for (VEvent event : events) {
-            calendarHelper.addUsersToVEvent(event, Collections.singletonList(user));
+            calendarHelper.addUsersToVEvent(event, Collections.singleton(user));
         }
 
         return events;

signup/impl/src/java/org/sakaiproject/signup/logic/messages/OrganizerEmailBase.java

@@ -7,7 +7,9 @@
 import org.sakaiproject.user.api.User;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 
 /**
@@ -27,7 +29,7 @@ public List<VEvent> generateEvents(User user, SignupCalendarHelper calendarHelpe
             return events;
         }
 
-        List<SignupAttendee> attendees = new ArrayList<SignupAttendee>();
+        Set<SignupAttendee> attendees = new HashSet<SignupAttendee>();
         for(SignupTimeslot timeslot: meeting.getSignupTimeSlots()) {
             attendees.addAll(timeslot.getAttendees());
         }

signup/impl/src/test/org/sakaiproject/signup/logic/messages/AttendeeEmailTest.java

@@ -15,6 +15,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
@@ -93,7 +94,7 @@ public void attendeeSignupOwnEmailTest() {
     private void assertGenerates(int n) {
         final List<VEvent> events = _email.generateEvents(_mockedAttendingUser, _mockedCalendarHelper);
         assertEquals(n, events.size());
-        verify(_mockedCalendarHelper, times(n)).addUsersToVEvent(eq(_mockedEvent), any(List.class));
+        verify(_mockedCalendarHelper, times(n)).addUsersToVEvent(eq(_mockedEvent), any(Set.class));
     }
 
     private void userIsAttendingTimeslot(String userId, SignupTimeslot mockedTimeslot) {