Improve null-safety in Factories and cleanup

src/main/kotlin/at/bitfire/dav4jvm/DavResource.kt

@@ -8,24 +8,13 @@ package at.bitfire.dav4jvm
 
 import at.bitfire.dav4jvm.XmlUtils.insertTag
 import at.bitfire.dav4jvm.XmlUtils.propertyName
-import at.bitfire.dav4jvm.exception.ConflictException
-import at.bitfire.dav4jvm.exception.DavException
-import at.bitfire.dav4jvm.exception.ForbiddenException
-import at.bitfire.dav4jvm.exception.HttpException
-import at.bitfire.dav4jvm.exception.NotFoundException
-import at.bitfire.dav4jvm.exception.PreconditionFailedException
-import at.bitfire.dav4jvm.exception.ServiceUnavailableException
-import at.bitfire.dav4jvm.exception.UnauthorizedException
+import at.bitfire.dav4jvm.exception.*
 import at.bitfire.dav4jvm.property.caldav.NS_CALDAV
 import at.bitfire.dav4jvm.property.carddav.NS_CARDDAV
 import at.bitfire.dav4jvm.property.webdav.NS_WEBDAV
 import at.bitfire.dav4jvm.property.webdav.SyncToken
-import okhttp3.Headers
-import okhttp3.HttpUrl
+import okhttp3.*
 import okhttp3.MediaType.Companion.toMediaType
-import okhttp3.OkHttpClient
-import okhttp3.Request
-import okhttp3.RequestBody
 import okhttp3.RequestBody.Companion.toRequestBody
 import okhttp3.Response
 import org.xmlpull.v1.XmlPullParser
