Skip to content

Commit 8f96097

Browse files
rfc2822rfc2822
authored
Fix TODO and add some tests for ResponseParser (#146)
- Move callback to parseResponse method in ResponseParser - Update MultiStatusParser to use new ResponseParser signature - Add comprehensive tests for ResponseParser
1 parent 391ca3d commit 8f96097

File tree

3 files changed

+111
-18
lines changed

3 files changed

+111
-18
lines changed

‎src/main/kotlin/at/bitfire/dav4jvm/ktor/MultiStatusParser.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class MultiStatusParser(
3030

3131
suspend fun parseResponse(parser: XmlPullParser): List<Property> {
3232
val responseProperties = mutableListOf<Property>()
33-
val responseParser = ResponseParser(location, callback)
33+
val responseParser = ResponseParser(location)
3434

3535
// <!ELEMENT multistatus (response*, responsedescription?,
3636
// sync-token?) >
@@ -40,7 +40,7 @@ class MultiStatusParser(
4040
if (eventType == XmlPullParser.START_TAG && parser.depth == depth + 1)
4141
when (parser.propertyName()) {
4242
WebDAV.Response ->
43-
responseParser.parseResponse(parser)
43+
responseParser.parseResponse(parser, callback)
4444
WebDAV.SyncToken ->
4545
XmlReader(parser).readText()?.let {
4646
responseProperties += SyncToken(it)

‎src/main/kotlin/at/bitfire/dav4jvm/ktor/ResponseParser.kt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ import java.util.logging.Logger
3232
* @param location location of the request (used to resolve possible relative `<href>`)
3333
*/
3434
class ResponseParser(
35-
private val location: Url,
36-
private val callback: MultiResponseCallback
35+
private val location: Url
3736
) {
3837

3938
private val logger
@@ -49,8 +48,11 @@ class ResponseParser(
4948
*
5049
* So if you want PROPFIND results to have a trailing slash when they are collections, make sure
5150
* that you query [at.bitfire.dav4jvm.property.webdav.ResourceType].
51+
*
52+
* @param parser XML document to parse
53+
* @param callback callback to be called for every multistatus `<response>`
5254
*/
53-
suspend fun parseResponse(parser: XmlPullParser) {
55+
suspend fun parseResponse(parser: XmlPullParser, callback: MultiResponseCallback) {
5456
val depth = parser.depth
5557

5658
var hrefOrNull: Url? = null
@@ -72,7 +74,7 @@ class ResponseParser(
7274
WebDAV.Error ->
7375
error = Error.parseError(parser)
7476
WebDAV.Location ->
75-
newLocation = Url(parser.nextText()) // TODO: Need to catch exception here?
77+
newLocation = parser.nextText().toUrlOrNull()
7678
}
7779
eventType = parser.next()
7880
}
@@ -94,8 +96,6 @@ class ResponseParser(
9496
href = UrlUtils.withTrailingSlash(href)
9597
}
9698

97-
//log.log(Level.FINE, "Received properties for $href", if (status != null) status else propStat)
98-
9999
// Which resource does this <response> represent?
100100
val relation = when {
101101
UrlUtils.omitTrailingSlash(href).equalsForWebDAV(UrlUtils.omitTrailingSlash(location)) ->

‎src/test/kotlin/at/bitfire/dav4jvm/ktor/ResponseParserTest.kt‎

Lines changed: 103 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,45 +10,138 @@
1010

1111
package at.bitfire.dav4jvm.ktor
1212

13+
import at.bitfire.dav4jvm.XmlUtils
1314
import io.ktor.http.Url
15+
import kotlinx.coroutines.test.runTest
1416
import org.junit.Assert.assertEquals
1517
import org.junit.Test
18+
import java.io.StringReader
1619

1720
class ResponseParserTest {
1821

19-
val baseUrl = Url("https://example.com/collection/")
20-
val parser = ResponseParser(baseUrl, callback = { _, _ ->
21-
// no-op
22-
})
22+
private val baseUrl = Url("http://www.example.com/container/")
23+
private val parser = ResponseParser(baseUrl)
24+
25+
26+
@Test
27+
fun `parseResponse relation=SELF`() = runTest {
28+
val xml = XmlUtils.newPullParser()
29+
xml.setInput(StringReader("<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" +
30+
"<multistatus xmlns=\"DAV:\">\n" +
31+
"<response>\n" +
32+
" <href>http://www.example.com/container/</href>\n" +
33+
" <propstat>\n" +
34+
" <prop xmlns:R=\"http://ns.example.com/boxschema/\">\n" +
35+
" <R:bigbox/>\n" +
36+
" <R:author/>\n" +
37+
" <creationdate/>\n" +
38+
" <displayname/>\n" +
39+
" <resourcetype/>\n" +
40+
" <supportedlock/>\n" +
41+
" </prop>\n" +
42+
" <status>HTTP/1.1 200 OK</status>\n" +
43+
" </propstat>\n" +
44+
"</response>"
45+
))
46+
xml.nextTag() // multistatus
47+
xml.nextTag() // response
48+
parser.parseResponse(xml) { response, relation ->
49+
assertEquals(Url("http://www.example.com/container/"), response.href)
50+
assertEquals(Response.HrefRelation.SELF, relation)
51+
}
52+
}
53+
54+
@Test
55+
fun `parseResponse relation=MEMBER`() = runTest {
56+
val xml = XmlUtils.newPullParser()
57+
xml.setInput(StringReader("<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" +
58+
"<multistatus xmlns=\"DAV:\">\n" +
59+
"<response>\n" +
60+
" <href>http://www.example.com/container/front.html</href>\n" +
61+
" <propstat>\n" +
62+
" <prop xmlns:R=\"http://ns.example.com/boxschema/\">\n" +
63+
" <R:bigbox/>\n" +
64+
" <creationdate/>\n" +
65+
" <displayname/>\n" +
66+
" <getcontentlength/>\n" +
67+
" <getcontenttype/>\n" +
68+
" <getetag/>\n" +
69+
" <getlastmodified/>\n" +
70+
" <resourcetype/>\n" +
71+
" <supportedlock/>\n" +
72+
" </prop>\n" +
73+
" <status>HTTP/1.1 200 OK</status>\n" +
74+
" </propstat>\n" +
75+
"</response>"
76+
))
77+
xml.nextTag() // multistatus
78+
xml.nextTag() // response
79+
parser.parseResponse(xml) { response, relation ->
80+
assertEquals(Url("http://www.example.com/container/front.html"), response.href)
81+
assertEquals(Response.HrefRelation.MEMBER, relation)
82+
}
83+
}
84+
85+
@Test
86+
fun `parseResponse relation=OTHER`() = runTest {
87+
val xml = XmlUtils.newPullParser()
88+
xml.setInput(StringReader("<?xml version=\"1.0\" encoding=\"utf-8\" ?>\n" +
89+
"<multistatus xmlns=\"DAV:\">\n" +
90+
"<response>\n" +
91+
" <href>http://other.example.com/was-not-requested</href>\n" +
92+
" <propstat>\n" +
93+
" <prop xmlns:R=\"http://ns.example.com/boxschema/\">\n" +
94+
" <R:bigbox/>\n" +
95+
" <creationdate/>\n" +
96+
" <displayname/>\n" +
97+
" <getcontentlength/>\n" +
98+
" <getcontenttype/>\n" +
99+
" <getetag/>\n" +
100+
" <getlastmodified/>\n" +
101+
" <resourcetype/>\n" +
102+
" <supportedlock/>\n" +
103+
" </prop>\n" +
104+
" <status>HTTP/1.1 200 OK</status>\n" +
105+
" </propstat>\n" +
106+
"</response>"
107+
))
108+
xml.nextTag() // multistatus
109+
xml.nextTag() // response
110+
parser.parseResponse(xml) { response, relation ->
111+
assertEquals(Url("http://other.example.com/was-not-requested"), response.href)
112+
assertEquals(Response.HrefRelation.OTHER, relation)
113+
}
114+
}
115+
23116

24117
@Test
25118
fun `resolveHref with absolute URL`() {
26119
assertEquals(
27-
Url("https://example.com/collection/member"),
28-
parser.resolveHref("https://example.com/collection/member")
120+
Url("http://www.example.com/container/member"),
121+
parser.resolveHref("http://www.example.com/container/member")
29122
)
30123
}
31124

32125
@Test
33126
fun `resolveHref with absolute path`() {
34127
assertEquals(
35-
Url("https://example.com/collection/member"),
36-
parser.resolveHref("/collection/member")
128+
Url("http://www.example.com/container/member"),
129+
parser.resolveHref("/container/member")
37130
)
38131
}
39132

40133
@Test
41134
fun `resolveHref with relative path`() {
42135
assertEquals(
43-
Url("https://example.com/collection/member"),
136+
Url("http://www.example.com/container/member"),
44137
parser.resolveHref("member")
45138
)
46139
}
47140

48141
@Test
49142
fun `resolveHref with relative path with colon`() {
50143
assertEquals(
51-
Url("https://example.com/collection/mem:ber"),
144+
Url("http://www.example.com/container/mem:ber"),
52145
parser.resolveHref("mem:ber")
53146
)
54147
}

0 commit comments

Comments
 (0)