subset2
subset2 copied to clipboard
Design change: throw exception on parsing when mandatory field is missing or its parsing has failed
Hi Alexander!
I wanted to hear your opinion on design change. As I'm more and more using your library in production (on two projects now) I noticed repetitive issue. Consider following example.
There case class A that has field of type Option[B] that has number of fields on its own. For example:
case class User(id: ObjectId, fb_info: Option[UserFbInfo])
object User {
private implicit object fromBson extends BsonFieldWithDoc[User] {
lazy val Doc: DocParser[User] = oid("_id") ~
get[UserFbInfo]("fb_info").opt map {
case id ~ fb_info => User(id, fb_info)
}
}
}
case class UserFbInfo(id: Long, token : FbToken)
object UserFbInfo {
implicit object fromBson extends BsonFieldWithDoc[UserFbInfo] {
lazy val Doc: DocParser[UserFbInfo] =
long("id") ~ get[FbToken]("token") map {
case id ~ token => UserFbInfo(id, token)
}
}
}
If somethign goes wrong in parsing of UserFbInfo field User.fb_info becomes None. And since MongoDB is schemaless database it happens quite often in rapidly changing project. Typically it's one of these two cases:
- Non-optional field of
UserFbInfodoesn't exist in Mongo document - Developer made mistake in parser of
FbTokenand forgot to specify correct type incasebranch of partial function ofField[FbToken].Unitwas returned instead ofFbToken.
It's very hard to catch bug because User.fb_info == None is perfectly legitimate use-case which indicates that user in my system hasn't attached his Facebook account to profile.
What if instead Subset would throw exception on parsing when mandatory field is missing ? Then at least I can catch it either in unit-test or production logs.
Thanks!
.opt is a combinator, it may be applied to quite complex parser and everything this combinator knows is whether the underlying parser succeeded or failed.
But the I understand your problem, it's related to #7 . It's actually about Field not reporting errors and hence a parser cannot distinguish field non-existance from wrong type.
I've prepared a temporary workaround for this issue, please see the commit. However, it should evidently be addressed in more consistent fashion via Field error reporting.