Skip to content

RavenDb issues #29

Closed
andreialecu opened this Issue Oct 4, 2011 · 3 comments

2 participants

@andreialecu

By the looks of it, it seems that the RavenDb library was never used because there are various problems with it:

1) RavenDbRepositoryWithTypeId incorrectly defines the T type as being constrained to Entity instead of EntityWithTypedId making it impossible to use.

2) BaseObject has the [JsonObject(MemberSerialization.OptIn)] attribute applied, meaning that entities are not being serialized properly by default and they would all need various other attributes applied on them. see http://groups.google.com/group/sharp-architecture/browse_thread/thread/1013519ac90e4ad1

3) RavenDb uses string based keys by default. Things like "users/1234". This means we'd have to use EntityWithTypedId on all entities and RavenDbRepositoryWithTypeId on all repositories, which is too verbose.

I suggest that RavenDbRepository is changed to use string keys by default instead of int keys. And a new class called RavenEntity be added:

public class RavenEntity : EntityWithTypedId<string>
{
}

4) RavenDbRepositoryWithTypeId should be RavenDbRepositoryWithType_d_Id for consistency purposes.

5) The NHibernateRepository implementation provides protected access to the underlaying ISession. RavenDbRepository has the IDocumentSession as private. It should be changed to protected and exposed as a property.

I'll attach a pull request with all of the changes. Comments are welcome.

@andreialecu

Regarding 3) -> I'm not sure what the best way to handle this is, putting RavenEntity in SharpArchitecture.RavenDb would mean that the Domain layer needs to reference it which seems bad.

I have attached a pull request at #30 that doesn't include the RavenEntity class.

Currently, I just put that small class inside my domain as a helper.

@andreialecu

Fixing this as I go with my app. The Get by id methods were also extremely inefficient. Instead of doing a simple raven load operation they were doing a query which makes raven make dynamic indexes for it. This is very bad practice and can also return stale results.

Also added a parameter to the Find/FindAll methods to allow waiting for non stale results.

@seif
sharparchitecture member
seif commented Jan 14, 2012

andreialecu, Thanks for your contributions, unfortunatly, we don't feel that we quite nailed the RavenDb implementation so have moved it from the 2.0 release into the develop branch.

@seif seif closed this Jan 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.