MongoRepository icon indicating copy to clipboard operation
MongoRepository copied to clipboard

Added Support for 2.0 async driver

Open dhanilan opened this issue 9 years ago • 15 comments

Added Support for 2.0 async driver .I Will write test cases in the next commit. Any ideas for removing UpdateDefinition class usage in Update methods are welcome.

dhanilan avatar Jan 04 '16 08:01 dhanilan

Cool, cool. I'll have a look later this week. Thanks! :+1:

RobThree avatar Jan 04 '16 09:01 RobThree

The PR looks OK to me. Regarding the UpdateAsyc, I think it'll be good idea to have another version of Update with T entity. So might be UpdateEntityAsync(T entity)?

I'll approve PR once I see some tests. Good work.

tegeek avatar Jan 04 '16 09:01 tegeek

Code looks okay indeed (haven't tested it yet though) but the docblocks might need a little brush-ups to be in line (same wording, style) with the rest of the code, I'll get back to it as soon as I can.

RobThree avatar Jan 04 '16 10:01 RobThree

@tegeek we need to pass updateDefinition as a parameter UpdateEntityAsync(T entity, UpdateDefinition<T> updateDefinition); . Is that fine?

dhanilan avatar Jan 04 '16 10:01 dhanilan

we need to pass updateDefinition as a parameter UpdateEntityAsync(T entity, UpdateDefinition updateDefinition);

Preferrably not because it breaks the abstraction / leaks implementation specifics. MongoRepository should work this out internally (if at all required; I'd rather update the entire entity). I think MongoRepository should always work with (or at least default to) entities as a whole. Which also brings me to https://github.com/RobThree/MongoRepository/issues/23; I don't think that's a good idea unless we provide specific methods for it.

RobThree avatar Jan 04 '16 10:01 RobThree

Okay thanks @RobThree . I will use ReplaceOneAsync for Update. (Update entire document).

dhanilan avatar Jan 04 '16 10:01 dhanilan

Committed the same . Please review them . Have added the test case also. Thanks

dhanilan avatar Jan 04 '16 11:01 dhanilan

@tegeek won't that be complicated to build UpdateDefinition for the generic class T? May be loop through the properties of the class?please advice.

dhanilan avatar Jan 04 '16 16:01 dhanilan

Something like this?

       UpdateDefinition<T> updateDefinition= Builders<T>.Update.Set(x=>x.Id, entity.Id);
        var props = typeof(T).GetType().GetProperties();
        foreach (var propertyInfo in props)
        {
            if (propertyInfo.CanRead)
                updateDefinition.Set(propertyInfo.Name ,propertyInfo.GetValue(entity,null));
        }
        this.collection.UpdateMany(GetIDFilter(entity.Id), updateDefinition);

dhanilan avatar Jan 04 '16 17:01 dhanilan

To be honest: that seems like a horrible hack. I can't imagine we'd have to iterate over all properties to create an updatedefinition? I'm not (yet) very familiar with the 2.0+ driver but I'm pretty sure there must be a (much) better way. The sync (note: not async) methods can do fine without, I can't imagine the async variants would require such different approach.

Again: I'll take a look / give it a try a.s.a.p.

RobThree avatar Jan 04 '16 17:01 RobThree

Yea. I never wanted to do that way. Meanwhile shall update you if I found better approach. Thanks

dhanilan avatar Jan 05 '16 03:01 dhanilan

When will this merge? just started to work on implementing async then something told me to go check PRs and sure enough here it is.

@dhanilan Why are the GetByID async versions missing? @RobThree Shouldn't we be using SingleOrDefault() with GetByID() @RobThree @dhanilan @tegeek The Update() doing Add() if Entity hasn't got an ID violates SRP for update. Saving new entity through Update() should throw an exception.

Please respond.

brgrz avatar May 19 '16 21:05 brgrz

As above, it's been months now, is this still maintained?

matt-lethargic avatar Feb 02 '17 09:02 matt-lethargic

MongoRepository is still maintained but old mongo c# driver. The version 2 of driver has different things and it seems none of us has used/explored how to fit those things in MongoRepository.

tegeek avatar Feb 02 '17 09:02 tegeek

The the v2 branch does have support for the v2 driver, I've been using this in production with added support for async driver functionality. Just sent a pull request (https://github.com/RobThree/MongoRepository/pull/46)

On Thu, Feb 2, 2017 at 4:41 AM, Khurram [email protected] wrote:

MongoRepository is still maintained but old mongo c# driver. The version 2 of driver has different things and it seems none of us has used/explored how to fit those things in MongoRepository.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/RobThree/MongoRepository/pull/24#issuecomment-276910789, or mute the thread https://github.com/notifications/unsubscribe-auth/AT0npv0RsmmZ3NTYjE07uqliEZUrkgq8ks5rYaTMgaJpZM4G968m .

f12358 avatar Feb 02 '17 15:02 f12358