higherkindness / mu-haskell

Mu (μ) is a purely functional framework for building micro services.
http://higherkindness.io/mu-haskell/
Apache License 2.0
333 stars 19 forks source link

oneof - more than 2 options #296

Closed rwakulszowa closed 3 years ago

rwakulszowa commented 3 years ago

Sorry if this has been asked before, but I'm having a hard time trying to map oneof fields with more than 2 fields to Hs.

From what I understand, two field oneofs, such as

message OneOfTwo {
  oneof value {
    string a = 1;
    bool b = 2;
  }
}

can be mapped to

data OneOfTwo 
  = OneOfTwo { value :: Either T.Text Bool }
   deriving (Eq, Show, Generic
             , ToSchema   TheSchema "OneOfTwo"
             , FromSchema TheSchema "OneOfTwo")

This compiles just fine.

How do I represent messages with 3 variants?

message OneOfThree {
  oneof value {
    string a = 1;
    bool b = 2;
    int32 c = 3;
  }
}   

I've tried nesting Eithers, but with no success:

data OneOfThree
    = OneOfThree { value :: Either T.Text (Either Bool Int32) }
    deriving (Eq, Show, Generic
                  , ToSchema   TheSchema "OneOfThree",
                  , FromSchema TheSchema "OneOfThree")    

gives the following error:

    • No instance for (Mu.Schema.Class.GFromSchemaFieldTypeUnion
                         '[ 'DRecord
                              "OneOfTwo"
                              '[ 'FieldDef
                                   "value" ('TUnion '[ 'TPrimitive T.Text, 'TPrimitive Bool])],
                            'DRecord
                              "OneOfThree"
                              '[ 'FieldDef
                                   "value"
                                   ('TUnion
                                      '[ 'TPrimitive T.Text, 'TPrimitive Bool, 'TPrimitive Int32])]]
                         '[ 'TPrimitive Bool, 'TPrimitive Int32]
                         (K1 R (Either Bool Int32)))
        arising from the 'deriving' clause of a data type declaration
      Possible fix:
        use a standalone 'deriving instance' declaration,
          so you can specify the instance context yourself
    • When deriving the instance for (FromSchema
                                        TheSchema "OneOfThree" OneOfThree)
   |
33 |            , FromSchema TheSchema "OneOfThree")
serras commented 3 years ago

The easiest way is to create your own data with three options:

data OneOfThreeValue = A T.Text | B Bool | C Int32
data OneOfThree = OneOfThree { value :: OneOfThreeValue }
rwakulszowa commented 3 years ago

Oh, it actually does seem to work. I was sure I've tried something along these lines, but I guess I was wrong.

Thanks!

Btw. , if I may make a suggestion - it would be great to see this in the documentation. I can send a PR if you point me to a place where you store the docs.

kutyel commented 3 years ago

Btw. , if I may make a suggestion - it would be great to see this in the documentation. I can send a PR if you point me to a place where you store the docs.

Sure, the docs are here: https://github.com/higherkindness/mu-haskell/tree/master/docs/docs, feel free to contribute a PR! 🙌🏻

rwakulszowa commented 3 years ago

Hm, I have another question - does it actually work for more than 3 fields?

data OneOfFourValue = A Int32  | B Int32 | C Int32 | D Int32 deriving (Eq, Show, Generic)

data OneOfFour
    = OneOfFour { value :: OneOfFourValue }
     deriving (Eq, Show, Generic, ToSchema TheSchema "OneOfFour", FromSchema TheSchema "OneOfFour")
message OneOfFour {
  oneof value {
    int32 A = 1;
    int32 B = 2;
    int32 C = 3;
    int32 D = 4;
  }
}

Gives a compiler error of

Overlapping instances for Mu.Schema.Class.GFromSchemaFieldTypeUnion ... • When deriving the instance for (FromSchema TheSchema "OneOfFour" OneOfFour)

Is this expected?

serras commented 3 years ago

Let's reopen this to keep track of this problem.

llattila commented 3 years ago

I had something that worked with 4 fields but all of the fields were of different type. Might this have an impact on the issue?

Also, is it possible to have only one of the fields with a oneof option while the rest are values that are always required?

message OneOfThree {
  int64 unixTime = 1;
  oneof value {
    string a = 2;
    bool b = 3;
    int32 c = 4;
  }
}   
serras commented 3 years ago

I realized that this is not clear in the docs (we need to fix that, @kutyel), but the idea is that from the point of view of Mu the inner value is another type altogether. For that reason you need to define two types. The first one is for the oneof value:

data OneOfThreeValue = A String | B Bool | C Int32

and then you use that type in the definition of the message itself:

data OneOfThree = OneOfThree { unixTime :: Int64, value :: OneOfThreeValue }

Does this make sense? I'm happy to figuring out simpler ways to interact with protobuf types, since I don't do that very often myself.

llattila commented 3 years ago

Yeah, I simply had an issue by using String, not Text, so that caused my issues. The error messages aren't the easiest ones, especially if one has no experience with Template Haskell stuff :)

llattila commented 3 years ago

Not sure whether this should be closed as the OneOfFour doesn't seem to work. I tried many different things there locally as well, but was getting those overlapping instance mistakes.

serras commented 3 years ago

Sorry, I misunderstood your last comment, @llattila. Do you still get the same problem when using the version in master?

serras commented 3 years ago

@llattila you can check it by using the following in your stack.yaml file:

extra-deps:
  - (other-dependencies go here)
  - github: higherkindness/mu-haskell
    commit: 6a5ae74d07d2b2183ccbacf2846983928a5547f6
    subdirs:
      - core/schema
llattila commented 3 years ago

@serras Now it seems to work with more than 3 options there, at least with 4 it worked :)

llattila commented 3 years ago

I was running some round-trip tests and it seems the oneof always parses only the first type. The encoding created different bytestrings but the decoding only wanted to parse with the first type. Will try to create a simple test-case as my own code-base is already a tad larger as trying to implement it on my previous code base.

llattila commented 3 years ago

That location contains the simplest thing I can make fail. Making round trips between OneOfFour and Bytestring and they are clearly not identical. Running the app clearly shows that the proto-buf parsings are different so presume it to fail in the decode only.

https://github.com/llattila/mu-haskell-test-case

serras commented 3 years ago

@llattila thanks very much for such a thorough example! 🙇‍♂️

I'll have a look at it during next week, I think I have a feeling of where things are going wrong, but I need a bit more digging beforehand.

llattila commented 3 years ago

@serras added a validity based test-case as well, should be able to easily identify when things are not working correctly.

llattila commented 3 years ago

@serras have you been able to investigate the issue? I tried following up a little bit what happens but can very clearly recognize that I have no clue what is going on with all of this Template Haskell and Type Families :D

llattila commented 3 years ago

@serras it seems I might have been only bad at making correct stack files in the test case. Revisited this issue, and noticed my simple-test-case had still extra-dep for the 4.2.0 version in addition to this core/schema version. I Update the project so per the test-case there are no issues in parsing one ofs. I think this ticket should be closed.