ironcev / Pragmatic

[The description is yet to come]
5 stars 3 forks source link

Missing registration of standard interaction handlers for classes that are not entities #20

Closed mroncev closed 4 years ago

mroncev commented 9 years ago

Hello,

There is a method RegisterStandardInteractionHandlersForEntities which works only with entities defined in PragmaticEnvironment.EntityAssemblies.EntityTypes collection. For all other types of classes (such as projections), standard queries cannot be used.

Given the following code:

// Classes
public class Song : Entity
{
}

public class SongProjection
{
}

// Client code call
QueryExecutor.GetAll<SongProjection>()

you will get the infamous "There is no query handler defined for the queries of type 'Pragmatic.Interaction.StandardQueries.GetAllQuery`1[SongProjection]...' " error.

The only way to circumvent this is to create custom query and handler to retrieve the projections. Is it possible to add support for this in Pragmatic, or custom queries should always be used for such cases?

Thank you!

ironcev commented 9 years ago

Interesting feature proposal. I gave it a long thought and also investigated in depth how projections are used in one project heavily based on Pragmatic. Here is the summary of my views so far.

It is important to understand that the standard interaction handlers actually do not work for entities just because they are Entitys. They work because the underlying ORMs have them registered and know how to deal with them. What I'm saying is, if the entity Song is not mapped by NHibernate or not listed in the DbContext of the Entity Framework, the standard query handler will be created by Pragmatic, but the exception will be thrown during their execution, simply because the underlying ORM has no clue what to do with the Song.

This premise that "we have to teach the concrete provider what a Song is" is very important and extends also to projections.

Keeping the previous sentence in mind, we can say the following. In order for Pragmatic to be able to create standard query handlers for projections, it has to be able to "explain" to underlying ORM what a particular projection is. And this definition must be defined by the programmer. It cannot be generated by Pragmatic without knowing how to actually create a projection.

For example, the SongProjection can contain song's album name (coming from the Album entity), rating (coming from Rating entity), comments (coming from Comment entity) etc.

In e.g. Entity Framework, the programmer must write something like this to load a SongProjection:

.Select(song => new SongProjection
{
    Album = context.Set<Album>().Where(album => album.Id = song.AlbumId).Select(album => album.Name);
    ....
})

This is something that cannot be guessed by Pragmatic.

So far so good. We can imagine a method similar to RegisterStandardInteractionHandlersForEntities that allows us to "teach" the underlying ORM what SongProjection is and allow it to build standard queries on the fly.

E.g.

RegisterStandardInteractionHandlersForEntities(projectionQuery, [type information])

where the projectionQuery is the query we would usually write to load SongProjection out of Song.

But already at this point my alarm starts ringing and I'm asking myself isn't this adding additional complexity to Pragmatic (which is by far in my opinion fairly complex for what it does) just to generically solve a corner case.

Than I investigated the projection queries in my project and came to conclusion, that even if I had such method, it would be useless because of the following two reasons.

The first one is what I call "dynamic projection definition" (I coined the term 5 minutes ago :-)). Basically, that means that the projectionQuery contains values that has to be supplied at runtime. In the project I took a close look at literally all projections take a time span which ends with midnight of the current day. "The midnight of the current day" is not something that can be hardcoded in the query by using DateTime.Now because we are "mocking the clock".

This is just one example of a "dynamic projection definition". I can see others, like. E.g. projection depends on the rights that the currently active user has.

If we want to cover "dynamic projection definition" on the Pragmatic level, it could be possible somehow, but I'm afraid that we would cross that boundary at which the complexity of the usage (explain to Pragmatic users all this) does not justify the benefits we have (not need to write the GetOneQuery`).

The second thing is the usage without custom query. To have a generic GetOneQuery for projections makes sense if we can often enough write something like:

queryExecutor.GetOne<SongProjection>(song => song.Album = "Sleep Through The Static")

After analyzing hand written equivalents of GetOneQueries on my project I realized that all of them are real queries, meaning they have some attributes that has to be interpreted in the query handler in a way which a simple lambda expression cannot cover.

