Added support for MVC attribute routes (fixes #767) #769

Merged
merged 12 commits into from Apr 23, 2014

Projects

None yet

4 participants

@PaulAtkins
Contributor

Updated Glimpse.AspNet Route to work with System.Web.Mvc.Routing.RouteCollection to fix MVC attribute routing issue #767

no767 mvc attr routing

@avanderhoorn
Member

As usual, great PR mate!!! Just wondering if you can update the MusicStore sample to include a couple of attribute routes so we have a working demo?

@avanderhoorn avanderhoorn added this to the vNext milestone Mar 25, 2014
@avanderhoorn avanderhoorn self-assigned this Mar 25, 2014
@PaulAtkins
Contributor

Yep, no problem. Is there already a MusicStore sample for Mvc5? I can see the Mvc3 and Mvc4 ones, but not one for Mvc5. Or maybe I'm looking in the wrong place...?

@CGijbels
Member

No there is none (yet)

When MVC 5 was released, @avanderhoorn asked @jongalloway (see tweet here) whether he had any plans on updating the Music Store sample, but I think @jongalloway has yet to find some time to do that?

Otherwise, create a copy and do a manual upgrade. But you won't have any MVC5 specific features then (if you don't add them yourself 😉)

@avanderhoorn
Member

Sorry, totally forgot that the sample proj was MVC4 not 5. Given this, how
about we just add a "file new project" MVC5 app that has a couple of
attribute routes. This isn't as good as having the music store, but we can
use it for MVC5 specific features like this.

On 25 March 2014 19:31, Christophe Gijbels notifications@github.com wrote:

No there is none (yet)

When MVC 5 was released, @avanderhoorn https://github.com/avanderhoornasked
@jongalloway https://github.com/jongalloway (see tweet herehttps://twitter.com/anthony_vdh/status/390960003815063552)
whether he had any plans on updating the Music Store sample, but I think
@jongalloway https://github.com/jongalloway has yet to find some time
to do that?

Otherwise, create a copy and do a manual upgrade. But you won't have any
MVC5 specific features then (if you don't add them yourself [image:
😉])

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/769#issuecomment-38634385
.

@PaulAtkins
Contributor

No worries! I've copied the Mvc4 Music Store sample project and followed the steps by Rick Anderson to upgrade it to Mvc5

But when I try running the project I was originally getting the cast exception below from this line in GlimpseDbProviderServices.cs.

Unable to cast object of type 'System.Data.SqlClient.SqlConnection' to type 'Glimpse.Ado.AlternateType.GlimpseDbConnection'.

Line 32:                     using (var context = new UsersContext())
Line 33:                     {
Line 34:                         if (!context.Database.Exists())
Line 35:                         {
Line 36:                             // Create the SimpleMembership database without Entity Framework migration schema

[InvalidCastException: Unable to cast object of type 'System.Data.SqlClient.SqlConnection' to type 'Glimpse.Ado.AlternateType.GlimpseDbConnection'.]
   Glimpse.EF.AlternateType.GlimpseDbProviderServices.GetDbProviderManifestToken(DbConnection connection) +96
   System.Data.Entity.Core.Common.DbProviderServices.GetProviderManifestToken(DbConnection connection) +132

[ProviderIncompatibleException: The provider did not return a ProviderManifestToken string.]
   System.Data.Entity.Core.Common.DbProviderServices.GetProviderManifestToken(DbConnection connection) +327
   System.Data.Entity.Utilities.DbProviderServicesExtensions.GetProviderManifestTokenChecked(DbProviderServices providerServices, DbConnection connection) +67

But after a few clean and rebuilds I'm getting Parser Error Message: Could not load type 'MvcMusicStore.MvcApplication'. Line 1: <%@ Application Codebehind="Global.asax.cs" Inherits="MvcMusicStore.MvcApplication" Language="C#" %>

@PaulAtkins
Contributor

Fixed it. I've added a simple test controller and view, with 3 attribute routes:

image

@avanderhoorn Could you give the Glimpse.Mvc5.MusicStore.Sample a try and check it works for you? Thanks.

@jongalloway

I've got updated code for MVC 5 (taking advantage of some of the other
features, bootstrap, etc.). I won't have time to wrap up and send out until
the second week of April - completely busy on Build conference.

On Wed, Mar 26, 2014 at 4:46 PM, PaulAtkins notifications@github.comwrote:

Fixed it. I've added a simple test controller and view, with 3 attribute
routes:

[image: image]https://camo.githubusercontent.com/ce7aebb8de3160c18c5aea189e30322bf4d33966/68747470733a2f2f636c6f75642e67697468756275736572636f6e74656e742e636f6d2f6173736574732f363437343330372f323533323335362f38363432313833612d623534302d313165332d386230332d6261656233396533636334392e706e67

@avanderhoorn https://github.com/avanderhoorn Could you give the
Glimpse.Mvc5.MusicStore.Sample a try and check it works for you? Thanks.

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/769#issuecomment-38754552
.

@PaulAtkins
Contributor

@jongalloway sounds great! (I'd forgotten about the bootstrap stuff). Once you have it ready we can remove the one above and add the proper one :)

@avanderhoorn
Member

Are we happy enough with this fix for it to go out in the next release?

@avanderhoorn avanderhoorn changed the title from no767 MVC attribute routes fix to Added support for MVC attribute routes (fixes #767) Apr 21, 2014
@avanderhoorn
Member

@PaulAtkins Any chance you can update this PR so it can be merged in?

@PaulAtkins
Contributor

@avanderhoorn I've updated from the Glimpse master and updated the EF package to 6.1.0

@avanderhoorn
Member

One very last thing, can you update update the DB's to follow the pattern as the other sample projs DBs do? That way they don't need to be checked with every commit. You will note that there is a build task in the proj file which setups up the db based on the readonly copy.

@PaulAtkins
Contributor

The build tasks were already in there (I copied the project from the Mvc4 sample). I've deleted the non-readonly mdf and ldf files.

@avanderhoorn
Member

Huge thanks. I think this means we are good to go!

@avanderhoorn avanderhoorn merged commit ca463a9 into Glimpse:master Apr 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment