Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Facility aspnet.systemweb #350

Closed
wants to merge 1 commit into from
Closed

Facility aspnet.systemweb #350

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 7, 2017

We need to close #283.

I will move on to OWIN next.

@ghost ghost changed the title Facilities systemweb Facility aspnet.systemweb Oct 7, 2017
- Removed build symbol from README.md

- Removed build symbol for SYSTEM_WEB

- Renamed system web namespace to include product 'AspNet'

- Made build pass and updated appveyor deployments

- Added some doco's ...

- Updated error report to include the new assembly

- Added missing package reference to Microsoft.Web.Infrastructure

- Added place holder for system web facility, just a copy of the logging facility docs

- Added link for system web facility

- Added link for system web facility

- Added some more notes to docs

- Removed per web request from lifestyles docs

- Updating installer documentation to not use per web request

- Made sure the generic implementation matching strategy does not reference per web request

- Adding CHANGELOG.md entry before I forget

- Added comment for better testing idea

- Bringing documentation into the solution

- Found more gubbins that needed moving

- A little more shifting

- Small refactor

- Completed the tests

- Ported testfixtures across to new test project, have the old test project compiling

- Moved all the code over, and starting to port the tests

- Started moving the microkernel lifestyles across for web requests

- Generate project stubs for facility
@ghost ghost requested a review from jonorossi October 7, 2017 23:55
@@ -1,4 +1,4 @@
// Copyright 2004-2012 Castle Project - http://www.castleproject.org/
// Copyright 2004-2011 Castle Project - http://www.castleproject.org/
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 2017...

/// Returns current request's scope and detaches it from the request context.
/// Does not throw if scope or context not present. To be used for disposing of the context.
/// </summary>
/// <returns></returns>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really hate XML

@@ -36,7 +36,6 @@ Symbol | .NET 4.5 | .NET Standard
`FEATURE_SECURITY_PERMISSIONS` | :white_check_mark: | :no_entry_sign:
`FEATURE_SERIALIZATION` | :white_check_mark: | :no_entry_sign:
`FEATURE_SYSTEM_CONFIGURATION` | :white_check_mark: | :no_entry_sign:
`FEATURE_SYSTEM_WEB` | :white_check_mark: | :no_entry_sign:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you are ok with this?

@@ -34,7 +34,7 @@
</PropertyGroup>

<PropertyGroup Condition="'$(TargetFramework)'=='net45'">
<DefineConstants>$(DefineConstants);FEATURE_PERFCOUNTERS;FEATURE_GAC;FEATURE_ISUPPORTINITIALIZE;FEATURE_REMOTING;FEATURE_SECURITY_PERMISSIONS;FEATURE_SYSTEM_WEB;FEATURE_SYSTEM_CONFIGURATION;FEATURE_WINFORMS;FEATURE_SERIALIZATION;FEATURE_URIMEMBERS;FEATURE_GETCALLINGASSEMBLY;FEATURE_APPDOMAIN;FEATURE_CODEDOM;FEATURE_ASSEMBLIES;FEATURE_REFLECTION_METHODBODY;CASTLE_SERVICES_LOGGING;FEATURE_EVENTLOG</DefineConstants>
<DefineConstants>$(DefineConstants);FEATURE_PERFCOUNTERS;FEATURE_GAC;FEATURE_ISUPPORTINITIALIZE;FEATURE_REMOTING;FEATURE_SECURITY_PERMISSIONS;FEATURE_SYSTEM_CONFIGURATION;FEATURE_WINFORMS;FEATURE_SERIALIZATION;FEATURE_URIMEMBERS;FEATURE_GETCALLINGASSEMBLY;FEATURE_APPDOMAIN;FEATURE_CODEDOM;FEATURE_ASSEMBLIES;FEATURE_REFLECTION_METHODBODY;CASTLE_SERVICES_LOGGING;FEATURE_EVENTLOG</DefineConstants>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was kind of hoping we could get rid of this.

@@ -7,7 +7,7 @@ Windsor supports so called *open generic components*, that is components that ar
For example:

```csharp
Container.Register(Component.For(typeof(IRepository<>)).ImplementedBy(typeof(MyRepository<>)).LifestylePerWebRequest());
Container.Register(Component.For(typeof(IRepository<>)).ImplementedBy(typeof(MyRepository<>)).LifestyleTransient());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web Requests really did not need to be tied to generics

@@ -28,7 +28,7 @@ public class MyRepository<T>: IRepository<T>, IRepository
}

// registration
Container.Register(Component.For(typeof(IRepository<>)).Forward<IRepository>().ImplementedBy(typeof(MyRepository<>)).LifestylePerWebRequest());
Container.Register(Component.For(typeof(IRepository<>)).Forward<IRepository>().ImplementedBy(typeof(MyRepository<>)).LifestyleTransient());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@@ -91,5 +91,5 @@ To instruct Windsor to use your implementation of `IGenericImplementationMatchin
Container.Register(Component.For(typeof(IRepository<>))
.Forward<IRepository>()
.ImplementedBy(typeof(MyRepository<>), new RepositoryGenericCloser())
.LifestylePerWebRequest());
.LifestyleTransient());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again. Generics and Web Requests ... not a thing.

@@ -38,16 +38,6 @@ Container.Register(Component.For<MyTransientComponent>().LifestyleTransient());

:information_source: **What transients are good for:** Transient lifestyle is a good choice when you want to be in control of instance's lifetime. When you need new instance, with new state every time. Also transient components don't need to be thread safe, unless you explicitly use them in multi-threaded situations. In most applications you'll find that a large percentage of your components will end up as transient.

### PerWebRequest
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a thing so I removed it from the main docs concerning lifestyles. We should be using scopes.

@@ -16,7 +16,7 @@ That's however as far as we can get for what we want from `ISession`. When it's

### The `PerWebRequest` lifestyle

To change the `ISession` lifestyle to be per web request, we need to specify that in the registration. So we need to change it to the following:
Please install the `Castle.AspNet.SystemWebFacility` NuGet first. To change the `ISession` lifestyle to be per web request, we need to specify that in the registration. So we need to change it to the following:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do believe this documentation is super dated. I did the minimum here, but after changing this I think there is a bit of work that needs to happen here to clear out the cruft.

using NUnit.Framework;

[TestFixture]
public class PerWebRequestTestCase
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boy scout rule, leave the camp cleaner then what you found it.

bool wasReleased = false;
object releasedService = null;

// Would love to create a testing facility for this which we could 'dog food' to clean up our test library in Windsor
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not just me, I think other people are trying to discover this. We should give them a way of doing it. I personally would love to clean up our tests.

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)]
public sealed class PerWebRequestAttribute : LifestyleAttribute
public sealed class PerWebRequestAttribute : ScopedAttribute
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very important. It fits in with the spirit of where the previous commits were going.

@ghost
Copy link
Author

ghost commented Oct 8, 2017

Closing this out, I have 2 more facilities. One that supports OWIN for web api self host.

@ghost ghost closed this Oct 8, 2017
@ghost
Copy link
Author

ghost commented Oct 8, 2017

Please see #351 instead.

@ghost ghost deleted the facilities-systemweb branch June 22, 2018 00:23
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OWIN-compatible implementation for "PerWebRequest" lifestyle
0 participants