Skip to content

Commit a8d7560

Browse files
colin-lamedcolin-lamed
committed
BDOG-175 tidy
1 parent 00b8f60 commit a8d7560

File tree

5 files changed

+73
-71
lines changed

5 files changed

+73
-71
lines changed

‎app/uk/gov/hmrc/cataloguefrontend/DependencyExplorerController.scala

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import play.api.http.HttpEntity
2626
import play.api.i18n.MessagesProvider
2727
import play.api.mvc.{Action, AnyContent, MessagesControllerComponents, ResponseHeader, Result}
2828
import uk.gov.hmrc.cataloguefrontend.connector.{SlugInfoFlag, TeamsAndRepositoriesConnector}
29-
import uk.gov.hmrc.cataloguefrontend.connector.model.{BobbyVersionRange, ServiceWithDependency, Version, VersionOp}
29+
import uk.gov.hmrc.cataloguefrontend.connector.model.{BobbyVersionRange, ServiceWithDependency, Version}
3030
import uk.gov.hmrc.cataloguefrontend.{ routes => appRoutes }
3131
import uk.gov.hmrc.cataloguefrontend.service.DependenciesService
3232
import uk.gov.hmrc.cataloguefrontend.util.CsvUtils
@@ -61,23 +61,29 @@ class DependencyExplorerController @Inject()(
6161
// first preserve old API
6262
if (request.queryString.contains("versionOp")) {
6363
(for {
64-
version <- EitherT.fromOption[Future](request.queryString.get("version").flatMap(_.headOption).flatMap(Version.parse), Redirect(appRoutes.DependencyExplorerController.landing))
65-
versionOp <- EitherT.fromOption[Future](request.queryString.get("versionOp").flatMap(_.headOption).flatMap(VersionOp.parse), Redirect(appRoutes.DependencyExplorerController.landing))
66-
versionRangeStr = versionOp match {
67-
case VersionOp.Gte => s"[$version,)"
68-
case VersionOp.Lte => s"(,$version]"
69-
case VersionOp.Eq => s"[$version]"
70-
}
71-
queryString = request.queryString - "version" - "versionOp" + ("versionRange" -> Seq(versionRangeStr))
72-
73-
// updating queryString with play, does not update uri!?
74-
//} yield Redirect(request.target.withQueryString(queryString).uri)
75-
//} yield Redirect(request.copy(queryString = queryString).uri)
76-
queryStr = queryString.flatMap { case (k, vs) =>
77-
vs.map(v => java.net.URLEncoder.encode(k, "UTF-8") + "=" + java.net.URLEncoder.encode(v, "UTF-8"))
78-
}.mkString("?", "&", "")
64+
version <- EitherT.fromOption[Future](
65+
request.queryString.get("version").flatMap(_.headOption).flatMap(Version.parse)
66+
, Redirect(appRoutes.DependencyExplorerController.landing)
67+
)
68+
versionRange <- EitherT.fromOption[Future](
69+
request.queryString.get("versionOp").flatMap(_.headOption).flatMap { versionOp =>
70+
PartialFunction.condOpt(versionOp) {
71+
case ">=" => s"[$version,)"
72+
case "<=" => s"(,$version]"
73+
case "==" => s"[$version]"
74+
}
75+
}
76+
, Redirect(appRoutes.DependencyExplorerController.landing)
77+
)
78+
queryString = request.queryString - "version" - "versionOp" + ("versionRange" -> Seq(versionRange))
79+
80+
// updating request with new querystring does not update uri!? - build uri manually...
81+
queryStr = queryString.flatMap { case (k, vs) =>
82+
vs.map(v => java.net.URLEncoder.encode(k, "UTF-8") + "=" + java.net.URLEncoder.encode(v, "UTF-8"))
83+
}.mkString("?", "&", "")
7984
} yield Redirect(request.path + queryStr)
8085
).merge
86+
// else continue to new API
8187
} else search2(request)
8288
}
8389

‎app/uk/gov/hmrc/cataloguefrontend/connector/model/Dependencies.scala

Lines changed: 46 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,10 @@ case class BobbyVersionRange(
8989
} else None
9090
}
9191

