Empty IEntity

[dberry-rcs]

!!!!!!!!!!!!!!!!!!!!!! Tests are not passing, DO NOT MERGE to master !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

I tried getting the library to support multiple ID types. The farthest I got was by deleteing ID from IEntity and changing GenerateNewID to return object. The code can now support _id, ID, or any [BsonId] property

Before I put more time in trying to fix the rest of the tests, I'd like to get your opinion on the direction. I tried several times to get IEntity working as IEntity, but at some point it hits a wall or you end up decorating everything with the type id. e.g. Cache<Book, string> DB.Collection<Book,string>, etc. I found it quite ugly.

The relationship code is not working well right now. ChildrenQueryable is not returning anything. add_child_to_many_relationship_with_ID is a good test to debug. If you could have a quick look I'd appreciate it.

[dj-nitehawk]

i like the direction you're going with the object? IDs. also added some minor optimizations. sooo... carry on i guess... and let me know if you get stuck somewhere... i'll try my best to help ;-)

[dj-nitehawk]

sooo... i've been trying to figure out how to make the relationships work with object based IDs and haven't yet found a good solution, as it seems like a hard problem to solve without invalidating the data already stored with current projects.

i'll keep trying...

if you decide to proceed on this route, please keep in mind that we should try to avoid any major breaking changes that would prevent existing users of the library from easily upgrading to the latest version. some amount of code changes would be tolerable but mass database changes should be avoided at all costs.

[dberry-rcs]

Understood about the breaking changes. As a user of the library, I am not looking for large refactorings on my projects.

[dberry-rcs]

@dj-nitehawk Is this a possibility for fixing the relations?

https://stackoverflow.com/questions/41093647/mongodb-join-on-id-field-from-string-to-objectid

if we do an .AppendStage, and do an $addfields to convert the id fields to strings and then always do a string comparison on the ids. This way, whether or not they are stored as ObjectIds, they would work.

It would require a minimum of mongodb 4.0

[dj-nitehawk]

might work... but i believe it would add a significant amount of overhead on the mongo server for something that is already a slowish process (lookups).

if we do a $toString on the local field, say in a db with millions of docs, the server now has to allocate millions of strings to compare against instead of what used to be an index scan. right?

[dberry-rcs]

@dj-nitehawk Looks like it is closer to passing. 1 error. JoinRecord needed a custom serializer that worked with object? I created a new attribute AsBsonId modeled after AsObjectId. The relationships are now passing. Not sure what this latest error is.

[dberry-rcs]

Odd that it is only happening on the Transaction Delete. I'll dig in some more.

[dj-nitehawk]

can you try updating the project to the latest mongo driver? this issue might possibly be related.

[dberry-rcs]

No joy

[dberry-rcs]

@dj-nitehawk I saw that you pushed out a new version, so I updated the branch and pushed the PR. Still one error, but it is on Templates this time. I think it is this line:

.Path(b => b.MainAuthor.ID)

it generates:

{MainAuthor: {ID: "647bb1dde07b5a673c155ee8"}, authors: []}

but the id is not wrapped as an ObjectId.

[dberry-rcs]

Looks like the ID field on the One class is not getting set correctly. I will keep working on it. So close.

[dberry-rcs]

@dj-nitehawk Finally it passes. Please take a look at One to see the fix. I needed to store the T locally so that the ID field would have the correct type and serialization would work properly. If you are good with this I will start to look at the MultiDB support.

[dj-nitehawk]

great!

will review as soon as i can...

might be a few days though.

[dj-nitehawk]

so.... i think i've managed to solve it without having to store the T inside One<>. checkout the dev branch, my fix is in there. i'll close this PR since it now conflicts with what i've done. if the dev branch looks ok, would it be possible for you to add a few tests that cover usage with a couple of different ID property types which people might wanna be using it with? you know guid/int/etc...

[dberry-rcs]

Will do.

[dj-nitehawk]

Will do.

hey mate! have you had a chance to look at the dev branch yet? no rush ;-)

[dberry-rcs]

I have. I think your changes look good, but wanted to make sure they are well tested. I tried refactoring the tests so that we can rerun all the tests for String, Long, ObjectID, and GUID ids. Unfortunately, you cannot have tests that take a generic, so I am trying to figure out a clean way to do this without have four sets of almost identical tests.

[dj-nitehawk]

all right cool... take your time and send in a PR whenever you're ready :-) thanks again for your effort! have a great day...