Skip to content

Commit fe97db3

Browse files
ArnyminerZrfc2822
ArnyminerZ
and
rfc2822
authored
Improve null-safety in Factories and cleanup (#53)
* Changed logging for Factory on missing topic Signed-off-by: Arnau Mora <[email protected]> * Changed logging level Signed-off-by: Arnau Mora <[email protected]> * Deprecated `requireReadText` Signed-off-by: Arnau Mora <[email protected]> * Replaced usages of `requireReadText` Signed-off-by: Arnau Mora <[email protected]> * Got rid of `requireReadText` Signed-off-by: Arnau Mora <[email protected]> * Returning non-null Signed-off-by: Arnau Mora <[email protected]> * Adjusted test for empty GetETag Signed-off-by: Arnau Mora <[email protected]> * Made all property's values nullable Signed-off-by: Arnau Mora Gras <[email protected]> * Fixed check Signed-off-by: Arnau Mora Gras <[email protected]> * Got rid of generic type for properties Signed-off-by: Arnau Mora Gras <[email protected]> * Got rid of fixme Signed-off-by: Arnau Mora Gras <[email protected]> * Got rid of fixme Signed-off-by: Arnau Mora Gras <[email protected]> * Unnecessary condition Signed-off-by: Arnau Mora Gras <[email protected]> * Code cleanup Signed-off-by: Arnau Mora Gras <[email protected]> * Renamed `readInstantOrNull` to `readHttpDateOrNull` Signed-off-by: Arnau Mora Gras <[email protected]> * Got rid of duplicate code fragment Signed-off-by: Arnau Mora Gras <[email protected]> * Improved logged message Signed-off-by: Arnau Mora Gras <[email protected]> * Created XmlReader Signed-off-by: Arnau Mora Gras <[email protected]> * Added tests for readLongOrNull Signed-off-by: Arnau Mora Gras <[email protected]> * Added tests for readHttpDateOrNull Signed-off-by: Arnau Mora Gras <[email protected]> * Added tests for readContentTypes Signed-off-by: Arnau Mora Gras <[email protected]> * KDoc, minor changes --------- Signed-off-by: Arnau Mora <[email protected]> Signed-off-by: Arnau Mora Gras <[email protected]> Co-authored-by: Ricki Hirner <[email protected]>
1 parent 2b41498 commit fe97db3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+490
-474
lines changed

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,13 @@ package at.bitfire.dav4jvm
88

99
import at.bitfire.dav4jvm.XmlUtils.insertTag
1010
import at.bitfire.dav4jvm.XmlUtils.propertyName
11-
import at.bitfire.dav4jvm.exception.ConflictException
12-
import at.bitfire.dav4jvm.exception.DavException
13-
import at.bitfire.dav4jvm.exception.ForbiddenException
14-
import at.bitfire.dav4jvm.exception.HttpException
15-
import at.bitfire.dav4jvm.exception.NotFoundException
16-
import at.bitfire.dav4jvm.exception.PreconditionFailedException
17-
import at.bitfire.dav4jvm.exception.ServiceUnavailableException
18-
import at.bitfire.dav4jvm.exception.UnauthorizedException
11+
import at.bitfire.dav4jvm.exception.*
1912
import at.bitfire.dav4jvm.property.caldav.NS_CALDAV
2013
import at.bitfire.dav4jvm.property.carddav.NS_CARDDAV
2114
import at.bitfire.dav4jvm.property.webdav.NS_WEBDAV
2215
import at.bitfire.dav4jvm.property.webdav.SyncToken
23-
import okhttp3.Headers
24-
import okhttp3.HttpUrl
16+
import okhttp3.*
2517
import okhttp3.MediaType.Companion.toMediaType
26-
import okhttp3.OkHttpClient
27-
import okhttp3.Request
28-
import okhttp3.RequestBody
2918
import okhttp3.RequestBody.Companion.toRequestBody
3019
import okhttp3.Response
3120
import org.xmlpull.v1.XmlPullParser
@@ -784,7 +773,7 @@ open class DavResource @JvmOverloads constructor(
784773
DavResponse.RESPONSE ->
785774
at.bitfire.dav4jvm.Response.parse(parser, location, callback)
786775
SyncToken.NAME ->
787-
XmlUtils.readText(parser)?.let {
776+
XmlReader(parser).readText()?.let {
788777
responseProperties += SyncToken(it)
789778
}
790779
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,19 @@ interface PropertyFactory {
1313

1414
/**
1515
* Name of the Property the factory creates,
16-
* e.g. Property.Name("DAV:", "displayname") if the factory creates DisplayName objects)
16+
* e.g. `Property.Name("DAV:", "displayname")` if the factory creates
17+
* [at.bitfire.dav4jvm.property.webdav.DisplayName] objects)
1718
*/
1819
fun getName(): Property.Name
1920

2021
/**
2122
* Parses XML of a property and returns its data class.
23+
*
24+
* Implementations shouldn't make assumptions on which sub-properties are available
25+
* or not and in doubt return an empty [Property].
26+
*
2227
* @throws XmlPullParserException in case of parsing errors
2328
*/
24-
fun create(parser: XmlPullParser): Property?
29+
fun create(parser: XmlPullParser): Property
2530

2631
}

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

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,13 @@
66

77
package at.bitfire.dav4jvm
88

9-
import at.bitfire.dav4jvm.property.caldav.CalendarColor
10-
import at.bitfire.dav4jvm.property.caldav.CalendarData
11-
import at.bitfire.dav4jvm.property.caldav.CalendarDescription
12-
import at.bitfire.dav4jvm.property.caldav.CalendarHomeSet
13-
import at.bitfire.dav4jvm.property.caldav.CalendarProxyReadFor
14-
import at.bitfire.dav4jvm.property.caldav.CalendarProxyWriteFor
15-
import at.bitfire.dav4jvm.property.caldav.CalendarTimezone
16-
import at.bitfire.dav4jvm.property.caldav.CalendarUserAddressSet
17-
import at.bitfire.dav4jvm.property.caldav.GetCTag
18-
import at.bitfire.dav4jvm.property.caldav.ScheduleTag
19-
import at.bitfire.dav4jvm.property.caldav.Source
20-
import at.bitfire.dav4jvm.property.caldav.SupportedCalendarComponentSet
21-
import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData
9+
import at.bitfire.dav4jvm.property.caldav.*
2210
import at.bitfire.dav4jvm.property.carddav.AddressData
2311
import at.bitfire.dav4jvm.property.carddav.AddressbookDescription
2412
import at.bitfire.dav4jvm.property.carddav.AddressbookHomeSet
2513
import at.bitfire.dav4jvm.property.carddav.SupportedAddressData
26-
import at.bitfire.dav4jvm.property.push.PushMessage
27-
import at.bitfire.dav4jvm.property.push.PushSubscribe
28-
import at.bitfire.dav4jvm.property.push.PushTransports
29-
import at.bitfire.dav4jvm.property.push.Subscription
30-
import at.bitfire.dav4jvm.property.push.Topic
31-
import at.bitfire.dav4jvm.property.push.WebPushSubscription
32-
import at.bitfire.dav4jvm.property.webdav.AddMember
33-
import at.bitfire.dav4jvm.property.webdav.CreationDate
34-
import at.bitfire.dav4jvm.property.webdav.CurrentUserPrincipal
35-
import at.bitfire.dav4jvm.property.webdav.CurrentUserPrivilegeSet
36-
import at.bitfire.dav4jvm.property.webdav.DisplayName
37-
import at.bitfire.dav4jvm.property.webdav.GetContentLength
38-
import at.bitfire.dav4jvm.property.webdav.GetContentType
39-
import at.bitfire.dav4jvm.property.webdav.GetETag
40-
import at.bitfire.dav4jvm.property.webdav.GetLastModified
41-
import at.bitfire.dav4jvm.property.webdav.GroupMembership
42-
import at.bitfire.dav4jvm.property.webdav.Owner
43-
import at.bitfire.dav4jvm.property.webdav.QuotaAvailableBytes
44-
import at.bitfire.dav4jvm.property.webdav.QuotaUsedBytes
45-
import at.bitfire.dav4jvm.property.webdav.ResourceType
46-
import at.bitfire.dav4jvm.property.webdav.SupportedReportSet
47-
import at.bitfire.dav4jvm.property.webdav.SyncToken
14+
import at.bitfire.dav4jvm.property.push.*
15+
import at.bitfire.dav4jvm.property.webdav.*
4816
import org.xmlpull.v1.XmlPullParser
4917
import org.xmlpull.v1.XmlPullParserException
5018
import java.util.logging.Level

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

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package at.bitfire.dav4jvm
2+
3+
import at.bitfire.dav4jvm.XmlUtils.propertyName
4+
import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData.Companion.CONTENT_TYPE
5+
import at.bitfire.dav4jvm.property.caldav.SupportedCalendarData.Companion.VERSION
6+
import okhttp3.MediaType
7+
import okhttp3.MediaType.Companion.toMediaTypeOrNull
8+
import org.xmlpull.v1.XmlPullParser
9+
import org.xmlpull.v1.XmlPullParserException
10+
import java.io.IOException
11+
import java.time.Instant
12+
import java.util.logging.Level
13+
import java.util.logging.Logger
14+
15+
/**
16+
* Reads/processes XML tags which are used for WebDAV.
17+
*
18+
* @param parser The parser to read from.
19+
*/
20+
class XmlReader(
21+
private val parser: XmlPullParser
22+
) {
23+
24+
// base processing
25+
26+
/**
27+
* Reads child elements of the current element. Whenever a direct child with the given name is found,
28+
* [processor] is called for each one.
29+
*/
30+
@Throws(IOException::class, XmlPullParserException::class)
31+
fun processTag(name: Property.Name, processor: XmlReader.() -> Unit) {
32+
val depth = parser.depth
33+
var eventType = parser.eventType
34+
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
35+
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
36+
processor()
37+
eventType = parser.next()
38+
}
39+
}
40+
41+
/**
42+
* Reads the inline text of the current element.
43+
*
44+
* For instance, if the parser is at the beginning of this XML:
45+
*
46+
* ```
47+
* <tag>text</tag>
48+
* ```
49+
*
50+
* this function will return "text".
51+
*
52+
* @return text or `null` if no text is found
53+
*/
54+
@Throws(IOException::class, XmlPullParserException::class)
55+
fun readText(): String? {
56+
var text: String? = null
57+
58+
val depth = parser.depth
59+
var eventType = parser.eventType
60+
while (!(eventType == XmlPullParser.END_TAG && parser.depth == depth)) {
61+
if (eventType == XmlPullParser.TEXT && parser.depth == depth)
62+
text = parser.text
63+
eventType = parser.next()
64+
}
65+
66+
return text
67+
}
68+
69+
/**
70+
* Reads child elements of the current element. When a direct child with the given name is found,
71+
* its text is returned.
72+
*
73+
* @param name The name of the tag to read.
74+
* @return The text inside the tag, or `null` if the tag is not found.
75+
*/
76+
@Throws(IOException::class, XmlPullParserException::class)
77+
fun readTextProperty(name: Property.Name): String? {
78+
var result: String? = null
79+
80+
val depth = parser.depth
81+
var eventType = parser.eventType
82+
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
83+
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
84+
result = parser.nextText()
85+
eventType = parser.next()
86+
}
87+
return result
88+
}
89+
90+
/**
91+
* Reads child elements of the current element. Whenever a direct child with the given name is
92+
* found, its text is added to the given list.
93+
*
94+
* @param name The name of the tag to read.
95+
* @param list The list to add the text to.
96+
*/
97+
@Throws(IOException::class, XmlPullParserException::class)
98+
fun readTextPropertyList(name: Property.Name, list: MutableCollection<String>) {
99+
val depth = parser.depth
100+
var eventType = parser.eventType
101+
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
102+
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
103+
list.add(parser.nextText())
104+
eventType = parser.next()
105+
}
106+
}
107+
108+
109+
// extended processing (uses readText etc.)
110+
111+
/**
112+
* Uses [readText] to read the tag's value (which is expected to be in _HTTP-date_ format), and converts
113+
* it into an [Instant] using [HttpUtils.parseDate].
114+
*
115+
* If the conversion fails for any reason, null is returned, and a message is displayed in log.
116+
*/
117+
fun readHttpDate(): Instant? {
118+
return readText()?.let { rawDate ->
119+
val date = HttpUtils.parseDate(rawDate)
120+
if (date != null)
121+
date
122+
else {
123+
val logger = Logger.getLogger(javaClass.name)
124+
logger.warning("Couldn't parse HTTP-date")
125+
null
126+
}
127+
}
128+
}
129+
130+
/**
131+
* Uses [readText] to read the tag's value (which is expected to be a number), and converts it
132+
* into a [Long] with [String.toLong].
133+
*
134+
* If the conversion fails for any reason, null is returned, and a message is displayed in log.
135+
*/
136+
fun readLong(): Long? {
137+
return readText()?.let { valueStr ->
138+
try {
139+
valueStr.toLong()
140+
} catch(e: NumberFormatException) {
141+
val logger = Logger.getLogger(javaClass.name)
142+
logger.log(Level.WARNING, "Couldn't parse as Long: $valueStr", e)
143+
null
144+
}
145+
}
146+
}
147+
148+
/**
149+
* Processes all the tags named [tagName], and sends every tag that has the [CONTENT_TYPE]
150+
* attribute with [onNewType].
151+
*
152+
* @param tagName The name of the tag that contains the [CONTENT_TYPE] attribute value.
153+
* @param onNewType Called every time a new [MediaType] is found.
154+
*/
155+
fun readContentTypes(tagName: Property.Name, onNewType: (MediaType) -> Unit) {
156+
try {
157+
processTag(tagName) {
158+
parser.getAttributeValue(null, CONTENT_TYPE)?.let { contentType ->
159+
var type = contentType
160+
parser.getAttributeValue(null, VERSION)?.let { version -> type += "; version=$version" }
161+
type.toMediaTypeOrNull()?.let(onNewType)
162+
}
163+
}
164+
} catch(e: XmlPullParserException) {
165+
val logger = Logger.getLogger(javaClass.name)
166+
logger.log(Level.SEVERE, "Couldn't parse content types", e)
167+
}
168+
}
169+
170+
}

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

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,10 @@
77
package at.bitfire.dav4jvm
88

99
import at.bitfire.dav4jvm.exception.DavException
10-
import at.bitfire.dav4jvm.exception.InvalidPropertyException
1110
import org.xmlpull.v1.XmlPullParser
1211
import org.xmlpull.v1.XmlPullParserException
1312
import org.xmlpull.v1.XmlPullParserFactory
1413
import org.xmlpull.v1.XmlSerializer
15-
import java.io.IOException
1614

1715
object XmlUtils {
1816

@@ -57,67 +55,6 @@ object XmlUtils {
5755
?: throw DavException("Couldn't create XML serializer")
5856

5957

60-
@Throws(IOException::class, XmlPullParserException::class)
61-
fun processTag(parser: XmlPullParser, name: Property.Name, processor: () -> Unit) {
62-
val depth = parser.depth
63-
var eventType = parser.eventType
64-
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
65-
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
66-
processor()
67-
eventType = parser.next()
68-
}
69-
}
70-
71-
@Throws(IOException::class, XmlPullParserException::class)
72-
fun readText(parser: XmlPullParser): String? {
73-
var text: String? = null
74-
75-
val depth = parser.depth
76-
var eventType = parser.eventType
77-
while (!(eventType == XmlPullParser.END_TAG && parser.depth == depth)) {
78-
if (eventType == XmlPullParser.TEXT && parser.depth == depth)
79-
text = parser.text
80-
eventType = parser.next()
81-
}
82-
83-
return text
84-
}
85-
86-
/**
87-
* Same as [readText], but requires a [XmlPullParser.TEXT] value.
88-
*
89-
* @throws InvalidPropertyException when no text could be read
90-
*/
91-
@Throws(InvalidPropertyException::class, IOException::class, XmlPullParserException::class)
92-
fun requireReadText(parser: XmlPullParser): String =
93-
readText(parser) ?:
94-
throw InvalidPropertyException("XML text for ${parser.namespace}:${parser.name} must not be empty")
95-
96-
@Throws(IOException::class, XmlPullParserException::class)
97-
fun readTextProperty(parser: XmlPullParser, name: Property.Name): String? {
98-
val depth = parser.depth
99-
var eventType = parser.eventType
100-
var result: String? = null
101-
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
102-
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
103-
result = parser.nextText()
104-
eventType = parser.next()
105-
}
106-
return result
107-
}
108-
109-
@Throws(IOException::class, XmlPullParserException::class)
110-
fun readTextPropertyList(parser: XmlPullParser, name: Property.Name, list: MutableCollection<String>) {
111-
val depth = parser.depth
112-
var eventType = parser.eventType
113-
while (!((eventType == XmlPullParser.END_TAG || eventType == XmlPullParser.END_DOCUMENT) && parser.depth == depth)) {
114-
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1 && parser.propertyName() == name)
115-
list.add(parser.nextText())
116-
eventType = parser.next()
117-
}
118-
}
119-
120-
12158
fun XmlSerializer.insertTag(name: Property.Name, contentGenerator: XmlSerializer.() -> Unit = {}) {
12259
startTag(name.namespace, name.name)
12360
contentGenerator(this)

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ package at.bitfire.dav4jvm.property.caldav
88

99
import at.bitfire.dav4jvm.Property
1010
import at.bitfire.dav4jvm.PropertyFactory
11-
import at.bitfire.dav4jvm.XmlUtils
11+
import at.bitfire.dav4jvm.XmlReader
1212
import org.xmlpull.v1.XmlPullParser
1313
import java.util.logging.Level
1414
import java.util.logging.Logger
1515
import java.util.regex.Pattern
1616

1717
data class CalendarColor(
18-
val color: Int
18+
val color: Int?
1919
): Property {
2020

2121
companion object {
@@ -49,16 +49,16 @@ data class CalendarColor(
4949

5050
override fun getName() = NAME
5151

52-
override fun create(parser: XmlPullParser): CalendarColor? {
53-
XmlUtils.readText(parser)?.let {
52+
override fun create(parser: XmlPullParser): CalendarColor {
53+
XmlReader(parser).readText()?.let {
5454
try {
5555
return CalendarColor(parseARGBColor(it))
5656
} catch (e: IllegalArgumentException) {
5757
val logger = Logger.getLogger(javaClass.name)
5858
logger.log(Level.WARNING, "Couldn't parse color, ignoring", e)
5959
}
6060
}
61-
return null
61+
return CalendarColor(null)
6262
}
6363

6464
}

0 commit comments

Comments
 (0)