92-
def isMatch(v: Version): Boolean = {
92+
def includes(v: Version): Boolean = {
9393
val qualFilter: Function1[Version, Boolean] = qualifier match {
9494
case Some(qual) => _.toString.contains(qual)
95-
case None => _ => false
95+
case None => _ => true
9696
}
9797
val lbFilter: Function1[Version, Boolean] = lowerBound match {
9898
case Some(BobbyVersion(version, true)) => _ >= version
@@ -104,14 +104,16 @@ case class BobbyVersionRange(
104104
case Some(BobbyVersion(version, false)) => _ < version
105105
case None => _ => true
106106
}
107-
qualFilter(v) || (lbFilter(v) && ubFilter(v))
107+
qualFilter(v) && lbFilter(v) && ubFilter(v)
108108
}
109+
110+
override def toString: String = range
109111
}
110112

111113
object BobbyVersionRange {
112114

113115
private val fixed = """^\[(\d+\.\d+.\d+)\]""".r
114-
private val fixedUpper = """^\(,?(\d+\.\d+.\d+)[\]\)]""".r
116+
private val fixedUpper = """^[\[\(],?(\d+\.\d+.\d+)[\]\)]""".r
115117
private val fixedLower = """^[\[\(](\d+\.\d+.\d+),[\]\)]""".r
116118
private val rangeRegex = """^[\[\(](\d+\.\d+.\d+),(\d+\.\d+.\d+)[\]\)]""".r
117119
private val qualifier = """^\[[-\*]+(.*)\]""".r
@@ -121,42 +123,54 @@ object BobbyVersionRange {
121123

122124
PartialFunction.condOpt(trimmedRange) {
123125
case fixed(v) =>
124-
val fixed = Version.parse(v).map(BobbyVersion(_, inclusive = true)) // TODO fail, if invalid version
125-
BobbyVersionRange(
126-
lowerBound = fixed
127-
, upperBound = fixed
128-
, qualifier = None
129-
, range = trimmedRange
130-
)
126+
Version.parse(v).map(BobbyVersion(_, inclusive = true))
127+
.map { fixed =>
128+
BobbyVersionRange(
129+
lowerBound = Some(fixed)
130+
, upperBound = Some(fixed)
131+
, qualifier = None
132+
, range = trimmedRange
133+
)
134+
}
131135
case fixedUpper(v) =>
132-
BobbyVersionRange(
133-
lowerBound = None
134-
, upperBound = Version.parse(v).map(BobbyVersion(_, inclusive = trimmedRange.endsWith("]"))) // TODO fail, if invalid version
135-
, qualifier = None
136-
, range = trimmedRange
137-
)
136+
Version.parse(v).map(BobbyVersion(_, inclusive = trimmedRange.endsWith("]")))
137+
.map { ub =>
138+
BobbyVersionRange(
139+
lowerBound = None
140+
, upperBound = Some(ub)
141+
, qualifier = None
142+
, range = trimmedRange
143+
)
144+
}
138145
case fixedLower(v) =>
139-
BobbyVersionRange(
140-
lowerBound = Version.parse(v).map(BobbyVersion(_, inclusive = trimmedRange.startsWith("["))) // TODO fail, if invalid version
141-
, upperBound = None
142-
, qualifier = None
143-
, range = trimmedRange
144-
)
146+
Version.parse(v).map(BobbyVersion(_, inclusive = trimmedRange.startsWith("[")))
147+
.map { lb =>
148+
BobbyVersionRange(
149+
lowerBound = Some(lb)
150+
, upperBound = None
151+
, qualifier = None
152+
, range = trimmedRange
153+
)
154+
}
145155
case rangeRegex(v1, v2) =>
146-
BobbyVersionRange(
147-
lowerBound = Version.parse(v1).map(BobbyVersion(_, inclusive = trimmedRange.startsWith("["))) // TODO fail, if invalid version
148-
, upperBound = Version.parse(v2).map(BobbyVersion(_, inclusive = trimmedRange.endsWith("]"))) // TODO fail, if invalid version
149-
, qualifier = None
150-
, range = trimmedRange
151-
)
156+
for {
157+
lb <- Version.parse(v1).map(BobbyVersion(_, inclusive = trimmedRange.startsWith("[")))
158+
ub <- Version.parse(v2).map(BobbyVersion(_, inclusive = trimmedRange.endsWith("]")))
159+
} yield
160+
BobbyVersionRange(
161+
lowerBound = Some(lb)
162+
, upperBound = Some(ub)
163+
, qualifier = None
164+
, range = trimmedRange
165+
)
152166
case qualifier(q) if q.length() > 1 =>
153-
BobbyVersionRange(
167+
Some(BobbyVersionRange(
154168
lowerBound = None
155169
, upperBound = None
156170
, qualifier = Some(q)
157171
, range = trimmedRange
158-
)
159-
}
172+
))
173+
}.flatten
160174
}
161175

162176
def apply(range: String): BobbyVersionRange =
@@ -273,24 +287,6 @@ object Version {
273287
}
274288

275289

276-
trait VersionOp {
277-
def s: String
278-
}
279-
object VersionOp {
280-
case object Gte extends VersionOp { val s = ">=" }
281-
case object Lte extends VersionOp { val s = "<=" }
282-
case object Eq extends VersionOp { val s = "==" }
283-
284-
def parse(s: String): Option[VersionOp] =
285-
s match {
286-
case Gte.s => Some(Gte)
287-
case Lte.s => Some(Lte)
288-
case Eq.s => Some(Eq)
289-
case _ => None
290-
}
291-
}
292-
293-
294290
case class ServiceWithDependency(
295291
slugName : String,
296292
slugVersion : String,

‎app/uk/gov/hmrc/cataloguefrontend/service/DependenciesService.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class DependenciesService @Inject()(
6262
}
6363
}
6464
.map { l =>
65-
l.filter(_.depSemanticVersion.map(versionRange.isMatch).getOrElse(true)) // include invalid semanticVersion in results
65+
l.filter(_.depSemanticVersion.map(versionRange.includes).getOrElse(true)) // include invalid semanticVersion in results
6666
}
6767
.map(_
6868
.sortBy(_.slugName)

‎app/views/BobbyExplorerPage.scala.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ <h1>Bobby Rules</h1>
4848
<tr>
4949
<th class="col-xs-3">@artifactType</th>
5050
<th class="col-xs-2" colspan="3">Banned Versions</th>
51-
<th class="col-xs-5">Reason</th>
51+
<th class="col-xs-4">Reason</th>
5252
<th class="col-xs-1">Active from</th>
53-
<th class="col-xs-1"></th>
53+
<th class="col-xs-2"></th>
5454
</tr>
5555
</thead>
5656
<tbody>
@@ -78,7 +78,7 @@ <h1>Bobby Rules</h1>
7878
}
7979
<td>@rule.reason</td>
8080
<td>@rule.from</td>
81-
<td>@if(inclViewSlugsLink) { <a href="@DependencyExplorerController.search(flag = SlugInfoFlag.Latest, group = rule.group, artefact = rule.artefact, versionRange = rule.range)">See affected slugs</a> }
81+
<td>@if(inclViewSlugsLink) { <a href="@DependencyExplorerController.search(flag = SlugInfoFlag.Latest, group = rule.group, artefact = rule.artefact, versionRange = rule.range)">Affected slugs</a> }
8282
</td>
8383
</tr>
8484
}

‎app/views/DependencyExplorerPage.scala.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
@import play.api.mvc.Call
1818
@import uk.gov.hmrc.cataloguefrontend.connector.{SlugInfoFlag, Team}
19-
@import uk.gov.hmrc.cataloguefrontend.connector.model.{BobbyVersion, BobbyVersionRange, GroupArtefacts, ServiceWithDependency, VersionOp}
19+
@import uk.gov.hmrc.cataloguefrontend.connector.model.{BobbyVersion, BobbyVersionRange, GroupArtefacts, ServiceWithDependency}
2020
@import uk.gov.hmrc.cataloguefrontend.DependencyExplorerController.PieData
2121
@import uk.gov.hmrc.cataloguefrontend.ViewMessages
2222
@import uk.gov.hmrc.cataloguefrontend.routes

0 commit comments

Comments
 (0)