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.

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

Fix issue #107 where /path and path were being considered the same #113

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/main/scala/com/netaporter/uri/PathPart.scala
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,6 @@ case class MatrixParams(part: String, params: ParamSeq) extends PathPart with Pa
object PathPart {
def apply(path: String, matrixParams: ParamSeq = Seq.empty) =
if(matrixParams.isEmpty) new StringPathPart(path) else MatrixParams(path, matrixParams)

val Empty = new StringPathPart("")
}
56 changes: 42 additions & 14 deletions src/main/scala/com/netaporter/uri/Uri.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,24 @@ case class Uri (
password: Option[String],
host: Option[String],
port: Option[Int],
pathParts: Seq[PathPart],
rawPathParts: Seq[PathPart],
query: QueryString,
fragment: Option[String]
) {

lazy val hostParts: Seq[String] =
host.map(h => h.split('.').toVector).getOrElse(Vector.empty)

/**
* Most of the time, we want to see the same path parts for:
* /a/b/c and a/b/c
*
* `rawPathParts` has an leading empty PathPart for /a/b/c
*/
lazy val pathParts: Seq[PathPart] = {
if(rawPathParts.head.part.isEmpty) rawPathParts.tail else rawPathParts
}

def subdomain = hostParts.headOption

def pathPartOption(name: String) =
Expand All @@ -39,16 +49,24 @@ case class Uri (
}

def addMatrixParam(pp: String, k: String, v: String) = copy (
pathParts = pathParts.map {
rawPathParts = rawPathParts.map {
case p: PathPart if p.part == pp => p.addParam(k -> Some(v))
case x => x
}
)

def addMatrixParam(k: String, v: String) = copy (
pathParts = pathParts.dropRight(1) :+ pathParts.last.addParam(k -> Some(v))
rawPathParts = rawPathParts.dropRight(1) :+ rawPathParts.last.addParam(k -> Some(v))
)

def uriType: UriType = {
if(host.isDefined) {
if(scheme.isDefined) Absolute else ProtocolRelative
} else {
if(rawPathParts.head.part.isEmpty) SiteRootRelative else DocumentRelative
}
}

/**
* Adds a new Query String parameter key-value pair. If the value for the Query String parmeter is None, then this
* Query String parameter will not be rendered in calls to toString or toStringRaw
Expand Down Expand Up @@ -132,8 +150,8 @@ case class Uri (
* @return String containing the path for this Uri
*/
def path(implicit c: UriConfig = UriConfig.default) =
if(pathParts.isEmpty) ""
else "/" + pathParts.map(_.partToString(c)).mkString("/")
if(rawPathParts.isEmpty) ""
else rawPathParts.map(_.partToString(c)).mkString("/")

def queryStringRaw(implicit c: UriConfig = UriConfig.default) =
queryString(c.withNoEncoding)
Expand Down Expand Up @@ -297,6 +315,12 @@ case class Uri (
def toURI(implicit c: UriConfig = UriConfig.conservative) = new java.net.URI(toString())
}

sealed trait UriType
case object Absolute extends UriType // e.g. http://test.com/index.html
case object ProtocolRelative extends UriType // e.g. //test.com/index.html
case object SiteRootRelative extends UriType // e.g. /index.html
case object DocumentRelative extends UriType // e.g. index.html

object Uri {

def parse(s: CharSequence)(implicit config: UriConfig = UriConfig.default): Uri =
Expand All @@ -314,15 +338,19 @@ object Uri {
pathParts: Seq[PathPart] = Seq.empty,
query: QueryString = EmptyQueryString,
fragment: String = null) = {
new Uri(Option(scheme),
Option(user),
Option(password),
Option(host),
if(port > 0) Some(port) else None,
pathParts,
query,
Option(fragment)
)

//For absolute URLs, add in an artificial path part to add the leading slash
val pathPartsMaybeLeadingSlash = if(host != null) PathPart.Empty +: pathParts else pathParts

new Uri(Option(scheme),
Option(user),
Option(password),
Option(host),
if(port > 0) Some(port) else None,
pathPartsMaybeLeadingSlash,
query,
Option(fragment)
)
}

def empty = apply()
Expand Down
12 changes: 8 additions & 4 deletions src/main/scala/com/netaporter/uri/dsl/UriDsl.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.netaporter.uri.dsl

import com.netaporter.uri.{Uri, StringPathPart}
import com.netaporter.uri.{PathPart, Uri, StringPathPart}

/**
* Value class to add DSL functionality to Uris
Expand Down Expand Up @@ -47,7 +47,11 @@ class UriDsl(val uri: Uri) extends AnyVal {
* @param pp The path part
* @return A new Uri with this path part appended
*/
def /(pp: String) = uri.copy(pathParts = uri.pathParts :+ StringPathPart(pp))
def /(pp: String) = {
// If there is no path already, add an empty path segment to create the leading slash
val before = if(uri.rawPathParts.isEmpty) Seq(PathPart.Empty) else uri.rawPathParts
uri.copy(rawPathParts = before :+ StringPathPart(pp))
}

/**
* Operator precedence in Scala will mean that our DSL will not always be executed left to right.
Expand All @@ -62,7 +66,7 @@ class UriDsl(val uri: Uri) extends AnyVal {
*
* (see Scala Reference - 6.12.3 Infix Operations: http://www.scala-lang.org/docu/files/ScalaReference.pdf)
*
* To handle cases where the right hard part of the DSL is executed first, we turn that into a Uri, and merge
* To handle cases where the right hand part of the DSL is executed first, we turn that into a Uri, and merge
* it with the left had side. It is assumed the right hand Uri is generated from this DSL only to add path
* parts, query parameters or to overwrite the fragment
*
Expand All @@ -71,7 +75,7 @@ class UriDsl(val uri: Uri) extends AnyVal {
*/
private def merge(other: Uri) =
uri.copy(
pathParts = uri.pathParts ++ other.pathParts,
rawPathParts = uri.rawPathParts ++ other.rawPathParts,
query = uri.query.addParams(other.query),
fragment = other.fragment.orElse(uri.fragment)
)
Expand Down
21 changes: 7 additions & 14 deletions src/main/scala/com/netaporter/uri/parsing/DefaultUriParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,10 @@ class DefaultUriParser(val input: ParserInput, conf: UriConfig) extends Parser w
}

/**
* A sequence of path parts that MUST start with a slash
* A sequence of path parts
*/
def _abs_path: Rule1[Vector[PathPart]] = rule {
zeroOrMore("/" ~ _pathSegment) ~> extractPathParts
}

/**
* A sequence of path parts optionally starting with a slash
*/
def _rel_path: Rule1[Vector[PathPart]] = rule {
optional("/") ~ zeroOrMore(_pathSegment).separatedBy("/") ~> extractPathParts
def _path: Rule1[Vector[PathPart]] = rule {
zeroOrMore(_pathSegment).separatedBy("/") ~> extractPathParts
}

def _queryParam: Rule1[Param] = rule {
Expand All @@ -63,15 +56,15 @@ class DefaultUriParser(val input: ParserInput, conf: UriConfig) extends Parser w
}

def _abs_uri: Rule1[Uri] = rule {
_scheme ~ "://" ~ optional(_authority) ~ _abs_path ~ optional(_queryString) ~ optional(_fragment) ~> extractAbsUri
_scheme ~ "://" ~ optional(_authority) ~ _path ~ optional(_queryString) ~ optional(_fragment) ~> extractAbsUri
}

def _protocol_rel_uri: Rule1[Uri] = rule {
"//" ~ optional(_authority) ~ _abs_path ~ optional(_queryString) ~ optional(_fragment) ~> extractProtocolRelUri
"//" ~ optional(_authority) ~ _path ~ optional(_queryString) ~ optional(_fragment) ~> extractProtocolRelUri
}

def _rel_uri: Rule1[Uri] = rule {
_rel_path ~ optional(_queryString) ~ optional(_fragment) ~> extractRelUri
_path ~ optional(_queryString) ~ optional(_fragment) ~> extractRelUri
}

def _uri: Rule1[Uri] = rule {
Expand Down Expand Up @@ -113,7 +106,7 @@ class DefaultUriParser(val input: ParserInput, conf: UriConfig) extends Parser w
password = authority.flatMap(_.password),
host = authority.map(_.host),
port = authority.flatMap(_.port),
pathParts = pathParts,
rawPathParts = pathParts,
query = query.getOrElse(EmptyQueryString),
fragment = fragment
)
Expand Down
7 changes: 6 additions & 1 deletion src/test/scala/com/netaporter/uri/GithubIssueTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class GithubIssueTests extends FlatSpec with Matchers with OptionValues {

uri.scheme should equal (None)
uri.host should equal (None)
uri.path should equal ("/abc")
uri.path should equal ("abc")
}

"Github Issue #15" should "now be fixed. Empty Query String values are parsed" in {
Expand Down Expand Up @@ -173,4 +173,9 @@ class GithubIssueTests extends FlatSpec with Matchers with OptionValues {
val withPathAndQuery = p / "some/path/segments" ? ("returnUrl" -> "http://localhost:1234/some/path/segments")
withPathAndQuery.toString should equal("http://localhost:1234/some/path/segments?returnUrl=http://localhost:1234/some/path/segments")
}

"Github Issue #107" should "now be fixed. path and /path are now treated differently" in {
Uri.parse("path").toString should equal("path")
Uri.parse("/path").toString should equal("/path")
}
}