Zaid-Ajaj / LiteDB.FSharp

Advanced F# Support for LiteDB, an embedded NoSql database for .NET with type-safe query expression through F# quotations
MIT License
180 stars 22 forks source link

Unhandled Exception: ... invalid name ... at LiteDB.BsonDocument.set_Item in 'mutable' field #27

Closed srlopez closed 5 years ago

srlopez commented 5 years ago

The mutable attribute avoids insert in LiteDB, or passing through Json's encoder, something happens that the name of the field is modified.

open System
open LiteDB
open LiteDB.FSharp

[<StructuredFormatDisplay("a{SizeGb}GB")>]
[<CLIMutable>]
type Disk = 
    { SizeGb : int }

[<StructuredFormatDisplay("Computer #{Id}: {Manufacturer}/{DiskCount}/{Disks}")>]
[<CLIMutable>]
type Computer =
    { Id: int
      mutable Manufacturer: string
      mutable Disks: Disk list }
    [<BsonIgnore>] 
    member __.DiskCount = __.Disks.Length  // property calculated

[<EntryPoint>]
let main argv =

    let myPc =
        { Id = 0
          Manufacturer = "Computers Inc."
          Disks =
            [ { SizeGb = 100 }
              { SizeGb = 250 }
              { SizeGb = 500 } ] }
    printfn "%A" myPc

    let mapper = FSharpBsonMapper()
    use db = new LiteDatabase("data.db", mapper)
    let computers = db.GetCollection<Computer>("computers")

    // All right , but if I remove the comment on next line I get the error
    // computers.Insert(myPc) |> ignore
    myPc.Manufacturer <- "Dell"
    printfn "%A" myPc
    ...

The field you are trying to modify Manufacturer seems to change its name Manufacturer@' The full error is:

Unhandled Exception: System.ArgumentException: Field 'Manufacturer@' has an invalid name.
   at LiteDB.BsonDocument.set_Item(String name, BsonValue value)
   at LiteDB.JsonReader.ReadObject()
   at LiteDB.JsonReader.ReadValue(JsonToken token)
   at LiteDB.JsonSerializer.Deserialize(String json)
   at LiteDB.FSharp.Bson.serialize[t](t entity)
   at LiteDB.FSharp.FSharpBsonMapper.ToDocument[t](t entity)
   at LiteDB.LiteCollection`1.Insert(T document)
   at Program.main(String[] argv) in /app/Program.fs:line ..

Thanks

humhei commented 5 years ago

mutable Manufacturer dnspy: Decompiled c# codes

public string Manufacturer@;

Converter will try to serialize both property Manufacturer and field Manufacturer@

immutable Manufacturer dnspy: Decompiled c# codes

internal string Manufacturer@;

Converter will only try to serialize property Manufacturer

srlopez commented 5 years ago

@humhei How can I fix it? It's a problem of my code or it's an authentic issue?

humhei commented 5 years ago

In fsharp world mutable fields in record type is not recommanded(Not thread safe; I didn't see others to use it) What's your use case of it?

workaround

Create a new record instead of setting property mutable

open System
open LiteDB
open LiteDB.FSharp

[<StructuredFormatDisplay("a{SizeGb}GB")>]
[<CLIMutable>]
type Disk = 
    { SizeGb : int }

[<StructuredFormatDisplay("Computer #{Id}: {Manufacturer}/{DiskCount}/{Disks}")>]
[<CLIMutable>]
type Computer =
    { Id: int
      Manufacturer: string
      Disks: Disk list }
    [<BsonIgnore>] 
    member __.DiskCount = __.Disks.Length  // property calculated

[<EntryPoint>]
let main argv =

    let myPc =
        { Id = 0
          Manufacturer = "Computers Inc."
          Disks =
            [ { SizeGb = 100 }
              { SizeGb = 250 }
              { SizeGb = 500 } ] }
    printfn "%A" myPc

    let mapper = FSharpBsonMapper()
    use db = new LiteDatabase("data.db", mapper)
    let computers = db.GetCollection<Computer>("computers")

    let myDellPc = 
            {myPc with Manufacturer = "Dell" }
    computers.Insert(myDellPc) |> ignore

...
Zaid-Ajaj commented 5 years ago

Hmm even though it is not recommended, I don't see an issue in supporting mutable constructs in the library, I will work on it...

srlopez commented 5 years ago

Hmm even though it is not recommended, I don't see an issue in supporting mutable constructs in the library, I will work on it...

And [<BsonIgnore>] and [<BsonField>] too, please

Zaid-Ajaj commented 5 years ago

I am afraid that [<BsonIgnore>] doesn't really make sense within F# records. Using a member within the record is the way to go as it will be used to compute properties derived from the record fields and doesn't get persisted (= ignored) as follows:

[<CLIMutable>]
type Computer =
    { Id: int
      Manufacturer: string
      Disks: Disk list }

    // this is not persisted
    with member this.Ignored = sprintf "%d => %s" this.Id this.Manufacturer

Aren't these members good enough for your use-case?

we can continue discussion at #26

As for [<BsonField>] I think it is possible but will require more work, I will open another issue for it.

Zaid-Ajaj commented 5 years ago

Fixed and released in version 2.8.0 to nuget, should be available really soon when nuget does it's magic. Now I decided to take control of how the fields are marshalled, so now these mutable records will work just fine :smile: