Haskell-OpenAPI-Client-Code-Generator icon indicating copy to clipboard operation
Haskell-OpenAPI-Client-Code-Generator copied to clipboard

Separate required and optional parameters types

Open chris-martin opened this issue 1 year ago • 3 comments

For each record type e.g. Get_foo that represents a set of query parameters, the generator also produces a function mkGet_foo whose parameters are only the values of Get_foo that are required. This can be nice compared to using the Get_foo constructor directly because when an API adds new optional query parameters to a resource, that doesn't have to be a breaking change to users of the client. However, we've found some drawbacks:

  • Since mkGet_foo uses positional arguments rather than named record fields, it's easy to accidentally transpose the arguments if they are of the same type e.g. Text.
  • We've had one bug where we constructed a value using mkGet_foo and immediately followed by a Get_foo record update that overwrote one of the required fields that mkGet_foo had set. There's no indication in the types and no easy way from looking at the user code to see which mkGet_foo parameters correspond to which Get_foo record fields.

I'd like to throw out an idea:

  • For each Get_foo type, generate two corresponding record types

    • Required_Get_foo
    • Optional_Get_foo

    which divide up the required and optional parameters respectively

  • Change mkGet_foo (or add a different function) to build a Get_foo from a Required_Get_foo and an Optional_Get_foo.

  • Define a constant Optional_Get_foo that has Nothing for all the fields, any of the following would work:

    • a separate named constant for each parameter set;
    • mempty (if a semigroup operation is also wanted)
    • def

    I think both Monoid and Default have the advantage that they can be derived with generics rather than writing yet more template haskell.

So parameter construction then could go from

parameters :: Get_foo
parameters = (mkGet_foo a b){ someOptionalParameter = c }

to:

parameters :: Get_foo
parameters =
 mkGet_foo'
   Required_Get_foo
     { requiredParameter = a
     , anotherRequiredParameter = b
     }
   $ def
     { someOptionalParameter = c
     }

That way we have the advantages of both the mk function and of record construction:

  • All the parameters are given explicitly by name.
  • Adding optional parameters doesn't cause a breaking change at the use site.

One additional thought: To limit the explosion in the number of new generated identifiers introduced, a type class with an associated data family might be a nice way to go about it.

class Parameters a where
  data RequiredParameters a :: Type
  data OptionalParameters a :: Type
  defaultOptionalParameters :: OptionalParameters a
  mkParameters :: RequiredParameters a -> OptionalParameters a -> a

Then adding support for this feature is mostly just a matter of generating Parameters instances.

chris-martin avatar Oct 22 '24 17:10 chris-martin

Thank you for your input! I agree that the current design can lead to problem when the underlying specificaiton changes as it relies on the positions and would prefer a solution which does not do that.

I will most likely not have time to implement this myself but I'll try to make time to review potential PRs. Regarding API design I'd be interested in hearing @NorfairKing's thoughts.

joel-bach avatar Nov 09 '24 07:11 joel-bach

@chris-martin I'm hesitant to make this very complex codegen task more complex, but it does look like a safety improvement. I have no time to donate either so if you @chris-martin are up for making a draft I'd be happy to review it!

NorfairKing avatar Nov 09 '24 10:11 NorfairKing

Thanks for input. I figured it'd be a nontrivial amount of work. I doubt I'll get to it anytime soon either; just wanted to verify the idea seemed sound before I try to find the time.

chris-martin avatar Nov 11 '24 17:11 chris-martin