Add CommandSource to CommandEventData

[Giorgi]

This PR is my attempt to implement #23719

If the approach looks good I can fix the failing tests and add new ones for this feature.

@ajcvickers Thoughts?

[roji]

I think some sort of tag mechanism for updates (#14078) would probably be more appropriate... This approach tells you that the command is coming e.g. from SaveChanges, but you still have no idea what is being changed and how. That seems to provide limited usefulness to interceptors and the like.

[Giorgi]

Tags for SaveChanges sounds useful but I think these are two different use cases. The scenario that I had in mind is that if you run a batch update with ExecuteSqlRaw it might be useful to differentiate between that and SaveChanges

[roji]

Tags for SaveChanges sounds useful but I think these are two different use cases. The scenario that I had in mind is that if you run a batch update with ExecuteSqlRaw it might be useful to differentiate between that and SaveChanges

Wouldn't SaveChanges (and raw SQL) tags fulfill the same need? Also, assuming your application has many different things being executed by both SaveChanges and ExecuteSqlRaw, how useful is it to just know that something is SaveChanges or ExecuteSqlRaw?

[Giorgi]

I guess it would if you are writing the queries and the interceptors but if you are writing an interceptor for others to use you probably can't rely on tags being present.

[roji]

I guess it's hard for me to imagine a generic interceptor (written for others) which cares specifically about whether a command comes from SaveChanges or ExecuteSqlRaw... Do you have any example in mind?

[Giorgi]

I wanted to process exceptions caused by ExecuteSqlRaw but IDbCommandInterceptor fires for both ExecuteSqlRaw and SaveChanges so I needed a way to find which one caused the interception to fire. For SaveChanges I use ISaveChangesInterceptor as it gives me access to the DbUpdateException to process. That's why I opened #24193

I guess it's a very specific use case but that's where the idea came from :)

[ajcvickers]

@Giorgi I disagree with @roji here. I wanted this when I was writing an interceptor example for someone, and multiple customers have also asked for this. Tags for updates is also useful, but is overkill for many cases where I just need to know the event came from SaveChanges.

[Giorgi]

@ajcvickers So should I implement tests for the new feature or do you want to discuss it first internally? Or maybe you want changes made to the way it is implemented?

[ajcvickers]

Looking good; added some comments around avoiding breaking changes.

[Giorgi]

@ajcvickers The test failed because PassiveReaderCommandInterceptor is used in two different tests: one that uses FromSqlRaw and another uses LinqQuery. Should I create a separate interceptor for each case or do you have a better idea for testing this feature?

[ajcvickers]

Looking good! Sorry for being so slow on this.

[Giorgi]

@ajcvickers Updated

[ajcvickers]

Thanks @Giorgi! Sorry for being so slow.

[Giorgi]

Thanks for merging @ajcvickers