Skip to content

Commit a887bf0

Browse files
benjbenbenjben
committed
Address review feedback
1 parent 9774b66 commit a887bf0

File tree

3 files changed

+183
-12
lines changed

3 files changed

+183
-12
lines changed

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,19 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
187187
): F[Either[ResolutionError, Json]] =
188188
lookupSchemaResult(schemaKey).map(_.map(_.value.schema))
189189

190+
/**
191+
* If Iglu Central or any of its mirrors doesn't have a schema,
192+
* it should be considered NotFound, even if one of them returned an error.
193+
*/
194+
private[resolver] def isNotFound(error: ResolutionError): Boolean = {
195+
val (igluCentral, custom) = error.value.partition { case (repo, _) =>
196+
allIgluCentral.contains(repo)
197+
}
198+
(igluCentral.isEmpty || igluCentral.values.exists(
199+
_.errors.forall(_ == RegistryError.NotFound)
200+
)) && custom.values.flatMap(_.errors).forall(_ == RegistryError.NotFound)
201+
}
202+
190203
/**
191204
* Looks up all the schemas with the same model until `maxSchemaKey`.
192205
* For the schemas of previous revisions, it starts with addition = 0
@@ -203,18 +216,6 @@ final case class Resolver[F[_]](repos: List[Registry], cache: Option[ResolverCac
203216
L: RegistryLookup[F],
204217
C: Clock[F]
205218
): F[Either[SchemaResolutionError, List[SelfDescribingSchema[Json]]]] = {
206-
207-
// If Iglu Central or any of its mirrors doesn't have a schema,
208-
// it should be considered NotFound, even if one of them returned an error
209-
def isNotFound(error: ResolutionError): Boolean = {
210-
val (igluCentral, custom) = error.value.partition { case (repo, _) =>
211-
allIgluCentral.contains(repo)
212-
}
213-
(igluCentral.isEmpty || igluCentral.values.exists(
214-
_.errors.forall(_ == RegistryError.NotFound)
215-
)) && custom.values.flatMap(_.errors).forall(_ == RegistryError.NotFound)
216-
}
217-
218219
def go(
219220
current: SchemaVer.Full,
220221
acc: List[SelfDescribingSchema[Json]]

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ object SpecHelpers {
5252
Registry.HttpConnection(URI.create("http://iglucentral.com"), None)
5353
)
5454

55+
val IgluCentralMirror: Registry =
56+
Registry.Http(
57+
Registry.Config("Iglu Central - GCP Mirror", 10, List("com.snowplowanalytics")),
58+
Registry.HttpConnection(URI.create("http://mirror01.iglucentral.com"), None)
59+
)
60+
5561
case class TrackingRegistry(
5662
lookupState: AtomicReference[List[String]],
5763
listState: AtomicReference[List[String]]

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ object ResolverSpec {
5656
val one: Registry.Embedded = embedRef("com.acme", 0)
5757
val two: Registry.Embedded = embedRef("de.acompany.snowplow", 40)
5858
val three: Registry.Embedded = embedRef("de.acompany.snowplow", 100)
59+
60+
val custom: Registry =
61+
Registry.Http(
62+
Registry.Config("Iglu Custom Repo", 10, List("com.acme")),
63+
Registry.HttpConnection(URI.create("http://iglu.acme.com"), None)
64+
)
5965
}
6066
}
6167

@@ -76,6 +82,14 @@ class ResolverSpec extends Specification with CatsEffect {
7682
a Resolver should cache SchemaLists with different models separately $e11
7783
a Resolver should use schemaKey provided in SchemaListLike for result validation $e12
7884
result from SchemaListLike should contain the exact schemaKey provided $e13
85+
isNotFound should
86+
return true if custom repo and Iglu Central repos don't have a schema $e14
87+
return true if one Iglu Central repo returns an error and the other one NotFound $e15
88+
return false if custom repo returns an error $e16
89+
return true if there is no custom repo, one Iglu Central repo returns an error and the other one NotFound $e17
90+
return false if there is no custom repo and Iglu Central ones return an error $e18
91+
return true if there is just one custom repo that returns NotFound $e19
92+
return false if there is just one custom repo that returns an error $e20
7993
"""
8094

8195
import ResolverSpec._
@@ -480,4 +494,154 @@ class ResolverSpec extends Specification with CatsEffect {
480494

481495
result must beRight(SchemaList.parseUnsafe(List(schema100, schema101, schema102)))
482496
}
497+
498+
def e14 = {
499+
val resolver: Resolver[Id] =
500+
Resolver
501+
.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror, Repos.custom)
502+
503+
val resolutionError = ResolutionError(
504+
SortedMap(
505+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
506+
Set(RegistryError.NotFound),
507+
1,
508+
Instant.now()
509+
),
510+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
511+
Set(RegistryError.NotFound),
512+
1,
513+
Instant.now()
514+
),
515+
Repos.custom.config.name -> LookupHistory(Set(RegistryError.NotFound), 1, Instant.now())
516+
)
517+
)
518+
519+
resolver.isNotFound(resolutionError) should beTrue
520+
}
521+
522+
def e15 = {
523+
val resolver: Resolver[Id] =
524+
Resolver
525+
.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror, Repos.custom)
526+
527+
val resolutionError = ResolutionError(
528+
SortedMap(
529+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
530+
Set(RegistryError.RepoFailure("Problem")),
531+
1,
532+
Instant.now()
533+
),
534+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
535+
Set(RegistryError.NotFound),
536+
1,
537+
Instant.now()
538+
),
539+
Repos.custom.config.name -> LookupHistory(Set(RegistryError.NotFound), 1, Instant.now())
540+
)
541+
)
542+
543+
resolver.isNotFound(resolutionError) should beTrue
544+
}
545+
546+
def e16 = {
547+
val resolver: Resolver[Id] =
548+
Resolver
549+
.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror, Repos.custom)
550+
551+
val resolutionError = ResolutionError(
552+
SortedMap(
553+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
554+
Set(RegistryError.NotFound),
555+
1,
556+
Instant.now()
557+
),
558+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
559+
Set(RegistryError.NotFound),
560+
1,
561+
Instant.now()
562+
),
563+
Repos.custom.config.name -> LookupHistory(
564+
Set(RegistryError.ClientFailure("Something went wrong")),
565+
1,
566+
Instant.now()
567+
)
568+
)
569+
)
570+
571+
resolver.isNotFound(resolutionError) should beFalse
572+
}
573+
574+
def e17 = {
575+
val resolver: Resolver[Id] =
576+
Resolver.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror)
577+
578+
val resolutionError = ResolutionError(
579+
SortedMap(
580+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
581+
Set(RegistryError.RepoFailure("Problem")),
582+
1,
583+
Instant.now()
584+
),
585+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
586+
Set(RegistryError.NotFound),
587+
1,
588+
Instant.now()
589+
)
590+
)
591+
)
592+
593+
resolver.isNotFound(resolutionError) should beTrue
594+
}
595+
596+
def e18 = {
597+
val resolver: Resolver[Id] =
598+
Resolver.init[Id](0, None, SpecHelpers.IgluCentral, SpecHelpers.IgluCentralMirror)
599+
600+
val resolutionError = ResolutionError(
601+
SortedMap(
602+
SpecHelpers.IgluCentral.config.name -> LookupHistory(
603+
Set(RegistryError.RepoFailure("Problem")),
604+
1,
605+
Instant.now()
606+
),
607+
SpecHelpers.IgluCentralMirror.config.name -> LookupHistory(
608+
Set(RegistryError.ClientFailure("Network issue")),
609+
1,
610+
Instant.now()
611+
)
612+
)
613+
)
614+
615+
resolver.isNotFound(resolutionError) should beFalse
616+
}
617+
618+
def e19 = {
619+
val resolver: Resolver[Id] =
620+
Resolver.init[Id](0, None, Repos.custom)
621+
622+
val resolutionError = ResolutionError(
623+
SortedMap(
624+
Repos.custom.config.name -> LookupHistory(Set(RegistryError.NotFound), 1, Instant.now())
625+
)
626+
)
627+
628+
resolver.isNotFound(resolutionError) should beTrue
629+
}
630+
631+
def e20 = {
632+
val resolver: Resolver[Id] =
633+
Resolver.init[Id](0, None, Repos.custom)
634+
635+
val resolutionError = ResolutionError(
636+
SortedMap(
637+
Repos.custom.config.name -> LookupHistory(
638+
Set(RegistryError.ClientFailure("Boom")),
639+
1,
640+
Instant.now()
641+
)
642+
)
643+
)
644+
645+
resolver.isNotFound(resolutionError) should beFalse
646+
}
483647
}

0 commit comments

Comments
 (0)