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

Commit 720c01db authored by Ricki Hirner's avatar Ricki Hirner
Browse files

Make WebDAV multistatus response objects immutable

* every operation on a resource generates exactly one response object
* no merging of properties anymore
parent 7968c5ee
Loading
Loading
Loading
Loading
+8 −8
Original line number Diff line number Diff line

buildscript {
    ext.kotlin_version = '1.2.40'
    ext.kotlin_version = '1.2.41'
    ext.dokka_version = '0.9.16'

    repositories {
@@ -55,14 +55,14 @@ android {
}

dependencies {
    compile "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
    implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"

    compile "com.squareup.okhttp3:okhttp:$okhttp_version"
    implementation "com.squareup.okhttp3:okhttp:$okhttp_version"

    androidTestCompile "com.squareup.okhttp3:mockwebserver:$okhttp_version"
    androidTestCompile 'com.android.support.test:runner:1.0.2'
    androidTestImplementation "com.squareup.okhttp3:mockwebserver:$okhttp_version"
    androidTestImplementation 'com.android.support.test:runner:1.0.2'

    testCompile "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
    testCompile 'junit:junit:4.12'
    testCompile "com.squareup.okhttp3:mockwebserver:$okhttp_version"
    testImplementation "org.jetbrains.kotlin:kotlin-test-junit:$kotlin_version"
    testImplementation 'junit:junit:4.12'
    testImplementation "com.squareup.okhttp3:mockwebserver:$okhttp_version"
}
+24 −25
Original line number Diff line number Diff line
@@ -10,7 +10,6 @@ package at.bitfire.dav4android

import at.bitfire.dav4android.exception.HttpException
import at.bitfire.dav4android.property.GetETag
import at.bitfire.dav4android.property.SyncToken
import okhttp3.OkHttpClient
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
@@ -95,25 +94,25 @@ class DavCollectionTest {
                        "     <D:sync-token>http://example.com/ns/sync/1234</D:sync-token>\n" +
                        "   </D:multistatus>")
        )
        collection.reportChanges(null, false, null, GetETag.NAME)
        val changes = collection.reportChanges(null, false, null, GetETag.NAME)

        assertEquals(3, collection.members.size)
        val members = collection.members.iterator()
        assertEquals(3, changes.members.size)
        val members = changes.members.iterator()
        val member1 = members.next()
        assertEquals(sampleUrl().newBuilder().addPathSegment("test.doc").build(), member1.location)
        assertEquals("00001-abcd1", member1.properties[GetETag::class.java]!!.eTag)
        assertEquals(sampleUrl().newBuilder().addPathSegment("test.doc").build(), member1.url)
        assertEquals("00001-abcd1", member1[GetETag::class.java]!!.eTag)

        val member2 = members.next()
        assertEquals(sampleUrl().newBuilder().addPathSegment("vcard.vcf").build(), member2.location)
        assertEquals("00002-abcd1", member2.properties[GetETag::class.java]!!.eTag)
        assertEquals(sampleUrl().newBuilder().addPathSegment("vcard.vcf").build(), member2.url)
        assertEquals("00002-abcd1", member2[GetETag::class.java]!!.eTag)

        val member3 = members.next()
        assertEquals(sampleUrl().newBuilder().addPathSegment("calendar.ics").build(), member3.location)
        assertEquals("00003-abcd1", member3.properties[GetETag::class.java]!!.eTag)
        assertEquals(sampleUrl().newBuilder().addPathSegment("calendar.ics").build(), member3.url)
        assertEquals("00003-abcd1", member3[GetETag::class.java]!!.eTag)

        assertEquals(0, collection.removedMembers.size)
        assertFalse(collection.furtherResults)
        assertEquals("http://example.com/ns/sync/1234", collection.properties[SyncToken::class.java]!!.token)
        assertEquals(0, changes.removedMembers.size)
        assertFalse(changes.furtherResults)
        assertEquals("http://example.com/ns/sync/1234", changes.syncToken!!.token)
    }

    /**
@@ -159,24 +158,24 @@ class DavCollectionTest {
                        "     <D:sync-token>http://example.com/ns/sync/1233</D:sync-token>\n" +
                        "   </D:multistatus>")
        )
        collection.reportChanges(null, false, null, GetETag.NAME)
        val changes = collection.reportChanges(null, false, null, GetETag.NAME)

        assertEquals(2, collection.members.size)
        val members = collection.members.iterator()
        assertEquals(2, changes.members.size)
        val members = changes.members.iterator()
        val member1 = members.next()
        assertEquals(sampleUrl().newBuilder().addPathSegment("test.doc").build(), member1.location)
        assertEquals("00001-abcd1", member1.properties[GetETag::class.java]!!.eTag)
        assertEquals(sampleUrl().newBuilder().addPathSegment("test.doc").build(), member1.url)
        assertEquals("00001-abcd1", member1[GetETag::class.java]!!.eTag)

        val member2 = members.next()
        assertEquals(sampleUrl().newBuilder().addPathSegment("vcard.vcf").build(), member2.location)
        assertEquals("00002-abcd1", member2.properties[GetETag::class.java]!!.eTag)
        assertEquals(sampleUrl().newBuilder().addPathSegment("vcard.vcf").build(), member2.url)
        assertEquals("00002-abcd1", member2[GetETag::class.java]!!.eTag)

        assertEquals(1, collection.removedMembers.size)
        val removedMember = collection.removedMembers.first()
        assertEquals(sampleUrl().newBuilder().addPathSegment("removed.txt").build(), removedMember.location)
        assertEquals(1, changes.removedMembers.size)
        val removedMember = changes.removedMembers.first()
        assertEquals(sampleUrl().newBuilder().addPathSegment("removed.txt").build(), removedMember.url)

        assertTrue(collection.furtherResults)
        assertEquals("http://example.com/ns/sync/1233", collection.properties[SyncToken::class.java]!!.token)
        assertTrue(changes.furtherResults)
        assertEquals("http://example.com/ns/sync/1233", changes.syncToken!!.token)
    }

    /**
+55 −102
Original line number Diff line number Diff line
@@ -11,7 +11,10 @@ package at.bitfire.dav4android
import at.bitfire.dav4android.exception.HttpException
import at.bitfire.dav4android.exception.InvalidDavResponseException
import at.bitfire.dav4android.exception.PreconditionFailedException
import at.bitfire.dav4android.property.*
import at.bitfire.dav4android.property.DisplayName
import at.bitfire.dav4android.property.GetContentType
import at.bitfire.dav4android.property.GetETag
import at.bitfire.dav4android.property.ResourceType
import okhttp3.MediaType
import okhttp3.OkHttpClient
import okhttp3.RequestBody
@@ -54,16 +57,15 @@ class DavResourceTest {
        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_OK)
                .setHeader("DAV", "  1,  2 ,3,hyperactive-access"))
        dav.options()
        assertTrue(dav.capabilities.contains("1"))
        assertTrue(dav.capabilities.contains("2"))
        assertTrue(dav.capabilities.contains("3"))
        assertTrue(dav.capabilities.contains("hyperactive-access"))
        val response = dav.options()
        assertTrue(response.capabilities.contains("1"))
        assertTrue(response.capabilities.contains("2"))
        assertTrue(response.capabilities.contains("3"))
        assertTrue(response.capabilities.contains("hyperactive-access"))

        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_OK))
        dav.options()
        assertTrue(dav.capabilities.isEmpty())
        assertTrue(dav.options().capabilities.isEmpty())
    }

    @Test
@@ -79,10 +81,10 @@ class DavResourceTest {
                .setHeader("ETag", "W/\"My Weak ETag\"")
                .setHeader("Content-Type", "application/x-test-result")
                .setBody(sampleText))
        var body = dav.get("*/*")
        assertEquals(sampleText, body.string())
        assertEquals("My Weak ETag", dav.properties[GetETag::class.java]?.eTag)
        assertEquals("application/x-test-result", dav.properties[GetContentType::class.java]?.type)
        val (responseOK, bodyOK) = dav.get("*/*")
        assertEquals(sampleText, bodyOK.string())
        assertEquals("My Weak ETag", responseOK[GetETag::class.java]?.eTag)
        assertEquals("application/x-test-result", responseOK[GetContentType::class.java]?.type)

        var rq = mockServer.takeRequest()
        assertEquals("GET", rq.method)
@@ -98,9 +100,9 @@ class DavResourceTest {
                .setResponseCode(HttpURLConnection.HTTP_OK)
                .setHeader("ETag", "\"StrongETag\"")
                .setBody(sampleText))
        body = dav.get("*/*")
        assertEquals(sampleText, body.string())
        assertEquals("StrongETag", dav.properties[GetETag::class.java]?.eTag)
        val (response302, body302) = dav.get("*/*")
        assertEquals(sampleText, body302.string())
        assertEquals("StrongETag", response302[GetETag::class.java]?.eTag)

        mockServer.takeRequest()
        rq = mockServer.takeRequest()
@@ -108,12 +110,11 @@ class DavResourceTest {
        assertEquals("/target", rq.path)

        // 200 OK without ETag in response
        dav.properties[GetETag.NAME] = GetETag("test")
        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_OK)
                .setBody(sampleText))
        dav.get("*/*")
        assertNull(dav.properties[GetETag::class.java])
        val (responseWithoutETag, _) = dav.get("*/*")
        assertNull(responseWithoutETag[GetETag::class.java])
    }

    @Test
@@ -127,8 +128,9 @@ class DavResourceTest {
        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_CREATED)
                .setHeader("ETag", "W/\"Weak PUT ETag\""))
        assertFalse(dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), null, false))
        assertEquals("Weak PUT ETag", dav.properties[GetETag::class.java]?.eTag)
        val (response201, redirect201) = dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), null, false)
        assertFalse(redirect201)
        assertEquals("Weak PUT ETag", response201[GetETag::class.java]?.eTag)

        var rq = mockServer.takeRequest()
        assertEquals("PUT", rq.method)
@@ -142,9 +144,10 @@ class DavResourceTest {
                .setHeader("Location", "/target"))
        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_NO_CONTENT))
        assertTrue(dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), null, true))
        val (response301_204, redirect301_204) = dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), null, true)
        assertTrue(redirect301_204)
        assertEquals(url.resolve("/target"), dav.location)
        assertNull(dav.properties[GetETag::class.java])
        assertNull(response301_204[GetETag::class.java])

        mockServer.takeRequest()
        rq = mockServer.takeRequest()
@@ -154,10 +157,12 @@ class DavResourceTest {
        // precondition: If-Match, 412 Precondition Failed
        mockServer.enqueue(MockResponse()
                .setResponseCode(HttpURLConnection.HTTP_PRECON_FAILED))
        try {
            dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), "ExistingETag", false)
        val (_, _) = try {
            val pair = dav.put(RequestBody.create(MediaType.parse("text/plain"), sampleText), "ExistingETag", false)
            fail()
            pair
        } catch(e: PreconditionFailedException) {
            Pair(null, null)
        }
        rq = mockServer.takeRequest()
        assertEquals("\"ExistingETag\"", rq.getHeader("If-Match"))
@@ -316,8 +321,8 @@ class DavResourceTest {
                        "    </propstat>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(0, ResourceType.NAME)
        assertNull(dav.properties[ResourceType::class.java])
        val invalidStatus = dav.propfind(0, ResourceType.NAME)
        assertNull(invalidStatus[ResourceType::class.java])


        /*** POSITIVE TESTS ***/
@@ -327,9 +332,9 @@ class DavResourceTest {
                .setResponseCode(207)
                .setHeader("Content-Type", "application/xml; charset=utf-8")
                .setBody("<multistatus xmlns='DAV:'></multistatus>"))
        dav.propfind(0, ResourceType.NAME)
        assertEquals(0, dav.properties.size())
        assertEquals(0, dav.members.size)
        val emptyResponse = dav.propfind(0, ResourceType.NAME)
        assertEquals(0, emptyResponse.properties.size)
        assertEquals(0, emptyResponse.members.size)

        // multi-status response with <response>/<status> element indicating success
        mockServer.enqueue(MockResponse()
@@ -341,9 +346,9 @@ class DavResourceTest {
                        "    <status>HTTP/1.1 200 OK</status>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(0, ResourceType.NAME)
        assertEquals(0, dav.properties.size())
        assertEquals(0, dav.members.size)
        val responseStatus = dav.propfind(0, ResourceType.NAME)
        assertEquals(0, responseStatus.properties.size)
        assertEquals(0, responseStatus.members.size)

        // multi-status response with <response>/<propstat> element
        mockServer.enqueue(MockResponse()
@@ -362,9 +367,9 @@ class DavResourceTest {
                         "    </propstat>" +
                         "  </response>" +
                         "</multistatus>"))
        dav.propfind(0, ResourceType.NAME, DisplayName.NAME)
        assertEquals("My DAV Collection", dav.properties[DisplayName::class.java]?.displayName)
        assertEquals(0, dav.members.size)
        val responsePropstat = dav.propfind(0, ResourceType.NAME, DisplayName.NAME)
        assertEquals("My DAV Collection", responsePropstat[DisplayName::class.java]?.displayName)
        assertEquals(0, responsePropstat.members.size)

        // multi-status response for collection with several members; incomplete (not all <resourcetype>s listed)
        mockServer.enqueue(MockResponse()
@@ -428,26 +433,26 @@ class DavResourceTest {
                        "    </propstat>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(1, ResourceType.NAME, DisplayName.NAME)
        assertEquals(4, dav.members.size)
        val responseMembers = dav.propfind(1, ResourceType.NAME, DisplayName.NAME)
        assertEquals(4, responseMembers.members.size)
        val ok = BooleanArray(4)
        for (member in dav.members)
            when (member.location) {
        for (member in responseMembers.members)
            when (member.url) {
                url.resolve("/dav/subcollection/") -> {
                    assertTrue(member.properties[ResourceType::class.java]!!.types.contains(ResourceType.COLLECTION))
                    assertEquals("A Subfolder", member.properties[DisplayName::class.java]?.displayName)
                    assertTrue(member[ResourceType::class.java]!!.types.contains(ResourceType.COLLECTION))
                    assertEquals("A Subfolder", member[DisplayName::class.java]?.displayName)
                    ok[0] = true
                }
                url.resolve("/dav/uid@host:file") -> {
                    assertEquals("Absolute path with @ and :", member.properties[DisplayName::class.java]?.displayName)
                    assertEquals("Absolute path with @ and :", member[DisplayName::class.java]?.displayName)
                    ok[1] = true
                }
                url.resolve("/dav/relative-uid@host.file") -> {
                    assertEquals("Relative path with @", member.properties[DisplayName::class.java]?.displayName)
                    assertEquals("Relative path with @", member[DisplayName::class.java]?.displayName)
                    ok[2] = true
                }
                url.resolve("/dav/relative:colon.vcf") -> {
                    assertEquals("Relative path with colon", member.properties[DisplayName::class.java]?.displayName)
                    assertEquals("Relative path with colon", member[DisplayName::class.java]?.displayName)
                    ok[3] = true
                }
            }
@@ -478,9 +483,9 @@ class DavResourceTest {
                        "    </propstat>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(0, ResourceType.NAME, DisplayName.NAME)
        assertTrue(dav.properties[ResourceType::class.java]!!.types.contains(ResourceType.COLLECTION))
        assertEquals("My DAV Collection", dav.properties[DisplayName::class.java]?.displayName)
        val some404Prop = dav.propfind(0, ResourceType.NAME, DisplayName.NAME)
        assertTrue(some404Prop[ResourceType::class.java]!!.types.contains(ResourceType.COLLECTION))
        assertEquals("My DAV Collection", some404Prop[DisplayName::class.java]?.displayName)

        // multi-status response with <propstat> that doesn't contain <status> (=> assume 200 OK)
        mockServer.enqueue(MockResponse()
@@ -496,60 +501,8 @@ class DavResourceTest {
                        "    </propstat>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(0, DisplayName.NAME)
        assertEquals("Without Status", dav.properties[DisplayName::class.java]?.displayName)
    }

    @Test
    fun testPropfindUpdateProperties() {
        val url = sampleUrl()
        val dav = DavResource(httpClient, url)

        mockServer.enqueue(MockResponse()
                .setResponseCode(207)
                .setHeader("Content-Type", "application/xml; charset=utf-8")
                .setBody("<multistatus xmlns='DAV:'>" +
                         "  <response>" +
                         "    <href>/dav</href>" +
                         "    <propstat>" +
                         "      <prop>" +
                         "        <displayname>DisplayName 1</displayname>" +
                         "        <getetag>ETag 1</getetag>" +
                         "        <getctag xmlns=\"http://calendarserver.org/ns/\">CTag 1</getctag>" +
                         "      </prop>" +
                         "      <status>HTTP/1.1 200 OK</status>" +
                         "    </propstat>" +
                         "  </response>" +
                         "</multistatus>"))
        dav.propfind(0, DisplayName.NAME, GetETag.NAME, GetCTag.NAME)
        assertEquals("DisplayName 1", dav.properties[DisplayName::class.java]?.displayName)
        assertEquals("ETag 1", dav.properties[GetETag::class.java]?.eTag)
        assertEquals("CTag 1", dav.properties[GetCTag::class.java]?.cTag)

        mockServer.enqueue(MockResponse()
                .setResponseCode(207)
                .setHeader("Content-Type", "application/xml; charset=utf-8")
                .setBody("<multistatus xmlns='DAV:'>" +
                        "  <response>" +
                        "    <href>/dav</href>" +
                        "    <propstat>" +
                        "      <prop>" +
                        "        <displayname>DisplayName 2</displayname>" +
                        "      </prop>" +
                        "      <status>HTTP/1.1 200 OK</status>" +
                        "    </propstat>" +
                        "    <propstat>" +
                        "      <prop>" +
                        "        <getetag/>" +
                        "      </prop>" +
                        "      <status>HTTP/1.1 404 Not Found</status>" +
                        "    </propstat>" +
                        "  </response>" +
                        "</multistatus>"))
        dav.propfind(0, ResourceType.NAME, DisplayName.NAME)
        assertEquals("DisplayName 2", dav.properties[DisplayName::class.java]?.displayName)
        assertNull(dav.properties[GetETag::class.java])
        assertEquals("CTag 1", dav.properties[GetCTag::class.java]?.cTag)
        val propstatWithoutStatus = dav.propfind(0, DisplayName.NAME)
        assertEquals("Without Status", propstatWithoutStatus[DisplayName::class.java]?.displayName)
    }

}
+13 −6
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ package at.bitfire.dav4android

import at.bitfire.dav4android.exception.DavException
import at.bitfire.dav4android.exception.HttpException
import at.bitfire.dav4android.exception.InvalidDavResponseException
import okhttp3.*
import java.io.IOException
import java.io.StringWriter
@@ -33,7 +34,7 @@ class DavAddressBook @JvmOverloads constructor(
     * @throws HttpException on HTTP error
     * @throws DavException on DAV error
     */
    fun addressbookQuery() {
    fun addressbookQuery(): DavResponse {
        /* <!ELEMENT addressbook-query ((DAV:allprop |
                                         DAV:propname |
                                         DAV:prop)?, filter, limit?)>
@@ -64,8 +65,11 @@ class DavAddressBook @JvmOverloads constructor(
        checkStatus(response, false)
        assertMultiStatus(response)

        resetMembers()
        response.body()?.charStream()?.use { processMultiStatus(it) }
        response.body()?.charStream()?.use {
            return processMultiStatus(it)
        }

        throw InvalidDavResponseException("Didn't receive 207 Multi-status response on REPORT addressbook-queryys")
    }

    /**
@@ -74,7 +78,7 @@ class DavAddressBook @JvmOverloads constructor(
     * @throws HttpException on HTTP error
     * @throws DavException on DAV error
     */
    fun multiget(urls: List<HttpUrl>, vCard4: Boolean) {
    fun multiget(urls: List<HttpUrl>, vCard4: Boolean): DavResponse {
        /* <!ELEMENT addressbook-multiget ((DAV:allprop |
                                            DAV:propname |
                                            DAV:prop)?,
@@ -116,8 +120,11 @@ class DavAddressBook @JvmOverloads constructor(
        checkStatus(response, false)
        assertMultiStatus(response)

        resetMembers()
        response.body()?.charStream()?.use { processMultiStatus(it) }
        response.body()?.charStream()?.use {
            return processMultiStatus(it)
        }

        throw InvalidDavResponseException("Didn't receive 207 Multi-status response on REPORT addressbook-multiget")
    }

}
+13 −6
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ package at.bitfire.dav4android

import at.bitfire.dav4android.exception.DavException
import at.bitfire.dav4android.exception.HttpException
import at.bitfire.dav4android.exception.InvalidDavResponseException
import okhttp3.*
import java.io.IOException
import java.io.StringWriter
@@ -40,7 +41,7 @@ class DavCalendar @JvmOverloads constructor(
     * @throws HttpException on HTTP error
     * @throws DavException on DAV error
     */
    fun calendarQuery(component: String, start: Date?, end: Date?) {
    fun calendarQuery(component: String, start: Date?, end: Date?): DavResponse {
        /* <!ELEMENT calendar-query ((DAV:allprop |
                                      DAV:propname |
                                      DAV:prop)?, filter, timezone?)>
@@ -91,8 +92,11 @@ class DavCalendar @JvmOverloads constructor(
        checkStatus(response, false)
        assertMultiStatus(response)

        resetMembers()
        response.body()?.charStream()?.use { processMultiStatus(it) }
        response.body()?.charStream()?.use {
            return processMultiStatus(it)
        }

        throw InvalidDavResponseException("Didn't receive 207 Multi-status response on REPORT calendar-query")
    }

    /**
@@ -101,7 +105,7 @@ class DavCalendar @JvmOverloads constructor(
     * @throws HttpException on HTTP error
     * @throws DavException on DAV error
     */
    fun multiget(urls: List<HttpUrl>) {
    fun multiget(urls: List<HttpUrl>): DavResponse {
        /* <!ELEMENT calendar-multiget ((DAV:allprop |
                                        DAV:propname |
                                        DAV:prop)?, DAV:href+)>
@@ -137,8 +141,11 @@ class DavCalendar @JvmOverloads constructor(
        checkStatus(response, false)
        assertMultiStatus(response)

        resetMembers()
        response.body()?.charStream()?.use { processMultiStatus(it) }
        response.body()?.charStream()?.use {
            return processMultiStatus(it)
        }

        throw InvalidDavResponseException("Didn't receive 207 Multi-status response on REPORT calendar-multiget")
    }

}
Loading