Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Unverified Commit ccea6cbd authored by Sunik Kupfer's avatar Sunik Kupfer Committed by GitHub
Browse files

validation and repairing (closes bitfireAT/ical4android#39, bitfireAT/davx5#8) (#45) 



* Drop RRULEs with UNTIL before DTSTART

Note: Validation/repair rules should be better formalized and modularized

* Tests show timezone problems

* validation and repairing (closes bitfireAT/ical4android#39, bitfireAT/davx5#8)

* As discussed in the call

* validation and repairing (closes bitfireAT/ical4android#39, bitfireAT/davx5#8)

* Move validator to validation package

* Minor reformatting

* use time from DTSTART in UNTIL instead of midnight

* merge remote changes

* drop ICalPreprocessor validation rule

* make EventValidator a class with only "repair()" as API and other methods internal

* validate each Event created from a VEvent before saving it to calendar provider, as well as each Event retrieved from calendar provider before creating a VEvent from it

* add tests to cover timezone cases

* stop use of strings and parsing for date building and add a until timezone in test

* [WIP] fix test with timezone offsets

* be more lenient with the result when repairing events where time is cut off

Co-authored-by: default avatarRicki Hirner <hirner@bitfire.at>
parent 41171b57

src/androidTest/java/at/bitfire/ical4android/AndroidEventTest.kt

+4 −8
Original line number Diff line number Diff line
@@ -10,9 +10,7 @@ import android.content.ContentUris 
import android.content.ContentValues

import android.database.DatabaseUtils

import android.net.Uri

import android.provider.CalendarContract

import android.provider.CalendarContract.*

import androidx.test.filters.LargeTest

import androidx.test.platform.app.InstrumentationRegistry.getInstrumentation

import androidx.test.rule.GrantPermissionRule

import at.bitfire.ical4android.MiscUtils.ContentProviderClientHelper.closeCompat
@@ -21,7 +19,6 @@ import at.bitfire.ical4android.impl.TestCalendar 
import at.bitfire.ical4android.impl.TestEvent

import at.bitfire.ical4android.util.AndroidTimeUtils

import net.fortuna.ical4j.model.*

import net.fortuna.ical4j.model.Date

import net.fortuna.ical4j.model.component.VAlarm

import net.fortuna.ical4j.model.parameter.*

import net.fortuna.ical4j.model.property.*
@@ -31,7 +28,6 @@ import org.junit.Assert.* 
import java.net.URI

import java.time.Duration

import java.time.Period

import java.util.*



class AndroidEventTest {
@@ -49,7 +45,7 @@ class AndroidEventTest { 
        @BeforeClass

        @JvmStatic

        fun connectProvider() {

            provider = getInstrumentation().targetContext.contentResolver.acquireContentProviderClient(CalendarContract.AUTHORITY)!!

            provider = getInstrumentation().targetContext.contentResolver.acquireContentProviderClient(AUTHORITY)!!

        }



        @AfterClass
@@ -60,7 +56,7 @@ class AndroidEventTest { 


    }



    private val testAccount = Account("ical4android@example.com", CalendarContract.ACCOUNT_TYPE_LOCAL)

    private val testAccount = Account("ical4android@example.com", ACCOUNT_TYPE_LOCAL)



    private val tzVienna = DateUtils.ical4jTimeZone("Europe/Vienna")!!

    private val tzShanghai = DateUtils.ical4jTimeZone("Asia/Shanghai")!!
@@ -75,7 +71,7 @@ class AndroidEventTest { 
    fun prepare() {

        calendar = TestCalendar.findOrCreate(testAccount, provider)

        assertNotNull(calendar)

        calendarUri = ContentUris.withAppendedId(CalendarContract.Calendars.CONTENT_URI, calendar.id)

        calendarUri = ContentUris.withAppendedId(Calendars.CONTENT_URI, calendar.id)

    }



    @After
@@ -1804,7 +1800,7 @@ class AndroidEventTest { 
    @Test

    fun testPopulateReminder_TypeEmail_AccountNameNotEmail() {

        // test account name that doesn't look like an email address

        val nonEmailAccount = Account("ical4android", CalendarContract.ACCOUNT_TYPE_LOCAL)

        val nonEmailAccount = Account("ical4android", ACCOUNT_TYPE_LOCAL)

        val testCalendar = TestCalendar.findOrCreate(nonEmailAccount, provider)

        try {

            populateReminder(testCalendar) {

src/main/java/at/bitfire/ical4android/AndroidEvent.kt

+4 −1
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate 
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalTime

import at.bitfire.ical4android.util.TimeApiExtensions.toRfc5545Duration

import at.bitfire.ical4android.util.TimeApiExtensions.toZonedDateTime

import at.bitfire.ical4android.validation.EventValidator

import net.fortuna.ical4j.model.*

import net.fortuna.ical4j.model.Date

import net.fortuna.ical4j.model.component.VAlarm
@@ -715,11 +716,12 @@ abstract class AndroidEvent( 


        val dtStart = event.dtStart ?: throw InvalidCalendarException("Events must have DTSTART")

        val allDay = DateUtils.isDate(dtStart)

        val recurring = event.rRules.isNotEmpty() || event.rDates.isNotEmpty()



        // make sure that time zone is supported by Android

        AndroidTimeUtils.androidifyTimeZone(dtStart)



        val recurring = event.rRules.isNotEmpty() || event.rDates.isNotEmpty()



        /* [CalendarContract.Events SDK documentation]

           When inserting a new event the following fields must be included:

           - dtstart
@@ -778,6 +780,7 @@ abstract class AndroidEvent( 
            builder .withValue(Events.DURATION, duration?.toRfc5545Duration(dtStart.date.toInstant()))

                    .withValue(Events.DTEND, null)



            // add RRULe

            if (event.rRules.isNotEmpty())

                builder.withValue(Events.RRULE, event.rRules.joinToString(AndroidTimeUtils.RECURRENCE_RULE_SEPARATOR) { it.value })

            else

src/main/java/at/bitfire/ical4android/Event.kt

+5 −7
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ package at.bitfire.ical4android 


import at.bitfire.ical4android.DateUtils.isDateTime

import at.bitfire.ical4android.ICalendar.Companion.CALENDAR_NAME

import at.bitfire.ical4android.validation.EventValidator

import net.fortuna.ical4j.data.CalendarOutputter

import net.fortuna.ical4j.data.ParserException

import net.fortuna.ical4j.model.*
@@ -194,13 +195,8 @@ class Event: ICalendar() { 


            e.alarms.addAll(event.alarms)



            // validation

            if (e.dtStart == null)

                throw InvalidCalendarException("Event without start time")

            else if (e.dtEnd != null && e.dtStart!!.date > e.dtEnd!!.date) {

                Ical4Android.log.warning("DTSTART after DTEND; removing DTEND")

                e.dtEnd = null

            }

            // validate and repair

            EventValidator(e).repair()



            return e

        }
@@ -221,6 +217,8 @@ class Event: ICalendar() { 


        val dtStart = dtStart ?: throw InvalidCalendarException("Won't generate event without start time")



        EventValidator(this).repair() // validate and repair this event before creating VEVENT



        // "main event" (without exceptions)

        val components = ical.components

        val mainEvent = toVEvent()

src/main/java/at/bitfire/ical4android/ICalendar.kt

+1 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ 


package at.bitfire.ical4android



import at.bitfire.ical4android.validation.ICalPreprocessor

import net.fortuna.ical4j.data.CalendarBuilder

import net.fortuna.ical4j.data.ParserException

import net.fortuna.ical4j.model.Calendar

src/main/java/at/bitfire/ical4android/validation/EventValidator.kt

0 → 100644
+107 −0
Original line number Diff line number Diff line 
/***************************************************************************************************

 * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.

 **************************************************************************************************/



package at.bitfire.ical4android.validation



import at.bitfire.ical4android.DateUtils

import at.bitfire.ical4android.Event

import at.bitfire.ical4android.Ical4Android

import at.bitfire.ical4android.InvalidCalendarException

import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate

import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDateTime

import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate

import at.bitfire.ical4android.util.TimeApiExtensions.toZoneIdCompat

import net.fortuna.ical4j.model.Date

import net.fortuna.ical4j.model.DateTime

import net.fortuna.ical4j.model.property.DtStart

import net.fortuna.ical4j.model.property.RRule

import java.time.*



/**

 * Sometimes CalendarStorage or servers respond with invalid event definitions. Here we try to

 * validate, repair and assume whatever seems appropriate before denying the whole event.

 */

class EventValidator(val e: Event) {



    fun repair() {

        val dtStart = correctStartAndEndTime(e)

        sameTypeForDtStartAndRruleUntil(dtStart, e.rRules)

        removeRRulesWithUntilBeforeDtStart(dtStart, e.rRules)

    }



    companion object {

        /**

         * Ensure proper start and end time

         */

        internal fun correctStartAndEndTime(e: Event): DtStart {

            val dtStart = e.dtStart ?: throw InvalidCalendarException("Event without start time")

            e.dtEnd?.let { dtEnd ->

                if (dtStart.date > dtEnd.date) {

                    Ical4Android.log.warning("DTSTART after DTEND; removing DTEND")

                    e.dtEnd = null

                }

            }

            return dtStart

        }



        /**

         * Tries to make the value type of UNTIL and DTSTART the same (both DATE or DATETIME).

         */

        internal fun sameTypeForDtStartAndRruleUntil(dtStart: DtStart, rRules: MutableList<RRule>) {

            if (DateUtils.isDate(dtStart)) {

                for (rRule in rRules) {

                    rRule.recur.until?.let { until ->

                        if (until is DateTime) {

                            Ical4Android.log.warning("DTSTART has DATE, but UNTIL has DATETIME; making UNTIL have DATE only")

                            rRule.recur.until = until.toLocalDate().toIcal4jDate()

                        }

                    }

                }

            } else if (DateUtils.isDateTime(dtStart)) {

                for (rRule in rRules) {

                    rRule.recur.until?.let { until ->

                        if (until !is DateTime) {

                            Ical4Android.log.warning("DTSTART has DATETIME, but UNTIL has DATE; copying time from DTSTART to UNTIL")

                            val timeZone = if (dtStart.timeZone != null)

                                dtStart.timeZone.toZoneIdCompat()

                            else if (dtStart.isUtc)

                                ZoneOffset.UTC

                            else /* floating time */

                                ZoneId.systemDefault()

                            rRule.recur.until =

                                ZonedDateTime.of(

                                    until.toLocalDate(),                                        // date from until

                                    LocalTime.ofInstant(dtStart.date.toInstant(), timeZone),    // time from dtStart

                                    timeZone

                                ).toIcal4jDateTime()

                        }

                    }

                }

            } else

                throw InvalidCalendarException("Event with invalid DTSTART value")

        }



        /**

         * Will remove the RRULES of an event where UNTIL lies before DTSTART

         */

        internal fun removeRRulesWithUntilBeforeDtStart(dtStart: DtStart, rRules: MutableList<RRule>) {

            val iter = rRules.iterator()

            while (iter.hasNext()) {

                val rRule = iter.next()



                // drop invalid RRULEs

                if (hasUntilBeforeDtStart(dtStart, rRule))

                    iter.remove()

            }

        }



        /**

         * Checks whether UNTIL of an RRULE lies before DTSTART

         */

        internal fun hasUntilBeforeDtStart(dtStart: DtStart, rRule: RRule): Boolean {

            val until = rRule.recur.until ?: return false

            return until < dtStart.date

        }

    }

}
 
 No newline at end of file