How to properly constrain `arbitrary` UUID-Generation?

I'm trying to create Arbitrary instances for some of my types to be used in QuickCheck property testing. I need randomly generated UUIDs, with the constraint that all-zero (nil) UUIDs are disallowed - that is, 00000000-0000-0000-0000-000000000000. Therefore, I set up the following generator:

nonzeroIdGen :: Gen UUID.UUID
nonzeroIdGen = arbitrary `suchThat` (not . UUID.null)

Which I use in an Arbitrary instance as follows:

instance Arbitrary E.EventId where
    arbitrary = do
        maybeEid <- E.mkEventId <$> nonzeroIdGen
        return $ fromJust maybeEid

In general, this is unsafe code; but for testing, with supposedly guaranteed nonzero UUIDs, I thought the fromJust to be ok.

mkEventId is defined as

mkEventId :: UUID.UUID -> Maybe EventId
mkEventId uid = EventId <$> validateId uid

with EventId a new type-wrapper around UUID.UUID, and

validateId :: UUID.UUID -> Maybe UUID.UUID
validateId uuid = if UUID.null uuid then Nothing else Just uuid

To my surprise, I get failing tests because of all-zero UUIDs generated by the above code. A trace in mkEventId shows the following:

00000001-0000-0001-0000-000000000001
Just (EventId {getEventId = 00000001-0000-0001-0000-000000000001})

00000000-0000-0000-0000-000000000000
Nothing
    Create valid Events. FAILED [1]

The first generated ID is fine, the second one is all-zero, despite my nonzeroIdGen generator from above. What am I missing?

Solution

I generally find that in cases like this, using newtypes to define instances of Arbitrary composes better. Here's one I made for valid UUID values:

newtype NonNilUUID = NonNilUUID { getNonNilUUID :: UUID } deriving (Eq, Show)

instance Arbitrary NonNilUUID where
  arbitrary = NonNilUUID <$> arbitrary `suchThat` (/= nil)

You can then compose other Arbitrary instances from this one, like I do here with a Reservation data type:

newtype ValidReservation =
  ValidReservation { getValidReservation :: Reservation } deriving (Eq, Show)

instance Arbitrary ValidReservation where
  arbitrary = do
    (NonNilUUID rid) <- arbitrary
    (FutureTime d) <- arbitrary
    n <- arbitrary
    e <- arbitrary
    (QuantityWithinCapacity q) <- arbitrary
    return $ ValidReservation $ Reservation rid d n e q

Notice the pattern match (NonNilUUID rid) <- arbitrary to deconstruct rid as a UUID value.

You may notice that I've also created a ValidReservation newtype for my Reservation data type. I consistently do this to avoid orphan instances, and to avoid polluting my domain model with a QuickCheck dependency. (I have nothing against QuickCheck, but test-specific capabilities don't belong in the 'production' code.)

All the code shown here is available in context on GitHub.