Skip to content

Commit ad85eb6

Browse files
committed
fix: silent failures on unparsable data (root) in some cases
1 parent 2262a25 commit ad85eb6

File tree

9 files changed

+83
-75
lines changed

9 files changed

+83
-75
lines changed

core/src/main/scala/scim/model/Group.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package scim.model
22

3+
import com.typesafe.scalalogging.LazyLogging
4+
import io.circe.Decoder.Result
35
import io.circe.Json
4-
import io.circe.syntax._
56
import io.circe.generic.auto._
7+
import io.circe.syntax._
68
import scim.model.Group.Root
9+
import Codecs._
710

8-
case class Group(json: Json) extends ExtensibleModel[Root] {
11+
case class Group(json: Json) extends ExtensibleModel[Root] with LazyLogging {
912
def schema: Schema = Schema.Group
10-
lazy val root: Root = json.as[Root].toOption.getOrElse(Root.fallback)
13+
lazy val root: Result[Root] = json.as[Root]
14+
def rootOrDefault: Root = root.toOption.getOrElse(Root.fallback)
1115

1216
def ++(other: Json): Group = Group(json.deepMerge(other))
1317
}
@@ -21,7 +25,7 @@ object Group {
2125
members: Option[Seq[Member]] = None,
2226
)
2327
object Root {
24-
def fallback: Root = Root(id=None, displayName = "")
28+
def fallback: Root = Root(id = None, displayName = "")
2529
}
2630
case class Member(
2731
value: String,

core/src/main/scala/scim/model/User.scala

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,18 @@ package scim.model
22

33
import java.net.URI
44
import java.util.Locale
5+
import com.typesafe.scalalogging.LazyLogging
6+
import io.circe.Decoder.Result
57
import io.circe.Json
68
import io.circe.generic.auto._
79
import io.circe.syntax._
810
import scim.model.User.Root
911
import Codecs._
1012

11-
case class User(json: Json) extends ExtensibleModel[Root] {
13+
case class User(json: Json) extends ExtensibleModel[Root] with LazyLogging {
1214
def schema: Schema = Schema.User
13-
lazy val root: Root = json.as[Root].toOption.getOrElse(Root.fallback)
15+
lazy val root: Result[Root] = json.as[Root]
16+
def rootOrDefault: Root = root.toOption.getOrElse(Root.fallback)
1417

1518
def ++(other: Json): User = User(json.deepMerge(other))
1619
}

core/src/main/scala/scim/model/model.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package scim.model
22

3-
import io.circe.Json
3+
import io.circe.{DecodingFailure, Json}
44

55
trait Model
66

@@ -16,7 +16,8 @@ trait ExtensibleModel[Root] extends Model {
1616
def schema: Schema
1717

1818
/** parsed representation */
19-
def root: Root
19+
def root: Either[DecodingFailure, Root]
20+
def rootOrDefault: Root
2021

2122
def asJson: Json = json
2223
}

core/src/test/scala/scim/model/GroupSpec.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ class GroupSpec extends AnyFunSpec with Matchers with OptionValues {
1616
}
1717

1818
it("should parse a the group json from the spec") {
19-
val root = Group(Jsons.group).root
19+
val root = Group(Jsons.group).rootOrDefault
2020
root.id.value should be("6c5bb468-14b2-4183-baf2-06d523e03bd3")
2121
root.displayName should be("Group B")
2222
root.members.value should have size(1)
2323
}
2424

2525
it("should parse the members of the group json from the spec") {
26-
val root = Group(Jsons.group).root
26+
val root = Group(Jsons.group).rootOrDefault
2727
root.members.value should have size(1)
2828
val m0 = root.members.value.head
2929
m0.value should be("c3a26dd3-27a0-4dec-a2ac-ce211e105f97")

core/src/test/scala/scim/model/PatchOpSpec.scala

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
247247
val before = Jsons.group
248248

249249
val r = patch.applyTo(Schema.Group)(before)
250-
val after = Group(r.getOrElse(fail(r.toString))).root
250+
val after = Group(r.getOrElse(fail(r.toString))).rootOrDefault
251251
after.id.value should be("6c5bb468-14b2-4183-baf2-06d523e03bd3")
252252
val ms = after.members.value
253253
ms should have size 2
@@ -285,7 +285,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
285285
val before = Jsons.userFull
286286

287287
val r = patch.applyTo(Schema.Group)(before)
288-
val after = User(r.getOrElse(fail(r.toString))).root
288+
val after = User(r.getOrElse(fail(r.toString))).rootOrDefault
289289
after.addresses.value should have size 2
290290
val home = after.addresses.value.find(_.`type`.value == "home").value
291291
home.country.value should be("USA")
@@ -315,7 +315,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
315315
val before = Jsons.userFull
316316

317317
val r = patch.applyTo(Schema.Group)(before)
318-
val after = User(r.getOrElse(fail(r.toString))).root
318+
val after = User(r.getOrElse(fail(r.toString))).rootOrDefault
319319
after.name.value.familyName.value should be("McDonald")
320320
after.name.value.givenName.value should be("Barbara")
321321
}
@@ -338,7 +338,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
338338
val before = Jsons.userFull
339339

340340
val r = patch.applyTo(Schema.Group)(before)
341-
val after = User(r.getOrElse(fail(r.toString))).root
341+
val after = User(r.getOrElse(fail(r.toString))).rootOrDefault
342342
after.addresses.value should have size 2
343343
val home = after.addresses.value.find(_.`type`.value == "home").value
344344
home.streetAddress.value should be("456 Hollywood Blvd")
@@ -363,7 +363,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
363363
val before = Jsons.group
364364

365365
val r = patch.applyTo(Schema.Group)(before)
366-
val after = Group(r.getOrElse(fail(r.toString))).root
366+
val after = Group(r.getOrElse(fail(r.toString))).rootOrDefault
367367
after.members should be(None)
368368
}
369369

@@ -384,7 +384,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
384384
val before = Jsons.userFull
385385

386386
val r = patch.applyTo(Schema.Group)(before)
387-
val after = User(r.getOrElse(fail(r.toString))).root
387+
val after = User(r.getOrElse(fail(r.toString))).rootOrDefault
388388
val as = after.addresses.value should have size 2
389389
val home = after.addresses.value.find(_.`type`.value == "home").value
390390
home.streetAddress.value should be("456 Hollywood Blvd")
@@ -411,7 +411,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
411411
val before = Jsons.group2
412412

413413
val r = patch.applyTo(Schema.Group)(before)
414-
val after = Group(r.getOrElse(fail(r.toString))).root
414+
val after = Group(r.getOrElse(fail(r.toString))).rootOrDefault
415415
after.members.value should have size 1
416416
after.members.value.head.value should be("c3a26dd3-27a0-4dec-a2ac-ce211e105f97")
417417
}
@@ -432,7 +432,7 @@ class PatchOpSpec extends AnyFunSpec with Checkers with Matchers with OptionValu
432432
val before = Jsons.userFull
433433

434434
val r = patch.applyTo(Schema.Group)(before)
435-
val after = User(r.getOrElse(fail(r.toString))).root
435+
val after = User(r.getOrElse(fail(r.toString))).rootOrDefault
436436
val as = after.addresses.value should have size 2
437437
val home = after.addresses.value.find(_.`type`.value == "home").value
438438
home.streetAddress.value should be("456 Hollywood Blvd")

core/src/test/scala/scim/model/UserSpec.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@ class UserSpec extends AnyFunSpec with Matchers with OptionValues {
1818
}
1919

2020
it("should parse 'userMinimal'") {
21-
val root = User(Jsons.userMinimal).root
21+
val root = User(Jsons.userMinimal).rootOrDefault
2222
root.userName should be("bjensen@example.com")
2323
root.id.value should be("2819c223-7f76-453a-919d-413861904646")
2424
root.name should be(None)
2525
}
2626
it("should parse 'userMinimalExternal'") {
27-
val root = User(Jsons.userMinimalExternal).root
27+
val root = User(Jsons.userMinimalExternal).rootOrDefault
2828
root.userName should be("bjensen@example.com")
2929
root.id should be(None)
3030
root.name should be(None)
3131
}
3232

3333
it("should parse 'userFull'") {
34-
val root = User(Jsons.userFull).root
34+
val root = User(Jsons.userFull).rootOrDefault
3535
root.id.value should be("2819c223-7f76-453a-919d-413861904646")
3636
root.userName should be("bjensen@example.com")
3737
root.name.get.formatted.value should be("Ms. Barbara J Jensen, III")
@@ -44,7 +44,7 @@ class UserSpec extends AnyFunSpec with Matchers with OptionValues {
4444
}
4545

4646
it("should parse enterprise attrs") {
47-
val root = User(Jsons.userFull).root
47+
val root = User(Jsons.userFull).rootOrDefault
4848
val enterprise = root.enterprise.value
4949
enterprise.employeeNumber.value should be("701984")
5050
enterprise.costCenter.value should be("4130")

core/src/test/scala/scim/rest/GroupResourceSpec.scala

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ class GroupResourceSpec extends AnyFunSpec with Matchers with OptionValues {
5050
if (r.status == 200) {
5151
r.status should be(200)
5252
Group(r.body.value).id.value should be("g1")
53-
Group(r.body.value).root should be(Group.Root(id = Some("g1"), displayName = "Group 1",
53+
Group(r.body.value).rootOrDefault should be(Group.Root(id = Some("g1"), displayName = "Group 1",
5454
members = Some(Seq(Member("u-1"), Member("u-2", display = Some("User 2"))))))
5555
} else {
5656
r.status should be(204)
5757
r.body should be(None)
5858
}
5959

6060
store.content should have size 2
61-
val members = store.content.find(_.id.contains("g1")).value.root.members.value
61+
val members = store.content.find(_.id.contains("g1")).value.rootOrDefault.members.value
6262
members should have size 2
6363
members.exists(_.value == "u-1") should be(true)
6464
members.exists(_.value == "u-2") should be(true)
@@ -86,15 +86,15 @@ class GroupResourceSpec extends AnyFunSpec with Matchers with OptionValues {
8686
if (r.status == 200) {
8787
r.status should be(200)
8888
Group(r.body.value).id.value should be("g1")
89-
Group(r.body.value).root should be(Group.Root(id = Some("g1"), displayName = "Group 1", members = Some(Seq(Member("u-1"),
89+
Group(r.body.value).rootOrDefault should be(Group.Root(id = Some("g1"), displayName = "Group 1", members = Some(Seq(Member("u-1"),
9090
Member("u-2", display = Some("User 2")), Member("u-3", display = Some("User 3")), Member("u-4", display = Some("User 4"))))))
9191
} else {
9292
r.status should be(204)
9393
r.body should be(None)
9494
}
9595

9696
store.content should have size 2
97-
val members = store.content.find(_.id.contains("g1")).value.root.members.value
97+
val members = store.content.find(_.id.contains("g1")).value.rootOrDefault.members.value
9898
members should have size 4
9999
members.exists(_.value == "u-1") should be(true)
100100
members.exists(_.value == "u-2") should be(true)
@@ -119,15 +119,15 @@ class GroupResourceSpec extends AnyFunSpec with Matchers with OptionValues {
119119
if (r.status == 200) {
120120
r.status should be(200)
121121
Group(r.body.value).id.value should be("g1")
122-
Group(r.body.value).root should be(Group.Root(id = Some("g1"), displayName = "Group 1",
122+
Group(r.body.value).rootOrDefault should be(Group.Root(id = Some("g1"), displayName = "Group 1",
123123
members = Some(Seq(Member("u-2"), Member("u-3")))))
124124
} else {
125125
r.status should be(204)
126126
r.body should be(None)
127127
}
128128

129129
store.content should have size 2
130-
val members = store.content.find(_.id.contains("g1")).value.root.members.value
130+
val members = store.content.find(_.id.contains("g1")).value.rootOrDefault.members.value
131131
members should have size 2
132132
members.exists(_.value == "u-2") should be(true)
133133
members.exists(_.value == "u-3") should be(true)
@@ -151,15 +151,15 @@ class GroupResourceSpec extends AnyFunSpec with Matchers with OptionValues {
151151
if (r.status == 200) {
152152
r.status should be(200)
153153
Group(r.body.value).id.value should be("g1")
154-
Group(r.body.value).root should be(Group.Root(id = Some("g1"), displayName = "Group 1",
154+
Group(r.body.value).rootOrDefault should be(Group.Root(id = Some("g1"), displayName = "Group 1",
155155
members = Some(Seq(Member("u-2"), Member("u-3")))))
156156
} else {
157157
r.status should be(204)
158158
r.body should be(None)
159159
}
160160

161161
store.content should have size 2
162-
val members = store.content.find(_.id.contains("g1")).value.root.members.value
162+
val members = store.content.find(_.id.contains("g1")).value.rootOrDefault.members.value
163163
members should have size 2
164164
members.exists(_.value == "u-2") should be(true)
165165
members.exists(_.value == "u-3") should be(true)
@@ -182,15 +182,15 @@ class GroupResourceSpec extends AnyFunSpec with Matchers with OptionValues {
182182
if (r.status == 200) {
183183
r.status should be(200)
184184
Group(r.body.value).id.value should be("g1")
185-
Group(r.body.value).root should be(Group.Root(id = Some("g1"), displayName = "Group 1",
185+
Group(r.body.value).rootOrDefault should be(Group.Root(id = Some("g1"), displayName = "Group 1",
186186
members = Some(Seq(Member("u-1")))))
187187
} else {
188188
r.status should be(204)
189189
r.body should be(None)
190190
}
191191

192192
store.content should have size 2
193-
val members = store.content.find(_.id.contains("g1")).value.root.members.value
193+
val members = store.content.find(_.id.contains("g1")).value.rootOrDefault.members.value
194194
members should have size 1
195195
members.head.value should be("u-1")
196196
})

core/src/test/scala/scim/rest/MockStore.scala

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,26 @@ trait MockStore[A <: ExtensibleModel[_]] {
5656
object MockStore {
5757
trait MockUserStore extends UserStore[Id] with MockStore[User] {
5858
override protected def schema = Schema.User
59-
override protected def duplicate(a: User, b: User) = a.root.userName == b.root.userName
59+
override protected def duplicate(a: User, b: User) = a.rootOrDefault.userName == b.rootOrDefault.userName
6060
override protected implicit def decoder: Decoder[User] = Codecs.userDecoder
6161
}
6262
trait MockGroupStore extends GroupStore[Id] with MockStore[Group] {
6363
override protected def schema = Schema.Group
64-
override protected def duplicate(a: Group, b: Group) = a.root.displayName == b.root.displayName
64+
override protected def duplicate(a: Group, b: Group) = a.rootOrDefault.displayName == b.rootOrDefault.displayName
6565
override protected implicit def decoder: Decoder[Group] = Codecs.groupDecoder
6666
}
6767
trait MockOptimizedGroupStore extends GroupStore[Id] with MockStore[Group] {
6868
var wasOptimized = false
6969
override protected def schema = Schema.Group
70-
override protected def duplicate(a: Group, b: Group) = a.root.displayName == b.root.displayName
70+
override protected def duplicate(a: Group, b: Group) = a.rootOrDefault.displayName == b.rootOrDefault.displayName
7171
override protected implicit def decoder: Decoder[Group] = Codecs.groupDecoder
7272

7373
override def addToGroup(groupId: String, members: Set[Group.Member]) = Some {
7474
wasOptimized = true
7575
content.find(_.id.contains(groupId))
7676
.map { group =>
77-
val ms = members ++ group.root.members.getOrElse(Seq.empty)
78-
val g = Group(group.root.copy(members = Some(ms.toSeq)))
77+
val ms = members ++ group.rootOrDefault.members.getOrElse(Seq.empty)
78+
val g = Group(group.rootOrDefault.copy(members = Some(ms.toSeq)))
7979
content = content.filterNot(_.id.contains(groupId)) :+ g
8080
Right(())
8181
}.getOrElse(Left(DoesNotExist(groupId)))
@@ -86,8 +86,8 @@ object MockStore {
8686
wasOptimized = true
8787
Some(content.find(_.id.contains(groupId))
8888
.map { group =>
89-
val ms = group.root.members.getOrElse(Seq.empty).filterNot(_.value == value)
90-
val g = Group(group.root.copy(members = Some(ms.toSeq)))
89+
val ms = group.rootOrDefault.members.getOrElse(Seq.empty).filterNot(_.value == value)
90+
val g = Group(group.rootOrDefault.copy(members = Some(ms.toSeq)))
9191
content = content.filterNot(_.id.contains(groupId)) :+ g
9292
Right(())
9393
}.getOrElse(Left(DoesNotExist(groupId))))

0 commit comments

Comments
 (0)