Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added typeshape mapper #38

Merged
merged 6 commits into from Sep 13, 2019

Conversation

@vshapenko
Copy link

commented Sep 13, 2019

This PR adds typeshape mapper support for LiteDb.Fsharp. Currently it passes 76 of 78 tests

Vasiliy Shapenko added 3 commits Sep 13, 2019
@Zaid-Ajaj

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

Awesome! can you add your mapper to the tests to see which ones are failing? There is this function:

let useDatabase (f: LiteDatabase -> unit) = 
    let mapper = FSharpBsonMapper()
    use memoryStream = new MemoryStream()
    use db = new LiteDatabase(memoryStream, mapper)
    f db

create another one useTypeShapeDb:

let useTypeShapeDb(f: LiteDatabase -> unit) = 
    let mapper = Experiemental.TypeShapeMapper()
    use memoryStream = new MemoryStream()
    use db = new LiteDatabase(memoryStream, mapper)
    f db

Then parameterize the tests such that they take (useDatabase : LiteDatabase -> unit) parameter, what do you think?

Vasiliy Shapenko added 2 commits Sep 13, 2019
Vasiliy Shapenko
LiteDB.FSharp.Tests/Tests.InheritedType.fs Outdated Show resolved Hide resolved
Vasiliy Shapenko
@vshapenko

This comment has been minimized.

Copy link
Author

commented Sep 13, 2019

@Zaid-Ajaj Based on tests, we got 3x performance gain

@Zaid-Ajaj

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

Wohoo Nice! 🚀 it looks like these tests are failing:

I guess that is because Bson.deserialize and Bson.serialize are still mapper-dependent, can you parameterize these as well such that take and use a mapper as input?

Lastly, it would be great to add a little section to the README regarding how to use the type-shape mapper 🙏

@vshapenko

This comment has been minimized.

Copy link
Author

commented Sep 13, 2019

Wohoo Nice! 🚀 it looks like these tests are failing:

I guess that is because Bson.deserialize and Bson.serialize are still mapper-dependent, can you parameterize these as well such that take and use a mapper as input?

Lastly, it would be great to add a little section to the README regarding how to use the type-shape mapper 🙏

Bson.serialize and deserialize do not depend on mapper

I guess tests failed because i use different serialization format for DU etc

Also looks like quotation expressions heavily tied to Bson.fs

@Zaid-Ajaj

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

I see, I though we could make Bson.fs mapper-dependent but that is OK, I guess we can merge this, I will put the Bson specific tests in their own testList that only takes FSharpBsonMapper

@Zaid-Ajaj Zaid-Ajaj merged commit 7a7eef6 into Zaid-Ajaj:master Sep 13, 2019
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@Zaid-Ajaj

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

I will add a README section as well, will be able to work on this later this evening

@Zaid-Ajaj

This comment has been minimized.

Copy link
Owner

commented Sep 13, 2019

Published as of v2.12.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.