Improve support for embedded resources and NuGet it up #1331

jrsconfitto · 2013-11-19T20:40:55Z

This PR seeks to put embedded content on the SDHP by fixing a remaining issue and closing out #732.

Questions i have

~~Should i move everything related to embedded views out of core Nancy and into Nancy.Embedded?~~
When wiring up embedded views for Nancy, i followed this post by Simon Cropp. It was pretty simple, but according to Create Nuget for working with embedded stuff #732 it sounds like @grumpydev had some ideas on how to improve wiring up embedded views. Is there a better way to wire up embedded views?

Done

Fix issue with embedded views in an outside assembly mentioned in this mailing list post. Thanks for the fix, Rex!
Add an extension that allows an AddDirectory for embedded static content, similar to the extension method on the default StaticContentConventions
Wire up embedded views via an ApplicationRegistration
Figure out the best way to handle issue with no RootNamespaces added and only one embedded view to serve.

jrsconfitto · 2013-11-19T21:00:04Z

cc @khellang

jrsconfitto · 2013-11-19T21:15:31Z

OK, i found some guidance from this mailing list post that talks about wiring this all up via an IApplicationRegistration.

jrsconfitto · 2013-11-20T23:12:03Z

OK, i've added a basic registrations class to the Nancy.Embedded stuff to make wiring up embedded views a little more automatic.

While running a demo project, i ran into an issue described in the mailing list by @thecodejunkie where i only had one embedded view and hadn't added to the RootNamespaces property. This throws a TinyIoCResolution exception because of what @thecodejunkie describes in the aforementioned mailing list post:

Now in the special case where there is only one resource located, it's unable to derive a common namespace, and since it's impossible to read out the "default namespace" that is registered in a project, nancy cannot stip out the common namespace. You can think of the common namespace as the rootpath of your application when you use the filesystem base view locator

When i add another view, or add my namespace to the RootNamespaces dictionary, everything is 👌

i don't think this is very SDHP. Someone like me will test it out on one view and see if it worked and probably be 😠. It seems (to me) like a common base case for someone to try out Embedding content.

Should we try to fill in RootNamespaces automatically for someone? Or should we work on throwing a nicer error so that someone can catch this issue quickly? i think at least a 500 would be better. i think i'll try to go down that route unless i hear different.

reidperyam · 2013-11-21T19:47:09Z

Juggling - not sure if this is applicable to your extensions or not; I am having a pig of a time getting my Nancy.Test.csproj to load my embedded resource views for testing. The views exist in a separate App.UI assembly (which is separate from my App.Nancy assembly) ; I forcibly load the App.UI assembly into the NUnit execution AppDomain at runtime.

