Better DTSTART/DTEND handling

* generalized InvalidResourceException for parsing errors
* only iCals with both DtStart and DtEnd/Duration are processed (DtEnd will be derived by iCal4j when not present in .ics)
* all-day events must last at least one day (fixes #166)
* other DtEnd/Duration rewriting + tests

src/at/bitfire/davdroid/resource/Event.java

@@ -122,11 +122,17 @@ public class Event extends Resource {
 
 	@Override
 	@SuppressWarnings("unchecked")
-	public void parseEntity(@NonNull InputStream entity) throws IOException, ParserException {
+	public void parseEntity(@NonNull InputStream entity) throws IOException, InvalidResourceException {
-		CalendarBuilder builder = new CalendarBuilder();
+		net.fortuna.ical4j.model.Calendar ical;
-		net.fortuna.ical4j.model.Calendar ical = builder.build(entity);
+		try {
-		if (ical == null)
+			CalendarBuilder builder = new CalendarBuilder();
-			return;
+			ical = builder.build(entity);
+
+			if (ical == null)
+				throw new InvalidResourceException("No iCalendar found");
+		} catch (ParserException e) {
+			throw new InvalidResourceException(e);
+		}
 		
 		// event
 		ComponentList events = ical.getComponents(Component.VEVENT);
@@ -141,10 +147,25 @@ public class Event extends Resource {
 			generateUID();
 		}
 		
-		dtStart = event.getStartDate();	validateTimeZone(dtStart);
+		if ((dtStart = event.getStartDate()) == null || (dtEnd = event.getEndDate()) == null)
-		dtEnd = event.getEndDate(); validateTimeZone(dtEnd);
+			throw new InvalidResourceException("Invalid start time/end time/duration");
+
+		if (hasTime(dtStart)) {
+			validateTimeZone(dtStart);
+			validateTimeZone(dtEnd);
+		}
+		
+		// all-day events and "events on that day":
+		// * related UNIX times must be in UTC
+		// * must have a duration (set to one day if missing)
+		if (!hasTime(dtStart) && !dtEnd.getDate().after(dtStart.getDate())) {
+			Log.i(TAG, "Repairing iCal: DTEND := DTSTART+1");
+			Calendar c = Calendar.getInstance(TimeZone.getTimeZone(Time.TIMEZONE_UTC));
+			c.setTime(dtStart.getDate());
+			c.add(Calendar.DATE, 1);
+			dtEnd.setDate(new Date(c.getTimeInMillis()));
+		}
 		
-		duration = event.getDuration();
 		rrule = (RRule)event.getProperty(Property.RRULE);
 		rdate = (RDate)event.getProperty(Property.RDATE);
 		exrule = (ExRule)event.getProperty(Property.EXRULE);
@@ -180,7 +201,7 @@ public class Event extends Resource {
 
 	@Override
 	@SuppressWarnings("unchecked")
-	public ByteArrayOutputStream toEntity() throws IOException, ValidationException {
+	public ByteArrayOutputStream toEntity() throws IOException {
 		net.fortuna.ical4j.model.Calendar ical = new net.fortuna.ical4j.model.Calendar();
 		ical.getProperties().add(Version.VERSION_2_0);
 		ical.getProperties().add(new ProdId("-//bitfire web engineering//DAVdroid " + Constants.APP_VERSION + "//EN"));
@@ -241,13 +262,17 @@ public class Event extends Resource {
 
 		CalendarOutputter output = new CalendarOutputter(false);
 		ByteArrayOutputStream os = new ByteArrayOutputStream();
-		output.output(ical, os);
+		try {
+			output.output(ical, os);
+		} catch (ValidationException e) {
+			Log.e(TAG, "Generated invalid iCalendar");
+		}
 		return os;
 	}
 
 	
 	public long getDtStartInMillis() {
-		return (dtStart != null && dtStart.getDate() != null) ? dtStart.getDate().getTime() : 0;
+		return dtStart.getDate().getTime();
 	}
 	
 	public String getDtStartTzID() {
@@ -265,18 +290,7 @@ public class Event extends Resource {
 	}
 	
 	
-	public Long getDtEndInMillis() {
+	public long getDtEndInMillis() {
-		if (hasNoTime(dtStart) && dtEnd == null) {		// "event on that day"
-			// dtEnd = dtStart + 1 day
-			Calendar c = Calendar.getInstance(TimeZone.getTimeZone(Time.TIMEZONE_UTC));
-			c.setTime(dtStart.getDate());
-			c.add(Calendar.DATE, 1);
-			return c.getTimeInMillis();
-			
-		} else if (dtEnd == null || dtEnd.getDate() == null) {	// no DTEND provided (maybe DURATION instead)
-			return null;
-		}
-		
 		return dtEnd.getDate().getTime();
 	}
 	
@@ -298,40 +312,28 @@ public class Event extends Resource {
 	// helpers
 	
 	public boolean isAllDay() {
-		if (hasNoTime(dtStart)) {
+		return !hasTime(dtStart);
-			// events on that day
-			if (dtEnd == null)
-				return true;
-			
-			// all-day events
-			if (hasNoTime(dtEnd))
-				return true;
-		}
-		return false;
 	}
 
-	protected static boolean hasNoTime(DateProperty date) {
+	protected static boolean hasTime(DateProperty date) {
-		if (date == null)
+		return date.getDate() instanceof DateTime;
-			return false;
-		return !(date.getDate() instanceof DateTime);
 	}
 
 	protected static String getTzId(DateProperty date) {
-		if (date == null)
+		if (date.isUtc() || !hasTime(date))
-			return null;
-		
-		if (hasNoTime(date) || date.isUtc())
 			return Time.TIMEZONE_UTC;
 		else if (date.getTimeZone() != null)
 			return date.getTimeZone().getID();
 		else if (date.getParameter(Value.TZID) != null)
 			return date.getParameter(Value.TZID).getValue();
-		return null;
+		
+		// fallback
+		return Time.TIMEZONE_UTC;
 	}
 
 	/* guess matching Android timezone ID */
 	protected static void validateTimeZone(DateProperty date) {
-		if (date == null || date.isUtc() || hasNoTime(date))
+		if (date.isUtc() || !hasTime(date))
 			return;
 		
 		String tzID = getTzId(date);

src/at/bitfire/davdroid/resource/InvalidResourceException.java

@@ -0,0 +1,13 @@
+package at.bitfire.davdroid.resource;
+
+public class InvalidResourceException extends Exception {
+	private static final long serialVersionUID = 1593585432655578220L;
+	
+	public InvalidResourceException(String message) {
+		super(message);
+	}
+
+	public InvalidResourceException(Throwable throwable) {
+		super(throwable);
+	}
+}

src/at/bitfire/davdroid/resource/LocalCalendar.java

@@ -13,7 +13,6 @@ package at.bitfire.davdroid.resource;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.text.ParseException;
-import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.regex.Matcher;
@@ -221,29 +220,25 @@ public class LocalCalendar extends LocalCollection<Event> {
 				e.setLocation(cursor.getString(1));
 				e.setDescription(cursor.getString(2));
 				
+				boolean allDay = cursor.getInt(7) != 0;
 				long tsStart = cursor.getLong(3),
 					 tsEnd = cursor.getLong(4);
+				String duration = cursor.getString(18);
 				
-				String tzId;
+				String tzId = null;
-				if (cursor.getInt(7) != 0) {	// ALL_DAY != 0
+				if (!allDay) {
-					tzId = null;				// -> use UTC
-				} else {
 					// use the start time zone for the end time, too
-					// because the Samsung Planner UI allows the user to change the time zone
+					// because apps like Samsung Planner allow the user to change "the" time zone but change the start time zone only
-					// but it will change the start time zone only
 					tzId = cursor.getString(5);
-					//tzIdEnd = cursor.getString(6);
 				}
 				e.setDtStart(tsStart, tzId);
 				if (tsEnd != 0)
 					e.setDtEnd(tsEnd, tzId);
-	
+				else if (!StringUtils.isEmpty(duration))
+					e.setDuration(new Duration(new Dur(duration)));
+					
 				// recurrence
 				try {
-					String duration = cursor.getString(18);
-					if (!StringUtils.isEmpty(duration))
-						e.setDuration(new Duration(new Dur(duration)));
-					
 					String strRRule = cursor.getString(10);
 					if (!StringUtils.isEmpty(strRRule))
 						e.setRrule(new RRule(strRRule));
@@ -436,30 +431,16 @@ public class LocalCalendar extends LocalCollection<Event> {
 		if (event.getExdate() != null)
 			builder = builder.withValue(Events.EXDATE, event.getExdate().getValue());
 		
-		// set DTEND for single-time events or DURATION for recurring events
+		// set either DTEND for single-time events or DURATION for recurring events
-		// because that's the way Android likes it
+		// because that's the way Android likes it (see docs)
-		if (!recurring) {
+		if (recurring) {
-			// not recurring: set DTEND
+			// calculate DURATION from start and end date
-			long dtEnd = 0;
+			Duration duration = new Duration(event.getDtStart().getDate(), event.getDtEnd().getDate());
-			String tzEnd = null;
+			builder = builder.withValue(Events.DURATION, duration.getValue());
-			if (event.getDtEndInMillis() != null) {
-				dtEnd = event.getDtEndInMillis();
-				tzEnd = event.getDtEndTzID();
-			} else if (event.getDuration() != null) {
-				Date dateEnd = event.getDuration().getDuration().getTime(event.getDtStart().getDate());
-				dtEnd = dateEnd.getTime();
-			}
-			builder = builder
-					.withValue(Events.DTEND, dtEnd)
-					.withValue(Events.EVENT_END_TIMEZONE, tzEnd);
 		} else {
-			// recurring: set DURATION
+			builder = builder
-			String duration = null;
+					.withValue(Events.DTEND, event.getDtEndInMillis())
-			if (event.getDuration() != null)
+					.withValue(Events.EVENT_END_TIMEZONE, event.getDtEndTzID());
-				duration = event.getDuration().getValue();
-			else if (event.getDtEnd() != null)
-				duration = new Duration(event.getDtStart().getDate(), event.getDtEnd().getDate()).getValue();
-			builder = builder.withValue(Events.DURATION, duration);
 		}
 		
 		if (event.getSummary() != null)

src/at/bitfire/davdroid/resource/RemoteCollection.java

@@ -21,7 +21,6 @@ import java.util.List;
 
 import lombok.Cleanup;
 import lombok.Getter;
-import net.fortuna.ical4j.data.ParserException;
 import net.fortuna.ical4j.model.ValidationException;
 
 import org.apache.http.HttpException;
@@ -33,7 +32,6 @@ import at.bitfire.davdroid.webdav.DavNoContentException;
 import at.bitfire.davdroid.webdav.HttpPropfind;
 import at.bitfire.davdroid.webdav.WebDavResource;
 import at.bitfire.davdroid.webdav.WebDavResource.PutMode;
-import ezvcard.VCardException;
 
 public abstract class RemoteCollection<T extends Resource> {
 	private static final String TAG = "davdroid.RemoteCollection";
@@ -98,18 +96,14 @@ public abstract class RemoteCollection<T extends Resource> {
 						foundResources.add(resource);
 					} else
 						Log.e(TAG, "Ignoring entity without content");
-				} catch (ParserException ex) {
+				} catch (InvalidResourceException e) {
-					Log.e(TAG, "Ignoring unparseable iCal in multi-response", ex);
+					Log.e(TAG, "Ignoring unparseable entity in multi-response", e);
-				} catch (VCardException ex) {
-					Log.e(TAG, "Ignoring unparseable vCard in multi-response", ex);
 				}
 			}
 			
 			return foundResources.toArray(new Resource[0]);
-		} catch (ParserException ex) {
+		} catch (InvalidResourceException e) {
-			Log.e(TAG, "Couldn't parse iCal from GET", ex);
+			Log.e(TAG, "Couldn't parse entity from GET", e);
-		} catch (VCardException ex) {
-			Log.e(TAG, "Couldn't parse vCard from GET", ex);
 		}
 		
 		return new Resource[0];
@@ -118,7 +112,7 @@ public abstract class RemoteCollection<T extends Resource> {
 	
 	/* internal member operations */
 
-	public Resource get(Resource resource) throws IOException, HttpException, ParserException, VCardException {
+	public Resource get(Resource resource) throws IOException, HttpException, InvalidResourceException {
 		WebDavResource member = new WebDavResource(collection, resource.getName());
 		member.get();
 		

src/at/bitfire/davdroid/resource/Resource.java

@@ -17,9 +17,6 @@ import java.io.InputStream;
 import lombok.Getter;
 import lombok.Setter;
 import lombok.ToString;
-import net.fortuna.ical4j.data.ParserException;
-import net.fortuna.ical4j.model.ValidationException;
-import ezvcard.VCardException;
 
 @ToString
 public abstract class Resource {
@@ -42,6 +39,6 @@ public abstract class Resource {
 	public abstract void generateUID();
 	public abstract void generateName();
 	
-	public abstract void parseEntity(InputStream entity) throws IOException, ParserException, VCardException;
+	public abstract void parseEntity(InputStream entity) throws IOException, InvalidResourceException;
-	public abstract ByteArrayOutputStream toEntity() throws IOException, ValidationException;
+	public abstract ByteArrayOutputStream toEntity() throws IOException;
 }

src/at/bitfire/davdroid/syncadapter/CalendarsSyncAdapterService.java

@@ -57,7 +57,8 @@ public class CalendarsSyncAdapterService extends Service {
 				Map<LocalCollection<?>, RemoteCollection<?>> map = new HashMap<LocalCollection<?>, RemoteCollection<?>>();
 				
 				for (LocalCalendar calendar : LocalCalendar.findAll(account, provider)) {
-					URI uri = new URI(accountManager.getUserData(account, Constants.ACCOUNT_KEY_BASE_URL)).resolve(calendar.getPath());
+					URI baseURI = new URI(accountManager.getUserData(account, Constants.ACCOUNT_KEY_BASE_URL));
+					URI uri = baseURI.resolve(calendar.getPath());
 					RemoteCollection<?> dav = new CalDavCalendar(uri.toString(),
 						accountManager.getUserData(account, Constants.ACCOUNT_KEY_USERNAME),
 						accountManager.getPassword(account),

test/assets/all-day-0sec.ics

@@ -0,0 +1,11 @@
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//hacksw/handcal//NONSGML v1.0//EN
+BEGIN:VEVENT
+UID:all-day-0sec@example.com
+DTSTAMP:20140101T000000Z
+DTSTART;VALUE=DATE:19970714
+DTEND;VALUE=DATE:19970714
+SUMMARY:0 Sec Event
+END:VEVENT
+END:VCALENDAR

test/assets/all-day-10days.ics

@@ -0,0 +1,11 @@
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//hacksw/handcal//NONSGML v1.0//EN
+BEGIN:VEVENT
+UID:all-day-10days@example.com
+DTSTAMP:20140101T000000Z
+DTSTART;VALUE=DATE:19970714
+DTEND;VALUE=DATE:19970724
+SUMMARY:All-Day 10 Days
+END:VEVENT
+END:VCALENDAR

test/assets/all-day-1day.ics

@@ -0,0 +1,11 @@
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//hacksw/handcal//NONSGML v1.0//EN
+BEGIN:VEVENT
+UID:all-day-1day@example.com
+DTSTAMP:20140101T000000Z
+DTSTART;VALUE=DATE:19970714
+DTEND;VALUE=DATE:19970714
+SUMMARY:All-Day 1 Day
+END:VEVENT
+END:VCALENDAR

test/assets/event-on-that-day.ics

@@ -0,0 +1,11 @@
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//hacksw/handcal//NONSGML v1.0//EN
+BEGIN:VEVENT
+UID:event-on-that-day@example.com
+DTSTAMP:19970714T170000Z
+ORGANIZER;CN=John Doe:MAILTO:john.doe@example.com
+DTSTART;VALUE=DATE:19970714
+SUMMARY:Bastille Day Party
+END:VEVENT
+END:VCALENDAR

test/src/at/bitfire/davdroid/resource/test/EventTest.java

@@ -0,0 +1,83 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Richard Hirner (bitfire web engineering).
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the GNU Public License v3.0
+ * which accompanies this distribution, and is available at
+ * http://www.gnu.org/licenses/gpl.html
+ ******************************************************************************/
+package at.bitfire.davdroid.resource.test;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import lombok.Cleanup;
+import net.fortuna.ical4j.data.ParserException;
+import android.content.res.AssetManager;
+import android.test.InstrumentationTestCase;
+import android.text.format.Time;
+import at.bitfire.davdroid.resource.Event;
+import at.bitfire.davdroid.resource.InvalidResourceException;
+
+public class EventTest extends InstrumentationTestCase {
+	AssetManager assetMgr;
+	
+	Event	eViennaEvolution,
+			eOnThatDay, eAllDay1Day, eAllDay10Days, eAllDay0Sec;
+	
+	public void setUp() throws IOException, InvalidResourceException {
+		assetMgr = getInstrumentation().getContext().getResources().getAssets();
+		
+		eViennaEvolution = parseCalendar("vienna-evolution.ics");
+		eOnThatDay = parseCalendar("event-on-that-day.ics");
+		eAllDay1Day = parseCalendar("all-day-1day.ics");
+		eAllDay10Days = parseCalendar("all-day-10days.ics");
+		eAllDay0Sec = parseCalendar("all-day-0sec.ics");
+		
+		//assertEquals("Test-Ereignis im schönen Wien", e.getSummary());
+	}
+	
+	
+	public void testStartEndTimes() throws IOException, ParserException {
+		// event with start+end date-time
+		assertEquals(1381330800000L, eViennaEvolution.getDtStartInMillis());
+		assertEquals("Europe/Vienna", eViennaEvolution.getDtStartTzID());
+		assertEquals(1381334400000L, eViennaEvolution.getDtEndInMillis());
+		assertEquals("Europe/Vienna", eViennaEvolution.getDtEndTzID());
+	}
+	
+	public void testStartEndTimesAllDay() throws IOException, ParserException {
+		// event with start date only
+		assertEquals(868838400000L, eOnThatDay.getDtStartInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eOnThatDay.getDtStartTzID());
+		// DTEND missing in VEVENT, must have been set to DTSTART+1 day
+		assertEquals(868838400000L + 86400000, eOnThatDay.getDtEndInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eOnThatDay.getDtEndTzID());
+		
+		// event with start+end date for all-day event (one day)
+		assertEquals(868838400000L, eAllDay1Day.getDtStartInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay1Day.getDtStartTzID());
+		assertEquals(868838400000L + 86400000, eAllDay1Day.getDtEndInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay1Day.getDtEndTzID());
+		
+		// event with start+end date for all-day event (ten days)
+		assertEquals(868838400000L, eAllDay10Days.getDtStartInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay10Days.getDtStartTzID());
+		assertEquals(868838400000L + 10*86400000, eAllDay10Days.getDtEndInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay10Days.getDtEndTzID());
+		
+		// event with start+end date on some day (invalid 0 sec-event)
+		assertEquals(868838400000L, eAllDay0Sec.getDtStartInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay0Sec.getDtStartTzID());
+		// DTEND invalid in VEVENT, must have been set to DTSTART+1 day
+		assertEquals(868838400000L + 86400000, eAllDay0Sec.getDtEndInMillis());
+		assertEquals(Time.TIMEZONE_UTC, eAllDay0Sec.getDtEndTzID());
+	}
+	
+	
+	protected Event parseCalendar(String fname) throws IOException, InvalidResourceException {
+		@Cleanup InputStream in = assetMgr.open(fname, AssetManager.ACCESS_STREAMING);
+		Event e = new Event(fname, null);
+		e.parseEntity(in);
+		return e;
+	}
+}

test/src/at/bitfire/davdroid/test/CalendarTest.java

@@ -1,42 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2013 Richard Hirner (bitfire web engineering).
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the GNU Public License v3.0
- * which accompanies this distribution, and is available at
- * http://www.gnu.org/licenses/gpl.html
- ******************************************************************************/
-package at.bitfire.davdroid.test;
-
-import java.io.IOException;
-import java.io.InputStream;
-
-import net.fortuna.ical4j.data.ParserException;
-import android.content.res.AssetManager;
-import android.test.InstrumentationTestCase;
-import at.bitfire.davdroid.resource.Event;
-
-public class CalendarTest extends InstrumentationTestCase {
-	AssetManager assetMgr;
-	
-	public void setUp() {
-		assetMgr = getInstrumentation().getContext().getResources().getAssets();
-	}
-	
-	
-	public void testTimeZonesByEvolution() throws IOException, ParserException {
-		Event e = parseCalendar("vienna-evolution.ics");
-		assertEquals("Test-Ereignis im schönen Wien", e.getSummary());		
-		
-		//DTSTART;TZID=/freeassociation.sourceforge.net/Tzfile/Europe/Vienna:20131009T170000
-		/*assertEquals(1381330800000L, e.getDtStartInMillis());
-		assertEquals(1381334400000L, (long)e.getDtEndInMillis());*/
-	}
-	
-	
-	protected Event parseCalendar(String fname) throws IOException, ParserException {
-		InputStream in = assetMgr.open(fname, AssetManager.ACCESS_STREAMING);
-		Event e = new Event(fname, null);
-		e.parseEntity(in);
-		return e;
-	}
-}