Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve Url equality #533

Closed
wants to merge 2 commits into from
Closed

Improve Url equality #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1a50e9
Improve `Url` equality
mickael-menu Jun 11, 2024
0525c35
Simplification
mickael-menu Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions readium/shared/src/main/java/org/readium/r2/shared/util/Url.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,6 @@ public sealed class Url : Parcelable {
override fun toString(): String =
uri.toString()

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as Url

if (uri.toString() != other.uri.toString()) return false

return true
}

override fun hashCode(): Int =
uri.toString().hashCode()

Expand Down Expand Up @@ -270,6 +259,19 @@ public class AbsoluteUrl private constructor(override val uri: Uri) : Url() {
*/
public fun toFile(): File? =
if (isFile) File(path!!) else null

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as AbsoluteUrl

return scheme == other.scheme &&
uri.authority == other.uri.authority &&
path == other.path &&
query == other.query &&
fragment == other.fragment
}
}

/**
Expand All @@ -294,6 +296,17 @@ public class RelativeUrl private constructor(override val uri: Uri) : Url() {
RelativeUrl(uri)
}
}

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (javaClass != other?.javaClass) return false

other as RelativeUrl

return path == other.path &&
query == other.query &&
fragment == other.fragment
}
}

/**
Expand Down
97 changes: 97 additions & 0 deletions readium/shared/src/test/java/org/readium/r2/shared/util/UrlTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import java.io.File
import java.net.URI
import java.net.URL
import kotlin.test.assertIs
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
Expand Down Expand Up @@ -399,4 +400,100 @@ class UrlTest {
assertEquals(params.allNamed("empty"), emptyList<String>())
assertEquals(params.allNamed("not-found"), emptyList<String>())
}

@Test
fun equality() {
assertNotEquals(
AbsoluteUrl("http://example.com/foo/bar")!! as Url,
RelativeUrl("foo/bar") as Url
)

// Paths must be equal.
assertEquals(
AbsoluteUrl("http://example.com/foo/bar")!!,
AbsoluteUrl("http://example.com/foo/bar")
)
assertEquals(RelativeUrl("foo/bar")!!, RelativeUrl("foo/bar"))
assertNotEquals(
AbsoluteUrl("http://example.com/foo/bar")!!,
AbsoluteUrl("http://example.com/foo/baz")
)
assertNotEquals(RelativeUrl("foo/bar")!!, RelativeUrl("foo/baz"))

// Paths is compared percent and entity-decoded.
assertEquals(
AbsoluteUrl("http://example.com/c%27est%20valide")!!,
AbsoluteUrl("http://example.com/c%27est%20valide")
)
assertEquals(
AbsoluteUrl("http://example.com/c%27est%20valide")!!,
AbsoluteUrl("http://example.com/c'est%20valide")
)
assertEquals(RelativeUrl("c%27est%20valide")!!, RelativeUrl("c%27est%20valide"))
assertEquals(RelativeUrl("c%27est%20valide")!!, RelativeUrl("c'est%20valide"))

// Authority must be equal.
assertEquals(AbsoluteUrl("http://example.com/foo")!!, AbsoluteUrl("http://example.com/foo"))
assertNotEquals(
AbsoluteUrl("http://example.com/foo")!!,
AbsoluteUrl("http://example.com:80/foo")
)
assertNotEquals(
AbsoluteUrl("http://example.com:443/foo")!!,
AbsoluteUrl("http://example.com:80/foo")
)
assertNotEquals(
AbsoluteUrl("http://example.com/foo")!!,
AbsoluteUrl("http://example.com:80/foo")
)
assertNotEquals(
AbsoluteUrl("http://example.com/foo")!!,
AbsoluteUrl("http://domain.com/foo")
)
assertNotEquals(
AbsoluteUrl("http://example.com/foo")!!,
AbsoluteUrl("http://user:[email protected]/foo")
)
assertNotEquals(
AbsoluteUrl("http://other:[email protected]/foo")!!,
AbsoluteUrl("http://user:[email protected]/foo")
)

// Order of query parameters is important.
assertNotEquals(
AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")!!,
AbsoluteUrl("http://example.com/foo/bar?b=b&a=a")
)
assertNotEquals(RelativeUrl("foo/bar?a=a&b=b")!!, RelativeUrl("foo/bar?b=b&a=a"))

// Content of parameters is important.
assertEquals(
AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")!!,
AbsoluteUrl("http://example.com/foo/bar?a=a&b=b")
)
assertNotEquals(
AbsoluteUrl("http://example.com/foo/bar?a=a")!!,
AbsoluteUrl("http://example.com/foo/bar?b=b")
)
assertNotEquals(RelativeUrl("foo/bar?a=a")!!, RelativeUrl("foo/bar?b=b"))

// Scheme is case insensitive.
assertEquals(AbsoluteUrl("http://example.com/foo")!!, AbsoluteUrl("HTTP://example.com/foo"))
assertNotEquals(
AbsoluteUrl("http://example.com/foo")!!,
AbsoluteUrl("https://example.com/foo")
)

// Fragment is relevant.
assertEquals(
AbsoluteUrl("http://example.com/foo#fragment")!!,
AbsoluteUrl("http://example.com/foo#fragment")
)
assertEquals(RelativeUrl("foo/bar#fragment")!!, RelativeUrl("foo/bar#fragment"))
assertNotEquals(
AbsoluteUrl("http://example.com/foo#fragment")!!,
AbsoluteUrl("http://example.com/foo#other")
)
assertNotEquals(RelativeUrl("foo/bar#fragment")!!, RelativeUrl("foo/bar#other"))
}
}
Loading