Skip to content

Commit ba2878f

Browse files
benjbenbenjben
committed
Address review feedback
1 parent a887bf0 commit ba2878f

File tree

3 files changed

+74
-64
lines changed

3 files changed

+74
-64
lines changed

‎modules/core/src/main/scala/com.snowplowanalytics.iglu/client/resolver/Resolver.scala

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
4040

4141
private val allIgluCentral: Set[String] = repos.collect {
4242
case Registry.Http(config, connection)
43-
if Option(connection).filter(_.uri.getHost.matches(""".*\biglucentral\b.*""")).isDefined =>
43+
if connection.uri.getHost.matches(""".*\biglucentral\b.*""") =>
4444
config.name
4545
}.toSet
4646

@@ -196,7 +196,7 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
196196
allIgluCentral.contains(repo)
197197
}
198198
(igluCentral.isEmpty || igluCentral.values.exists(
199-
_.errors.forall(_ == RegistryError.NotFound)
199+
_.errors.exists(_ == RegistryError.NotFound)
200200
)) && custom.values.flatMap(_.errors).forall(_ == RegistryError.NotFound)
201201
}
202202

@@ -215,11 +215,11 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
215215
F: Monad[F],
216216
L: RegistryLookup[F],
217217
C: Clock[F]
218-
): F[Either[SchemaResolutionError, List[SelfDescribingSchema[Json]]]] = {
218+
): F[Either[SchemaResolutionError, NonEmptyList[SelfDescribingSchema[Json]]]] = {
219219
def go(
220220
current: SchemaVer.Full,
221-
acc: List[SelfDescribingSchema[Json]]
222-
): F[Either[SchemaResolutionError, List[SelfDescribingSchema[Json]]]] = {
221+
acc: NonEmptyList[SelfDescribingSchema[Json]]
222+
): F[Either[SchemaResolutionError, NonEmptyList[SelfDescribingSchema[Json]]]] = {
223223
val currentSchemaKey = maxSchemaKey.copy(version = current)
224224
lookupSchema(currentSchemaKey).flatMap {
225225
case Left(e) =>
@@ -247,7 +247,20 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
247247
}
248248
}
249249

250-
go(SchemaVer.Full(maxSchemaKey.version.model, 0, 0), Nil)
250+
val firstSchemaKey =
251+
maxSchemaKey.copy(version = SchemaVer.Full(maxSchemaKey.version.model, 0, 0))
252+
253+
EitherT(lookupSchema(firstSchemaKey))
254+
.leftMap(SchemaResolutionError(firstSchemaKey, _))
255+
.flatMap { firstJson =>
256+
val acc = NonEmptyList.one(SelfDescribingSchema(SchemaMap(firstSchemaKey), firstJson))
257+
258+
if (maxSchemaKey.version.addition == 0 && maxSchemaKey.version.revision == 0)
259+
EitherT.rightT[F, SchemaResolutionError](acc)
260+
else
261+
EitherT(go(SchemaVer.Full(maxSchemaKey.version.model, 0, 1), acc))
262+
}
263+
.value
251264
}
252265

253266
/**

‎modules/core/src/test/scala/com.snowplowanalytics.iglu.client/resolver/ResolverResultSpec.scala

Lines changed: 20 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import scala.concurrent.duration._
2323

2424
// Cats
2525
import cats.Id
26+
import cats.data.NonEmptyList
2627
import cats.effect.IO
2728
import cats.effect.implicits._
2829
import cats.implicits._
@@ -228,16 +229,13 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
228229
val time = Instant.ofEpochMilli(3L)
229230
val responses = List(timeoutError, correctResult)
230231

231-
val httpRep =
232-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
233-
234232
implicit val cache = ResolverSpecHelpers.staticResolverCache
235233
implicit val clock = ResolverSpecHelpers.staticClock
236234
implicit val registryLookup: RegistryLookup[StaticLookup] =
237235
ResolverSpecHelpers.getLookup(responses, Nil)
238236

239237
val result = for {
240-
resolver <- Resolver.init[StaticLookup](10, None, httpRep)
238+
resolver <- Resolver.init[StaticLookup](10, None, Repos.httpRep)
241239
response1 <- resolver.lookupSchemaResult(schemaKey)
242240
response2 <- resolver.lookupSchemaResult(schemaKey)
243241
_ <- StaticLookup.addTime(600.milliseconds)
@@ -284,9 +282,6 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
284282
RegistryError.RepoFailure("Should never be reached").asLeft
285283
)
286284

287-
val httpRep =
288-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
289-
290285
implicit val cache = ResolverSpecHelpers.staticResolverCache
291286
implicit val clock = ResolverSpecHelpers.staticClock
292287
implicit val registryLookup: RegistryLookup[StaticLookup] =
@@ -298,7 +293,7 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
298293
.init[StaticLookup](
299294
10,
300295
Some(1.seconds),
301-
httpRep
296+
Repos.httpRep
302297
)
303298
_ <- resolver.lookupSchemaResult(schemaKey)
304299
_ <- StaticLookup.addTime(2.seconds)
@@ -337,14 +332,8 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
337332
val error4 = RegistryError.RepoFailure("Server segfault")
338333

339334
// Mocking repositories
340-
val httpRep1 = Registry.Http(
341-
Registry.Config("Mock Repo 1", 1, List("com.snowplowanalytics.iglu-test")),
342-
null
343-
)
344-
val httpRep2 = Registry.Http(
345-
Registry.Config("Mock Repo 2", 1, List("com.snowplowanalytics.iglu-test")),
346-
null
347-
)
335+
val httpRep1 = Repos.httpRep.copy(config = Repos.httpRep.config.copy(name = "Mock Repo 1"))
336+
val httpRep2 = Repos.httpRep.copy(config = Repos.httpRep.config.copy(name = "Mock Repo 2"))
348337

349338
implicit val cache = ResolverSpecHelpers.staticResolverCache
350339
implicit val clock = ResolverSpecHelpers.staticClock
@@ -479,16 +468,13 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
479468
Json.Null.asRight[RegistryError]
480469
val responses = List(schema, schema)
481470

482-
val httpRep =
483-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
484-
485471
implicit val cache = ResolverSpecHelpers.staticResolverCache
486472
implicit val clock = ResolverSpecHelpers.staticClock
487473
implicit val registryLookup: RegistryLookup[StaticLookup] =
488474
ResolverSpecHelpers.getLookup(responses, Nil)
489475

490476
val result = for {
491-
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), httpRep)
477+
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), Repos.httpRep)
492478
response1 <- resolver.lookupSchemaResult(schemaKey)
493479
_ <- StaticLookup.addTime(150.seconds) // ttl 200, delay 150
494480
response2 <- resolver.lookupSchemaResult(schemaKey)
@@ -517,16 +503,13 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
517503
Json.Null.asRight[RegistryError]
518504
val responses = List(schema, schema)
519505

520-
val httpRep =
521-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
522-
523506
implicit val cache = ResolverSpecHelpers.staticResolverCache
524507
implicit val clock = ResolverSpecHelpers.staticClock
525508
implicit val registryLookup: RegistryLookup[StaticLookup] =
526509
ResolverSpecHelpers.getLookup(responses, Nil)
527510

528511
val result = for {
529-
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), httpRep)
512+
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), Repos.httpRep)
530513
response1 <- resolver.lookupSchemaResult(schemaKey)
531514
_ <- StaticLookup.addTime(250.seconds) // ttl 200, delay 250
532515
response2 <- resolver.lookupSchemaResult(schemaKey)
@@ -668,16 +651,13 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
668651
val notFound = RegistryError.NotFound.asLeft[Json]
669652
val responses = List(notFound, schema)
670653

671-
val httpRep =
672-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
673-
674654
implicit val cache = ResolverSpecHelpers.staticResolverCache
675655
implicit val clock = ResolverSpecHelpers.staticClock
676656
implicit val registryLookup: RegistryLookup[StaticLookup] =
677657
ResolverSpecHelpers.getLookup(responses, Nil)
678658

679659
val result = for {
680-
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), httpRep)
660+
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), Repos.httpRep)
681661
response1 <- resolver.lookupSchemaResult(schemaKey)
682662
_ <- StaticLookup.addTime(150.seconds) // ttl 200, delay 150
683663
response2 <- resolver.lookupSchemaResult(schemaKey)
@@ -719,17 +699,15 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
719699
val notFound = RegistryError.NotFound.asLeft[Json]
720700
val responses = List(notFound, notFound, schema)
721701

722-
val repoName = "Mock Repo"
723-
val httpRep =
724-
Registry.Http(Registry.Config(repoName, 1, List("com.snowplowanalytics.iglu-test")), null)
702+
val repoName = Repos.httpRep.config.name
725703

726704
implicit val cache = ResolverSpecHelpers.staticResolverCache
727705
implicit val clock = ResolverSpecHelpers.staticClock
728706
implicit val registryLookup: RegistryLookup[StaticLookup] =
729707
ResolverSpecHelpers.getLookup(responses, Nil)
730708

731709
val result = for {
732-
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), httpRep)
710+
resolver <- Resolver.init[StaticLookup](10, Some(200.seconds), Repos.httpRep)
733711
response1 <- resolver.lookupSchemaResult(schemaKey)
734712
_ <- StaticLookup.addTime(250.seconds) // ttl 200, delay 250
735713
response2 <- resolver.lookupSchemaResult(schemaKey)
@@ -765,45 +743,45 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
765743

766744
import ResolverSpecHelpers.LookupSchemasUntil._
767745

768-
def testLookupUntil(maxSchemaKey: SchemaKey, expected: List[SelfDescribingSchema[Json]]) =
746+
def testLookupUntil(maxSchemaKey: SchemaKey, expected: NonEmptyList[SelfDescribingSchema[Json]]) =
769747
for {
770748
resolver <- mkResolver
771749
result <- resolver.lookupSchemasUntil(maxSchemaKey)
772750
} yield result must beRight.like { case schemas => schemas must beEqualTo(expected) }
773751

774752
def e20 = testLookupUntil(
775753
getUntilSchemaKey(1, 0, 0),
776-
List(until100)
754+
NonEmptyList.one(until100)
777755
)
778756

779757
def e21 = testLookupUntil(
780758
getUntilSchemaKey(1, 1, 0),
781-
List(until100, until110)
759+
NonEmptyList.of(until100, until110)
782760
)
783761

784762
def e22 = testLookupUntil(
785763
getUntilSchemaKey(1, 1, 1),
786-
List(until100, until110, until111)
764+
NonEmptyList.of(until100, until110, until111)
787765
)
788766

789767
def e23 = testLookupUntil(
790768
getUntilSchemaKey(1, 1, 2),
791-
List(until100, until110, until111, until112)
769+
NonEmptyList.of(until100, until110, until111, until112)
792770
)
793771

794772
def e24 = testLookupUntil(
795773
getUntilSchemaKey(1, 2, 0),
796-
List(until100, until110, until111, until112, until120)
774+
NonEmptyList.of(until100, until110, until111, until112, until120)
797775
)
798776

799777
def e25 = testLookupUntil(
800778
getUntilSchemaKey(1, 2, 1),
801-
List(until100, until110, until111, until112, until120, until121)
779+
NonEmptyList.of(until100, until110, until111, until112, until120, until121)
802780
)
803781

804782
def e26 = testLookupUntil(
805783
getUntilSchemaKey(1, 2, 2),
806-
List(until100, until110, until111, until112, until120, until121, until122)
784+
NonEmptyList.of(until100, until110, until111, until112, until120, until121, until122)
807785
)
808786

809787
def e27 = for {
@@ -836,11 +814,11 @@ class ResolverResultSpec extends Specification with ValidatedMatchers with CatsE
836814

837815
def e31 = testLookupUntil(
838816
getUntilSchemaKey(3, 0, 0),
839-
List(until300)
817+
NonEmptyList.one(until300)
840818
)
841819

842820
def e32 = testLookupUntil(
843821
getUntilSchemaKey(3, 1, 0),
844-
List(until300, until310)
822+
NonEmptyList.of(until300, until310)
845823
)
846824
}

‎modules/core/src/test/scala/com.snowplowanalytics.iglu.client/resolver/ResolverSpec.scala

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ object ResolverSpec {
6262
Registry.Config("Iglu Custom Repo", 10, List("com.acme")),
6363
Registry.HttpConnection(URI.create("http://iglu.acme.com"), None)
6464
)
65+
66+
val httpRep =
67+
Registry.Http(
68+
Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")),
69+
Registry.HttpConnection(URI.create("http://iglu.mock.org"), None)
70+
)
6571
}
6672
}
6773

@@ -90,6 +96,7 @@ class ResolverSpec extends Specification with CatsEffect {
9096
return false if there is no custom repo and Iglu Central ones return an error $e18
9197
return true if there is just one custom repo that returns NotFound $e19
9298
return false if there is just one custom repo that returns an error $e20
99+
return true if one Iglu Central repo returns 2 errors and the other one returns one error and one NotFound $e21
93100
"""
94101

95102
import ResolverSpec._
@@ -222,16 +229,13 @@ class ResolverSpec extends Specification with CatsEffect {
222229
val time = Instant.ofEpochMilli(3L)
223230
val responses = List(timeoutError, correctSchema)
224231

225-
val httpRep =
226-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
227-
228232
implicit val cache: CreateResolverCache[StaticLookup] = ResolverSpecHelpers.staticResolverCache
229233
implicit val clock: Clock[StaticLookup] = ResolverSpecHelpers.staticClock
230234
implicit val registryLookup: RegistryLookup[StaticLookup] =
231235
ResolverSpecHelpers.getLookup(responses, Nil)
232236

233237
val result = for {
234-
resolver <- Resolver.init[StaticLookup](10, None, httpRep)
238+
resolver <- Resolver.init[StaticLookup](10, None, Repos.httpRep)
235239
response1 <- resolver.lookupSchema(schemaKey)
236240
response2 <- resolver.lookupSchema(schemaKey)
237241
_ <- StaticLookup.addTime(600.millis)
@@ -275,9 +279,6 @@ class ResolverSpec extends Specification with CatsEffect {
275279
RegistryError.RepoFailure("Should never be reached").asLeft
276280
)
277281

278-
val httpRep =
279-
Registry.Http(Registry.Config("Mock Repo", 1, List("com.snowplowanalytics.iglu-test")), null)
280-
281282
implicit val cache: CreateResolverCache[StaticLookup] = ResolverSpecHelpers.staticResolverCache
282283
implicit val clock: Clock[StaticLookup] = ResolverSpecHelpers.staticClock
283284
implicit val registryLookup: RegistryLookup[StaticLookup] =
@@ -286,7 +287,7 @@ class ResolverSpec extends Specification with CatsEffect {
286287
val result = for {
287288
resolver <-
288289
Resolver
289-
.init[StaticLookup](10, Some(1.second), httpRep)
290+
.init[StaticLookup](10, Some(1.second), Repos.httpRep)
290291
_ <- resolver.lookupSchema(schemaKey)
291292
_ <- StaticLookup.addTime(2.seconds)
292293
_ <- resolver.lookupSchema(schemaKey)
@@ -321,14 +322,8 @@ class ResolverSpec extends Specification with CatsEffect {
321322
val error4 = RegistryError.RepoFailure("Server segfault")
322323

323324
// Mocking repositories
324-
val httpRep1 = Registry.Http(
325-
Registry.Config("Mock Repo 1", 1, List("com.snowplowanalytics.iglu-test")),
326-
null
327-
)
328-
val httpRep2 = Registry.Http(
329-
Registry.Config("Mock Repo 2", 1, List("com.snowplowanalytics.iglu-test")),
330-
null
331-
)
325+
val httpRep1 = Repos.httpRep.copy(config = Repos.httpRep.config.copy(name = "Mock Repo 1"))
326+
val httpRep2 = Repos.httpRep.copy(config = Repos.httpRep.config.copy(name = "Mock Repo 2"))
332327

333328
implicit val cache: CreateResolverCache[StaticLookup] = ResolverSpecHelpers.staticResolverCache
334329
implicit val clock: Clock[StaticLookup] = ResolverSpecHelpers.staticClock
@@ -644,4 +639,28 @@ class ResolverSpec extends Specification with CatsEffect {
644639

645640
resolver.isNotFound(resolutionError) should beFalse
646641
}
642+
643+
def e21 = {
644+
val resolver: Resolver[Id] =
645+
Resolver
646+
.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror, Repos.custom)
647+
648+
val resolutionError = ResolutionError(
649+
SortedMap(
650+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
651+
Set(RegistryError.RepoFailure("Problem"), RegistryError.ClientFailure("Boom")),
652+
2,
653+
Instant.now()
654+
),
655+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
656+
Set(RegistryError.RepoFailure("Problem"), RegistryError.NotFound),
657+
2,
658+
Instant.now()
659+
),
660+
Repos.custom.config.name -> LookupHistory(Set(RegistryError.NotFound), 1, Instant.now())
661+
)
662+
)
663+
664+
resolver.isNotFound(resolutionError) should beTrue
665+
}
647666
}

0 commit comments

Comments
 (0)