circles-arrows / blueprint41

An Object Graph Mapper for CSharp to connect to Neo4j or Memgraph.
http://www.blueprint41.com/
MIT License
31 stars 8 forks source link

A bunch'a questions #60

Open RobHudson72 opened 5 months ago

RobHudson72 commented 5 months ago

Reaching out to the rockstars of Neo4j, for your wisdom, and expertise!

circles-arrows commented 5 months ago
RobHudson72 commented 5 months ago

Beautiful response! I am saving this and adding it to documentation for my developers. It may be helpful to others if this reply could be stickied somewhere, so that it doesn't fall out of view once this issue is closed.

One of the things I am trying to solve for is DTO conversion, so I can have a well optimized API<->DataService layer in my app.

Let's say I have a list of movies, and related actors and directors.

The movie DTO might look something like:

public class MovieDTO{
    public string Uid {get; set;}
    public string Name {get; set;}
    public string GenreUid {get; set;}
    public string DirectorUid {get; set;} 
}

It seemed to me that the ideal place to do the DTO conversion would be in the constructor, like so:

public class MovieDTO{
    public string Uid {get; set;}
    public string Name {get; set;}
    public string GenreUid {get; set;}
    public string DirectorUid {get; set;} 

    public MovieDTO(Movie movie){
        this.Uid=movie.Uid;
        this.Name=movie.Name;
        this.GenreUid=movie.Genre.Uid;
        this.DirectorUid=movie.DirectorUid;
    }
}

There are several problems with this

The first problem is if movie members are not loaded, then a transaction is required for retrieving Genre and Director. In testing, it appears that if the Transaction is in a parent class, it doesn't get passed to the class constructor, meaning that the constructor errors.

using(Blueprint41.Transaction.Begin(){
    var movieObj=Movie.LoadByUid(uid);
    var moveiDto = new MovieDTO(movie);
}

Even if that were to work though, due to lazy loading, retrieving a list of movies would result in 3 database calls for each movie retrieved.

Doing this efficiently for a list of movies would require passing in the movie entity with members fully loaded. Maybe there is a way to do this with OptmizeFor.RecursiveSubGraphAccess. I definitely plan to test this. My hunch that it won't work, because the class constructor is outside of the transaction scope, so the transaction doesn't know which members are required, or how deep to go in the recursion to retrieve members.

I am puzzling that out. But fundamentally, I am not even sure if my approach to representing relationships in the DTO is aligned with good practice. What are the best practices for representing relationships in the DTO?

RobHudson72 commented 5 months ago

If you want the already committed part to retain even if the full transaction gets rolled back, you can simply make two transactions in a row. Commit the first transaction then create a new transaction for the rest of the changes.

I tried this, and I think I encountered a problem with using entities retrieved in the first transaction in the second transaction.

My memory on this is hazy... if I run into it again, I'll post a more specific use case to explore.

circles-arrows commented 5 months ago

For your 1st comment, taking your example:

using(Blueprint41.Transaction.Begin(){
    var movieObj=Movie.LoadByUid(uid);
    var movieDto = new MovieDTO(movie);
}

OptmizeFor.RecursiveSubGraphAccess would work if you would do the following:

using(Blueprint41.Transaction.Begin(){
    var movieObj1=Movie.LoadByUid(uid1);
    var movieObj2=Movie.LoadByUid(uid2);
    var movieDto1 = new MovieDTO(movie);
    var movieDto2 = new MovieDTO(movie);
}

OptmizeFor.RecursiveSubGraphAccess would NOT work if you would do the following:

using(Blueprint41.Transaction.Begin(){
    var movieObj1=Movie.LoadByUid(uid1);
    var movieDto1 = new MovieDTO(movie);
    var movieObj2=Movie.LoadByUid(uid2);
    var movieDto2 = new MovieDTO(movie);
}

Reason being is that when inside the constructor of MovieDTO the referenced nodes are loaded, the transaction is checked to see if other movies were loaded in memory and the property being accessed is loaded for all movies that are in memory (and recorded in the transaction). If the 2nd movie is not yet in memory when accessing the related property, the load will only happen for 1 movie when inside the 1st constructor call. Inside the 2nd constructor call the load will happen again, but now for the 2nd movie, which was in memory by that time.

Also, the constructor is inside & down the call stack, and therefore will have access to the transaction. The transaction is within scope inside the constructor.

Your 2nd comment about starting a 2nd transaction, even though the content loaded during the 1st transaction is available inside the 2nd transaction, you cannot alter the object loaded in the first transaction after it has been committed or rolled back. So, lazy-loading additional data or setting properties will cause the error you mention to happen. You should reload the object (by its Uid for example) while inside the 2nd transaction, if you want to change it there.

circles-arrows commented 5 months ago

I am not even sure if my approach to representing relationships in the DTO is aligned with good practice. What are the best practices for representing relationships in the DTO?

I think your idea of "passing the OGM object to the constructor of the DTO method" works well and is definitely a good solution in my mind.

How we did it to have an REST API with locking and versioning (backward compatibility) your could do it as follows:

We added a DateTime RowVersion field to all the entities, with a script like this:

Entities["xxx"].SetRowVersionField("VersionStamp");

Optimistic locking will then be supported in the REST API using the following flow:

  1. GET item from REST API which includes the lock (=current RowVersion field value)
  2. Edit item on the caller side, without changing the lock value
  3. Send item, including the lock, back to the REST API using PUT
  4. Api serializes the DTO, including the lock to its OGM object and commits the transaction

If the lock doesn't match the lock in the database, it means someone else changed the node while it was being edited but before it was saved in the graph. This means the optimistic lock apparently failed and the API should not be permitted to overwrite the edited node. This checking of the RowVersion lock is already included in Blueprint41 and comes for free if you add a RowVersion field to your entity. If the lock failed, an Exception will be throw inside the transaction commit and the transaction is automatically rolled back when it goes out of scope. The API then sends the client/caller a "failed" status with the reason "optimistic lock failed". The client can now restart at step 1 and get the item with its updated values, including the updated lock value.

Such an API needs conversion of both OGM to DTO and DTO to OGM. Of course adding a constructor to the OGM object is not how it's supposed to be done, so what we did is to add an explicit cast to the DTO to cast from OGM object to DTO object. We also also add an implicit cast from DTO to OGM.

The explicit cast is useful to support multiple versions of the REST API and the DTO's it uses. This way we were able to have a single database model with backward compatibility via a versioned REST API.

If you have more questions or simply want to brainstorm a bit about how this translates to your code base, feel free to contact me on Discord