Mostly weird because everything is peachy when I F5 the actual application (after implementing #1332 ) -- views are picked up and served as expected, just not within the testing context. My NancyModules can't bind the view even though its resource path is registered with its parent assembly via ResourceViewLocationProvider.RootNamespaces.Add(...) and the testing browser's bootstrapper has its
ViewLocationProvider = typeof(ResourceViewLocationProvider). Hard to debug wondering if any of this rings a bell.

jrsconfitto · 2013-11-21T19:55:04Z

i haven't tried any tests on embedded stuff. i'll try to reproduce this and make sure it doesn't affect the work on this PR.

In the meantime, is Nancy.Test.csproj the name of your test project? i'm not sure if this is an issue, but my first guess would be to try changing the assembly name of your test project and make sure the name isn't causing Nancy to ignore your test assembly.

reidperyam · 2013-11-21T20:03:39Z

Good suggestion; however my test assembly, containing Nancy tests, is actually *Application.Test -- so no references to Nancy. I'll keep digging; appreciate the response.

jrsconfitto · 2013-11-21T20:16:34Z

i have working tests in my test project for this PR, so i'm not sure what could be wrong.

Here's a gist of my test class in case an example might help point out a direction to pursue in debugging your issue.

If you ping me in chat maybe i can help you out.

khellang · 2013-11-22T16:15:38Z

Oh snap! 👊 The Mono build hates me 💔 😢

khellang · 2013-11-22T16:16:49Z

Figure out the best way to handle issue with no RootNamespaces added and only one embedded view to serve 😉

jrsconfitto · 2013-11-22T16:20:38Z

Figure out how to make the mono builds not fail on gem install rake ... boo on connection errors 😦

jrsconfitto · 2013-11-22T16:24:23Z

OK, @thecodejunkie & @grumpydev, i think this is ready for some review. Mind throwing some 👀 over this one?

This fixes a bug discovered by Rex Whitten and mentioned in the mailing list: https://groups.google.com/d/msg/nancy-web-framework/9N4f6-Y4dNA/ZcGu35SBMoYJ

…w exceptions if there's only one view found

jrsconfitto · 2013-11-25T16:22:07Z

Rebased against master, sorry if that blows anybody's work away 😭

jrsconfitto · 2013-11-25T16:22:51Z

Apparently rebasing breaks travis?

This closes NancyFx#732 and closes NancyFx#1332.

jrsconfitto · 2013-12-04T18:41:44Z

i added a home route and view to your example because some dumb guy was confused when he navigated to the host URL and saw a 404 monster. He also might have incorrectly assumed that @khellang screwed up 😝.

jrsconfitto · 2013-12-04T20:31:41Z

AIGHT! We're good to go @thecodejunkie!

khellang · 2013-12-05T17:56:21Z

Is there a reason why the AccountModule.cs contains the HomeModule and the HomeModule.cs contains the AccountModule? 😝

khellang · 2013-12-05T17:59:33Z

src/Nancy.Embedded/EmbeddedViewLocationProvider.cs

+        /// <summary>
+        /// Initializes a new instance of the <see cref="EmbeddedViewLocationProvider"/> class.
+        /// </summary>
+        public EmbeddedViewLocationProvider() 


Can we ditch this constructor and register the DefaultResourceReader in Registrations?

jrsconfitto · 2013-12-05T18:40:19Z

Is there a reason why the AccountModule.cs contains the HomeModule and the HomeModule.cs contains the AccountModule?

IF IT AIN'T BROKE DON'T FIX IT.

Good catch, i must not have kept track of which file i was editing when i copied the account module into the home module.

Use IoC for great good!

reidperyam · 2013-12-06T18:46:13Z

Hey @jugglingnutcase - have an issue serving static files as embedded resources (from a separate assembly) using the StaticContentsConventions along with Microsoft.Owin.Host.SystemWeb . IIS doesn't seem to like Nancy from OWIN. (example: http://salesapp.us/_Nancy ) I made a repo for a repro: http://goo.gl/FtqLmH ; everything works fine when self hosted on a cloud service: (http://sel2.us/_Nancy) .

jrsconfitto · 2013-12-09T16:15:31Z

@TheFastCat i see you were able to post this issue on the mailing list. Fortunately for this PR, it looks like it's not related.

jrsconfitto · 2013-12-11T16:50:08Z

@thecodejunkie Any chance of this getting into 0.22?

jrsconfitto · 2014-01-30T15:33:55Z

At this point, i'm warring with this PR. Let me vomit out my thoughts for you 👅

Here's what i'm using in my own project that i want this PR for:

We're embedding Nancy in a desktop application, so i'm not sure if i can use Cassette... i need to do more research
OWIN-hosted
DotLiquid views
A bunch of css/js static resources

Here's what i want:

i want to package all my views and resources into a dll because that eases deployment for my team's leader who runs the installation stuff
i want to be able to have both debug and release versions
i want to have minification and compression going on too

This PR meets my first want, but not the others, i think (unless i do some ugly hackery).

Some options i'm considering for moving forward:

Get this pulled in because other people think it's useful as part of Nancy proper. Should i? cc @khellang, @thecodejunkie
Maintain this separately from Nancy... i'll pull this out of Nancy, make my own repo, and distribute it via NuGet.
Ditch this for something that better suits my needs, like an OWIN-compatible version of Cassette. This is the direction i'm planning to head in, if not now, soon(ish).

i feel obligated to point out that option 3 doesn't really exclude the other options. i'm willing to put this out as its own NuGet package and then move forward on my own.

Any thoughts?

khellang · 2014-01-30T15:46:04Z

I still want this very much, the embedded story is lacking a bit today. I don't really care whether it's part of the Nancy repo or not, t'll still be a separate NuGet package 😄

reidperyam · 2014-01-30T16:01:41Z

I want it as well but since it's been a few months on getting out the door I've started investigating Owin.StaticFiles to serve the same purpose. randompunter and pinpointtownes and tracher have been helpful with the transition. It looks like it will simplify my verbose Nancy StaticContentsConventions I currently have to define:

...but there is an issue that needs to be resolved first... https://katanaproject.codeplex.com/workitem/202

khellang · 2014-01-30T16:14:29Z

Haha! Remember - there's nothing stopping you from creating your own extension method 😄

xt0rted · 2014-01-30T17:06:46Z

@jugglingnutcase I started an OWIN middleware project for Cassette in case you haven't seen it. It could use some unit test love (it currently has none) but otherwise it seems to work well for my needs.

jrsconfitto · 2014-01-30T17:24:23Z

@xt0rted i had that project in mind when i wrote that 😉 i should've put in a link...

i'll be exploring that project soon 😄

jrsconfitto · 2014-01-30T17:53:54Z

I don't really care whether it's part of the Nancy repo or not

@khellang i'm also OK with either. If i get a merge, then 🆒. If not, i'd like a thumbs down or a ping at some point (i'm not in a huge rush) so i can start moving this out to another repo.

jrsconfitto · 2014-01-30T17:58:59Z

It looks like it will simplify my verbose Nancy StaticContentsConventions

@TheFastCat Maybe you've already thought through this, but why aren't you just putting all your javascripts in a common folder and then adding the parent scripts directory? Wouldn't that work without all that code ceremony? Am i missing something?

This reverts commit 6ccb39a. Conflicts: src/Nancy.Embedded.Tests/Unit/EmbeddedViewLocationProviderFixture.cs src/Nancy.Embedded.Tests/Unit/ResourceViewLocationProviderFixture.cs src/Nancy.Tests/Unit/ViewEngines/ResourceViewLocationProviderFixture.cs src/Nancy/ViewEngines/ResourceViewLocationProvider.cs

jrsconfitto · 2014-03-03T13:44:43Z

i'm planning on moving this stuff to my GitHub account and releasing it through NuGet. i'll let you know when it's up 😄

This code is branching off the work done in NancyFx/Nancy and the work in NancyFx/Nancy#1331.

jrsconfitto · 2014-03-04T14:41:59Z

Okey-dokey, it's moved.

i'll publish it up on NuGet soon. Please let me know if there's any licensing stuff that i need to take care of.

khellang mentioned this pull request Nov 20, 2013

Implementing fix to issue where Nancy can't find embedded resource views... #1332

Merged

jrsconfitto and others added 16 commits November 25, 2013 11:20

Discover views registered from other assemblies

0f17644

This fixes a bug discovered by Rex Whitten and mentioned in the mailing list: https://groups.google.com/d/msg/nancy-web-framework/9N4f6-Y4dNA/ZcGu35SBMoYJ

Fix whitespace

e859118

Add extension helper for embedded static content

8d2e66a

Update xml documentation

bc3606b

Including Rootnamespaces should not break ignore

cceb6cc

Move embedded resource provider to Nancy.Embedded

6ccb39a

Rename the embedded view location provider

02dbca6

Add registrations class

85ef742

Remove System.Reflection from registrations

ce76144

Add docs

2407ade

Rename the embedded view location provider

dd12549

Rename the embedded fixture

ed87fa7

A minor rewrite of some parts of the embedded view location provider

1194ab9

Removed failing tests. They're not needed anymore since we don't thro…

96327db

…w exceptions if there's only one view found

Add tests for new root namespace behavior

762ff07

Format embedded tests fixture

140b3dc

Deliver a view at the root for the embedded demo

5b2d9b4

This closes NancyFx#732 and closes NancyFx#1332.

khellang reviewed Dec 5, 2013
View reviewed changes

jrsconfitto added 3 commits December 5, 2013 13:43

Strike that, reverse it

1429786

Use registrations instead of default constructor

971e68a

Use IoC for great good!

Register types not instances

c576482

jrsconfitto added 3 commits February 6, 2014 10:55

Fix the projects and tests after previous reversion

66ac070

Merge remote-tracking branch 'nancy/master' into embedded-improvements

c9fa9fe

jrsconfitto closed this Mar 3, 2014

jrsconfitto added a commit to jrsconfitto/Nancy.EmbeddedContent that referenced this pull request Mar 4, 2014

Initial commit

ab805bd

This code is branching off the work done in NancyFx/Nancy and the work in NancyFx/Nancy#1331.

jrsconfitto deleted the embedded-improvements branch March 17, 2014 17:45

reidperyam mentioned this pull request Jun 28, 2014

Create Nuget for working with embedded stuff #732

Closed

jrsconfitto mentioned this pull request Jul 14, 2014

Rework Nancy Embedded content #1608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve support for embedded resources and NuGet it up #1331

Improve support for embedded resources and NuGet it up #1331

jrsconfitto commented Nov 19, 2013

jrsconfitto commented Nov 19, 2013

jrsconfitto commented Nov 19, 2013

jrsconfitto commented Nov 20, 2013

reidperyam commented Nov 21, 2013

jrsconfitto commented Nov 21, 2013

reidperyam commented Nov 21, 2013

jrsconfitto commented Nov 21, 2013

khellang commented Nov 22, 2013

khellang commented Nov 22, 2013

jrsconfitto commented Nov 22, 2013

jrsconfitto commented Nov 22, 2013

jrsconfitto commented Nov 25, 2013

jrsconfitto commented Nov 25, 2013

jrsconfitto commented Dec 4, 2013

jrsconfitto commented Dec 4, 2013

khellang commented Dec 5, 2013

khellang Dec 5, 2013

jrsconfitto Dec 5, 2013

jrsconfitto commented Dec 5, 2013

reidperyam commented Dec 6, 2013

jrsconfitto commented Dec 9, 2013

jrsconfitto commented Dec 11, 2013

jrsconfitto commented Jan 30, 2014

khellang commented Jan 30, 2014

reidperyam commented Jan 30, 2014

khellang commented Jan 30, 2014

xt0rted commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Mar 3, 2014

jrsconfitto commented Mar 4, 2014

Improve support for embedded resources and NuGet it up #1331

Improve support for embedded resources and NuGet it up #1331

Conversation

jrsconfitto commented Nov 19, 2013

Questions i have

Done

jrsconfitto commented Nov 19, 2013

jrsconfitto commented Nov 19, 2013

jrsconfitto commented Nov 20, 2013

reidperyam commented Nov 21, 2013

jrsconfitto commented Nov 21, 2013

reidperyam commented Nov 21, 2013

jrsconfitto commented Nov 21, 2013

khellang commented Nov 22, 2013

khellang commented Nov 22, 2013

jrsconfitto commented Nov 22, 2013

jrsconfitto commented Nov 22, 2013

jrsconfitto commented Nov 25, 2013

jrsconfitto commented Nov 25, 2013

jrsconfitto commented Dec 4, 2013

jrsconfitto commented Dec 4, 2013

khellang commented Dec 5, 2013

khellang Dec 5, 2013

Choose a reason for hiding this comment

jrsconfitto Dec 5, 2013

Choose a reason for hiding this comment

jrsconfitto commented Dec 5, 2013

reidperyam commented Dec 6, 2013

jrsconfitto commented Dec 9, 2013

jrsconfitto commented Dec 11, 2013

jrsconfitto commented Jan 30, 2014

khellang commented Jan 30, 2014

reidperyam commented Jan 30, 2014

khellang commented Jan 30, 2014

xt0rted commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Jan 30, 2014

jrsconfitto commented Mar 3, 2014

jrsconfitto commented Mar 4, 2014