@@ -784,7 +773,7 @@ open class DavResource @JvmOverloads constructor(
                         DavResponse.RESPONSE ->
                             at.bitfire.dav4jvm.Response.parse(parser, location, callback)
                         SyncToken.NAME ->
-                            XmlUtils.readText(parser)?.let {
+                            XmlReader(parser).readText()?.let {
                                 responseProperties += SyncToken(it)
                             }
                     }

src/main/kotlin/at/bitfire/dav4jvm/PropertyFactory.kt

@@ -13,14 +13,19 @@ interface PropertyFactory {
 
     /**
      * Name of the Property the factory creates,
-     * e.g. Property.Name("DAV:", "displayname") if the factory creates DisplayName objects)
+     * e.g. `Property.Name("DAV:", "displayname")` if the factory creates
+     * [at.bitfire.dav4jvm.property.webdav.DisplayName] objects)
      */
     fun getName(): Property.Name
 
     /**
      * Parses XML of a property and returns its data class.
+     *
+     * Implementations shouldn't make assumptions on which sub-properties are available
+     * or not and in doubt return an empty [Property].
+     *
      * @throws XmlPullParserException in case of parsing errors
      */
-    fun create(parser: XmlPullParser): Property?
+    fun create(parser: XmlPullParser): Property
 
 }

src/main/kotlin/at/bitfire/dav4jvm/PropertyRegistry.kt

@@ -6,45 +6,13 @@
 
 package at.bitfire.dav4jvm
 
-import at.bitfire.dav4jvm.property.caldav.CalendarColor
-import at.bitfire.dav4jvm.property.caldav.CalendarData
-import at.bitfire.dav4jvm.property.caldav.CalendarDescription
-import at.bitfire.dav4jvm.property.caldav.CalendarHomeSet
-import at.bitfire.dav4jvm.property.caldav.CalendarProxyReadFor
-import at.bitfire.dav4jvm.property.caldav.CalendarProxyWriteFor
-import at.bitfire.dav4jvm.property.caldav.CalendarTimezone
-import at.bitfire.dav4jvm.property.caldav.CalendarUserAddressSet
-import at.bitfire.dav4jvm.property.caldav.GetCTag
-import at.bitfire.dav4jvm.property.caldav.ScheduleTag
-import at.bitfire.dav4jvm.property.caldav.Source
-import at.bitfire.dav4jvm.property.caldav.SupportedCalendarComponentSet
-import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData
+import at.bitfire.dav4jvm.property.caldav.*
 import at.bitfire.dav4jvm.property.carddav.AddressData
 import at.bitfire.dav4jvm.property.carddav.AddressbookDescription
 import at.bitfire.dav4jvm.property.carddav.AddressbookHomeSet
 import at.bitfire.dav4jvm.property.carddav.SupportedAddressData
-import at.bitfire.dav4jvm.property.push.PushMessage
-import at.bitfire.dav4jvm.property.push.PushSubscribe
-import at.bitfire.dav4jvm.property.push.PushTransports
-import at.bitfire.dav4jvm.property.push.Subscription
-import at.bitfire.dav4jvm.property.push.Topic
-import at.bitfire.dav4jvm.property.push.WebPushSubscription
-import at.bitfire.dav4jvm.property.webdav.AddMember
-import at.bitfire.dav4jvm.property.webdav.CreationDate
-import at.bitfire.dav4jvm.property.webdav.CurrentUserPrincipal
-import at.bitfire.dav4jvm.property.webdav.CurrentUserPrivilegeSet
-import at.bitfire.dav4jvm.property.webdav.DisplayName
-import at.bitfire.dav4jvm.property.webdav.GetContentLength
-import at.bitfire.dav4jvm.property.webdav.GetContentType
-import at.bitfire.dav4jvm.property.webdav.GetETag
-import at.bitfire.dav4jvm.property.webdav.GetLastModified
-import at.bitfire.dav4jvm.property.webdav.GroupMembership
-import at.bitfire.dav4jvm.property.webdav.Owner
-import at.bitfire.dav4jvm.property.webdav.QuotaAvailableBytes
-import at.bitfire.dav4jvm.property.webdav.QuotaUsedBytes
-import at.bitfire.dav4jvm.property.webdav.ResourceType
-import at.bitfire.dav4jvm.property.webdav.SupportedReportSet
-import at.bitfire.dav4jvm.property.webdav.SyncToken
+import at.bitfire.dav4jvm.property.push.*
+import at.bitfire.dav4jvm.property.webdav.*
 import org.xmlpull.v1.XmlPullParser
 import org.xmlpull.v1.XmlPullParserException
 import java.util.logging.Level

src/main/kotlin/at/bitfire/dav4jvm/XmlReader.kt

@@ -0,0 +1,170 @@
+package at.bitfire.dav4jvm
+
+import at.bitfire.dav4jvm.XmlUtils.propertyName
+import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData.Companion.CONTENT_TYPE
+import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData.Companion.VERSION
+import okhttp3.MediaType
+import okhttp3.MediaType.Companion.toMediaTypeOrNull
+import org.xmlpull.v1.XmlPullParser
+import org.xmlpull.v1.XmlPullParserException
+import java.io.IOException
+import java.time.Instant
+import java.util.logging.Level
+import java.util.logging.Logger
+
+/**
+ * Reads/processes XML tags which are used for WebDAV.
+ *
+ * @param parser The parser to read from.
+ */
+class XmlReader(
+    private val parser: XmlPullParser
+) {
+
+    // base processing
+
+    /**
+     * Reads child elements of the current element. Whenever a direct child with the given name is found,
+     * [processor] is called for each one.
+     */
+    @Throws(IOException::class, XmlPullParserException::class)
+    fun processTag(name: Property.Name, processor: XmlReader.() -> Unit) {
+        val depth = parser.depth
+        var eventType = parser.eventType
+        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
+            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
+                processor()
+            eventType = parser.next()
+        }
+    }
+
+    /**
+     * Reads the inline text of the current element.
+     *
+     * For instance, if the parser is at the beginning of this XML:
+     *
+     * ```
+     * <tag>text</tag>
+     * ```
+     *
+     * this function will return "text".
+     *
+     * @return text or `null` if no text is found
+     */
+    @Throws(IOException::class, XmlPullParserException::class)
+    fun readText(): String? {
+        var text: String? = null
+
+        val depth = parser.depth
+        var eventType = parser.eventType
+        while (!(eventType == XmlPullParser.END_TAG && parser.depth == depth)) {
+            if (eventType == XmlPullParser.TEXT && parser.depth == depth)
+                text = parser.text
+            eventType = parser.next()
+        }
+
+        return text
+    }
+
+    /**
+     * Reads child elements of the current element. When a direct child with the given name is found,
+     * its text is returned.
+     *
+     * @param name The name of the tag to read.
+     * @return The text inside the tag, or `null` if the tag is not found.
+     */
+    @Throws(IOException::class, XmlPullParserException::class)
+    fun readTextProperty(name: Property.Name): String? {
+        var result: String? = null
+
+        val depth = parser.depth
+        var eventType = parser.eventType
+        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
+            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
+                result = parser.nextText()
+            eventType = parser.next()
+        }
+        return result
+    }
+
+    /**
+     * Reads child elements of the current element. Whenever a direct child with the given name is
+     * found, its text is added to the given list.
+     *
+     * @param name The name of the tag to read.
+     * @param list The list to add the text to.
+     */
+    @Throws(IOException::class, XmlPullParserException::class)
+    fun readTextPropertyList(name: Property.Name, list: MutableCollection<String>) {
+        val depth = parser.depth
+        var eventType = parser.eventType
+        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
+            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
+                list.add(parser.nextText())
+            eventType = parser.next()
+        }
+    }
+
+
+    // extended processing (uses readText etc.)
+
+    /**
+     * Uses [readText] to read the tag's value (which is expected to be in _HTTP-date_ format), and converts
+     * it into an [Instant] using [HttpUtils.parseDate].
+     *
+     * If the conversion fails for any reason, null is returned, and a message is displayed in log.
+     */
+    fun readHttpDate(): Instant? {
+        return readText()?.let { rawDate ->
+            val date = HttpUtils.parseDate(rawDate)
+            if (date != null)
+                date
+            else {
+                val logger = Logger.getLogger(javaClass.name)
+                logger.warning("Couldn't parse HTTP-date")
+                null
+            }
+        }
+    }
+
+    /**
+     * Uses [readText] to read the tag's value (which is expected to be a number), and converts it
+     * into a [Long] with [String.toLong].
+     *
+     * If the conversion fails for any reason, null is returned, and a message is displayed in log.
+     */
+    fun readLong(): Long? {
+        return readText()?.let { valueStr ->
+            try {
+                valueStr.toLong()
+            } catch(e: NumberFormatException) {
+                val logger = Logger.getLogger(javaClass.name)
+                logger.log(Level.WARNING, "Couldn't parse as Long: $valueStr", e)
+                null
+            }
+        }
+    }
+
+    /**
+     * Processes all the tags named [tagName], and sends every tag that has the [CONTENT_TYPE]
+     * attribute with [onNewType].
+     *
+     * @param tagName The name of the tag that contains the [CONTENT_TYPE] attribute value.
+     * @param onNewType Called every time a new [MediaType] is found.
+     */
+    fun readContentTypes(tagName: Property.Name, onNewType: (MediaType) -> Unit) {
+        try {
+            processTag(tagName) {
+                parser.getAttributeValue(null, CONTENT_TYPE)?.let { contentType ->
+                    var type = contentType
+                    parser.getAttributeValue(null, VERSION)?.let { version -> type += "; version=$version" }
+                    type.toMediaTypeOrNull()?.let(onNewType)
+                }
+            }
+        } catch(e: XmlPullParserException) {
+            val logger = Logger.getLogger(javaClass.name)
+            logger.log(Level.SEVERE, "Couldn't parse content types", e)
+        }
+    }
+
+}

src/main/kotlin/at/bitfire/dav4jvm/XmlUtils.kt

@@ -7,12 +7,10 @@
 package at.bitfire.dav4jvm
 
 import at.bitfire.dav4jvm.exception.DavException
-import at.bitfire.dav4jvm.exception.InvalidPropertyException
 import org.xmlpull.v1.XmlPullParser
 import org.xmlpull.v1.XmlPullParserException
 import org.xmlpull.v1.XmlPullParserFactory
 import org.xmlpull.v1.XmlSerializer
-import java.io.IOException
 
 object XmlUtils {
 
@@ -57,67 +55,6 @@ object XmlUtils {
         ?: throw DavException("Couldn't create XML serializer")
 
 
-    @Throws(IOException::class, XmlPullParserException::class)
-    fun processTag(parser: XmlPullParser, name: Property.Name, processor: () -> Unit) {
-        val depth = parser.depth
-        var eventType = parser.eventType
-        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
-            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
-                processor()
-            eventType = parser.next()
-        }
-    }
-
-    @Throws(IOException::class, XmlPullParserException::class)
-    fun readText(parser: XmlPullParser): String? {
-        var text: String? = null
-
-        val depth = parser.depth
-        var eventType = parser.eventType
-        while (!(eventType == XmlPullParser.END_TAG && parser.depth == depth)) {
-            if (eventType == XmlPullParser.TEXT && parser.depth == depth)
-                text = parser.text
-            eventType = parser.next()
-        }
-
-        return text
-    }
-
-    /**
-     * Same as [readText], but requires a [XmlPullParser.TEXT] value.
-     *
-     * @throws InvalidPropertyException when no text could be read
-     */
-    @Throws(InvalidPropertyException::class, IOException::class, XmlPullParserException::class)
-    fun requireReadText(parser: XmlPullParser): String =
-        readText(parser) ?:
-        throw InvalidPropertyException("XML text for ${parser.namespace}:${parser.name} must not be empty")
-
-    @Throws(IOException::class, XmlPullParserException::class)
-    fun readTextProperty(parser: XmlPullParser, name: Property.Name): String? {
-        val depth = parser.depth
-        var eventType = parser.eventType
-        var result: String? = null
-        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
-            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
-                result = parser.nextText()
-            eventType = parser.next()
-        }
-        return result
-    }
-
-    @Throws(IOException::class, XmlPullParserException::class)
-    fun readTextPropertyList(parser: XmlPullParser, name: Property.Name, list: MutableCollection<String>) {
-        val depth = parser.depth
-        var eventType = parser.eventType
-        while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
-            if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
-                list.add(parser.nextText())
-            eventType = parser.next()
-        }
-    }
-
-
     fun XmlSerializer.insertTag(name: Property.Name, contentGenerator: XmlSerializer.() -> Unit = {}) {
         startTag(name.namespace, name.name)
         contentGenerator(this)

src/main/kotlin/at/bitfire/dav4jvm/property/caldav/CalendarColor.kt

@@ -8,14 +8,14 @@ package at.bitfire.dav4jvm.property.caldav
 
 import at.bitfire.dav4jvm.Property
 import at.bitfire.dav4jvm.PropertyFactory
-import at.bitfire.dav4jvm.XmlUtils
+import at.bitfire.dav4jvm.XmlReader
 import org.xmlpull.v1.XmlPullParser
 import java.util.logging.Level
 import java.util.logging.Logger
 import java.util.regex.Pattern
 
 data class CalendarColor(
-        val color: Int
+    val color: Int?
 ): Property {
 
     companion object {
@@ -49,16 +49,16 @@ data class CalendarColor(
 
         override fun getName() = NAME
 
-        override fun create(parser: XmlPullParser): CalendarColor? {
-            XmlUtils.readText(parser)?.let {
+        override fun create(parser: XmlPullParser): CalendarColor {
+            XmlReader(parser).readText()?.let {
                 try {
                     return CalendarColor(parseARGBColor(it))
                 } catch (e: IllegalArgumentException) {
                     val logger = Logger.getLogger(javaClass.name)
                     logger.log(Level.WARNING, "Couldn't parse color, ignoring", e)
                 }
             }
-            return null
+            return CalendarColor(null)
         }
 
     }