This makes me ask a question how often would this really be used.

The last thing is performance. At this point, we should do some measurement because the eventual performance penalty heavily depends on the way how ORM maps the query and how the database executes it. It could be that we wouldn't have performance penalty on e.g. Entity Framework + SQL Server, but it is a concern worth pointing out.

Namely, all the queries that include projections are currently formulated like this (in pseudo code):

from Song
where some Song attributes
select SongProjection
where some SongProjection attributes

Note that we are filtering the Songs before selecting projections out of them. This maps to SQL that first scans the Song table, filters out relevant entities and than creates subquery for SongProjections.

If we go for the generic version in which the programmer "teaches" Pragmatic what the SongProjection is, and than we add lambdas on top of it in the GetOne and GetAll queries, we would end up with this equivalent:

from Song  
select SongProjection
where some SongProjection attributes

Of course, the Song attributes are included in the SongProjection attributes.

SQL Server will most likely be intelligent enough to create query plan of equal performance, but that's something we should test.

And if it doesn't create an equally performant query, the story goes on. We have to teach it how to do it, and the whole story gets more complex.

This answer is becoming too long already :-)

To sum it up, my feeling is that such feature would introduce additional complexity (this for sure). It is questionable if it would pay of. This most likely depends from project to project. In the concrete project I'm working on, it wouldn't bring any benefits, in your maybe yes.

What we could do is to create a branch in which this kind of support could be implemented. But I personally would like to focus my time spent on Pragmatic on some other topics which I find necessary. Since there is no official roadmap for Pragmatic I'll list them here for you information:

Oh, what I forgot to mention, please feel free to contribute to the code base and extend Pragmatic on your own :-) Pull requests are always welcome! :-)

Please share your thoughts.

I'm adding @hudo to the conversation, since he is also very interested in the future features of Pragmatic.

mroncev commented 9 years ago

Thank you for the elaborate and quick answer!

I agree with you when you say that Pragmatic query handlers make sense only if underlying ORM knows about entities which are being queried. Of course, as you also noted, the same holds for any class that is returned by ORM, whatever we call it (in our case we call it projection). Actually, if you look at definitions of Pragmatic standard queries, they are defined (as it should be), with T : class constraint, not T : Entity.

In reality, since RegisterStandardInteractionHandlersForEntities method connects interaction handlers only for entity types registered in the PragmaticEnvironment.EntityAssemblies, we can say that, out of the box, Pragmatic (i.e. Pragmatic.StructureMap) supports only wiring interaction handlers for entity types. Of course, nothing stops me from wiring Pragmatic standard interaction handlers for projection classes in my project (which I did in order to avoid writing custom queries). I understand there are projects which does not need such feature, but in my case, it is of great use. Even though my projections are complex, they still fit very nicely with generic queries.

I understand you have other priorities and I don't mind contributing to the code. The question is do we want to add this feature to Pragmatic i.e. should it be part of Pragmatic? If you don't like the idea of putting it there, I don't want to force it. If you are fine with it, do you have a suggestion how to implement it? I was thinking of adding some marker interface for all classes which are not entities, but which can be handled by standard interaction handlers (only the Get ones). Those classes can then be registered and wired to interaction handlers similar to the PragmaticEnvironment.EntityAssemblies approach. What do you think?

Thank you!

ironcev commented 9 years ago

Hi Marin,

If I understood your answer correctly, you already have the solution for the feature that you need and what you are asking is should we put it to Pragmatic and how to identify projection classes.

How to identify the classes I see as a simple question. The proposal with the attribute sounds good. We can see if some other approach would fit, and play around with a proposal or two. But that's a minor design detail.

Should we put it to Pragmatic? I would say "Yes!" :-) But before we do it, let's make sure that we both talk about the same feature :-) Out of that what I described you can see that I perceive it as a very complex feature wit limited usage, if any. Out of your answer:

Of course, nothing stops me from wiring Pragmatic standard interaction handlers for projection classes in my project (which I did in order to avoid writing custom queries).

