Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design change: throw exception on parsing when mandatory field is missing or its parsing has failed #10

Open
unoexperto opened this issue Aug 10, 2013 · 1 comment

Comments

@unoexperto
Copy link

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:

  1. Non-optional field of UserFbInfo doesn't exist in Mongo document
  2. Developer made mistake in parser of FbTokenand forgot to specify correct type in case branch of partial function of Field[FbToken]. Unit was returned instead of FbToken.

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!

alaz added a commit that referenced this issue Aug 10, 2013
The test case demonstrate how it is possible to make sure the field either exists or has correct type.
@alaz
Copy link
Member

alaz commented Aug 10, 2013

.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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants