Introduce a descriptor sum type to better reflect the domain logic

[mdimjasevic]

This patch updates the descriptor definition type DescriptorDefinition by turning it from a record type with multiple Maybe values into a sum type such that these Maybe values are replaced with proper values in each data constructor. Until now a user was able to provide a configuration that made no sense in the domain.

This change reflects the domain logic that an inner descriptor cannot have a rate limit and that it must have sub-descriptors, and conversely, that a leaf descriptor must have a rate limit and it must have no sub-descriptors. Two new tests are added to the Fencer.Types.Test module to confirm the negative cases: test_parseJSONFaultyDescriptorDefinition1 and test_parseJSONFaultyDescriptorDefinition2.

A suggested order of reviewing:

1. lib/Fencer/Types.hs
2. lib/Fencer/Rules.hs
3. test/Fencer/Rules/Test/Helpers.hs
4. test/Fencer/Types/Test.hs
5. test/Fencer/Rules/Test/Examples.hs
6. test/Fencer/Logic/Test.hs

This closes issue #111.

[mdimjasevic]

@effectfully, would the following be a good solution for the partial fields problem you brought up?

Instead of using a sum type where data constructors are given as records, I'd define a GADT. To keep accessors for common fields to all data, I can simply write an accessor function for each such field; in the case of descriptor definitions, there would be one for the key and one for the optional value.

See commit https://github.com/juspay/fencer/pull/112/commits/c4c16a943dc1fb57b6a11dc4730f1bc1a7e6fc01 for the actual implementation of the proposal.

[effectfully]

Instead of using a sum type where data constructors are given as records, I'd define a GADT.

It's completely fine to use sum types, surely I'm not advocating against them. The code that I've written above has a sum type. A GADT is an unnecessary overcomplication and I believe you should revert that commit. I'm not proposing to get rid of sum types, I'm only proposing to get rid of partial fields:

The option -Wpartial-fields warns about record fields that could fail when accessed via a lacking constructor. The function f below will fail when applied to Bar, so the compiler will emit a warning at its definition when -Wpartial-fields is enabled.

data Foo = Foo { f :: Int } | Bar

I.e. in the example above I'd suggest to remove f, not to restructure Foo. And this is precisely what I propose with RuleBranch: just remove the partial accessors (ruleBranchNested and ruleBranchRateLimit) without touching anything else.

In DescriptionDefinition I suggest to restructure things a bit, but note that my version is isomorphic to yours, I just find my version more ergonomic.

[mdimjasevic]

Thank you both for all the clarifications!

At first I thought that partial fields are at odds with sum types, but in fact partial fields are at odds with sum types where data constructors are records.

@effectfully, I realized that GADTs are an overkill for this task and I replaced them with plain ADTs. In the current version I have no records, but if you think it'd be better to get them back, let me know and I'll do that; otherwise I'm merging with plain ADTs, like below:

-- | Config for a single rule tree.
data DescriptorDefinition
  =
    -- | An inner node with no rate limit
    DescriptorDefinitionInnerNode
      !RuleKey
      !(Maybe RuleValue)
      ![DescriptorDefinition]

    -- | A leaf node with a rate limit
  | DescriptorDefinitionLeafNode
      !RuleKey
      !(Maybe RuleValue)
      !RateLimit
  deriving stock (Eq, Show)

descriptorDefinitionKey
  :: DescriptorDefinition
  -> RuleKey
descriptorDefinitionKey (DescriptorDefinitionInnerNode k _ _) = k
descriptorDefinitionKey (DescriptorDefinitionLeafNode  k _ _) = k

descriptorDefinitionValue
  :: DescriptorDefinition
  -> Maybe RuleValue
descriptorDefinitionValue (DescriptorDefinitionInnerNode _ v _) = v
descriptorDefinitionValue (DescriptorDefinitionLeafNode  _ v _) = v

-- | A single branch in a rule tree, containing several (or perhaps zero)
-- nested rules.
data RuleBranch
  = RuleBranch !RuleTree
  | RuleLeaf !RateLimit