Even though my projections are complex, they still fit very nicely with generic queries.

I see that you see it as a simple feature that you already have running in your project and that is very useful. This can mean that we are not talking about the same feature :-)

Would it be possible to provide some pieces of source code from your project? Ideally, those that would answer the questions I stated in my previous comment. For example, this would give us a good concrete example on which we can sketch the feature and make sure that we are talking about the same thing:

My questions are those I wrote in the previous comment:

In addition, I have to ask if your current solution works on the Pragmatic level. That means it is not ORM specific. Will it work e.g. for all three ORMs/databases currently supported by Pragmatic: NHibernate, Entity Framework and RavenDB?

I hope that answering these questions needs just some copy paste from your side.

Alternatively, if you have the code already the quickest way to proceed is to fork Pragmatic, copy your code in fork and than continue the discussion on a real example. This is the most efficient way, but I'm not sure if it takes more time from your side.

Long story short - I'm looking forward to a nice feature contribution :-) Let's just try to see upfront if it fits, to save you of writing something that maybe doesn't fit. But out of you answer, I got an impression that the feature is useful, it runs already and we "just" have to import it :-)

Thanks again for an interesting feature proposal.

mroncev commented 9 years ago

I think we are going somewhere :). Beside obvious point of how to teach ORM/RavenDB what the projection is, it seems to me that the confusion lies also in mixing two completely different things, namely complex query and complex projection. I assume we agree that all our projections come from underlying database. They can be quite complex, but, when I say complex, I mean on a database level. In SQL world, this would almost always be implemented with the underlying indexed view which projects data from different tables. Most often than not, just having a projection and default Pragmatic queries does not mean we would not need to make custom queries/handlers. This is valid for entities as well. I see at least two situations where custom queries are necessary:

  1. When you have dynamic queries (e.g. filter criteria that can change depending on the user input)
  2. When you have a complex query which the underlying Linq provider implementation cannot understand and process.

The second point is related to your question:

In addition, I have to ask if your current solution works on the Pragmatic level. That means it is not ORM specific. Will it work e.g. for all three ORMs/databases currently supported by Pragmatic: NHibernate, Entity Framework and RavenDB?

When considering SQL databases, in practice, changing ORM is not something you do unless you find yourself in a big trouble with it. Both NH and EF seems mature enough to be used in everyday project. At least I don't see the viable alternative in SQL world.

But, your point becomes important when it comes to RavenDB. I had experience working with it. Although very nice on the surface, it was unreliable when it comes to indexes and I don't think I will use it again. Aslo, I don't think having support for standard projection queries in Pragmatic would benefit RavenDB users since RavenDB requires index for every query. In practice, this means, if you want to query projections, you need to tell DocumentSession explicitly which index to use. NH and EF does not suffer from this, since everything is handled via mappings.

Thus said, if we consider only NH and EF, I can say feature would be very helpful. If you insists on support for all three (NH, EF, RavenDB), then it's a no brainer, the feature cannot satisfy requirement and it should not be part of Pragmatic.

Answers to your other questions:

How do you "teach" Pragmatic "what an Album is in the SongProjection"? In other words, how Pragmatic and the underlying ORM knows what SongProjection is?

A: Via mapping files. NOTE: As said above, RavenDb is unteachable outside of the custom query handler itself.

Which standard queries do you support? GetAll, GetById, and GetOne I assume. Does it make sense to have it for GetCount since you can most likely always count the entities? (Although I can imagine situations where a projection would be needed for counting as well.)

A: All Get queries, including the TotalCount.

I am attaching an example source code which you can download here: https://goo.gl/2Lqlgf. Please note that I didn't put anything ORM specific in it. The reason behind is I don't think the issue is about whether it is possible to teach ORM/RavenDB how to handle projections (since obviously you can't teach RavenDB anyway), but whether we want Pragmatic to wire projections with default Get queries/handlers at least for NH/EF.

To conclude: IMHO, if support for all three ORM/databases is mandatory, feature is off, otherwise, lets implement it :).