search

castleproject / Windsor

Castle Windsor is a best of breed, mature Inversion of Control container available for .NET
http://www.castleproject.org
Apache License 2.0
1.52k stars 455 forks source link

Discussion on deprecated APIs for removal in future versions #338

Open jonorossi opened 7 years ago

jonorossi commented 7 years ago

The community needs to discuss what obsolete public public API's can be removed for Windsor V5, including anything that might no longer be useful or could be replaced with something from the framework.

Anything that is identified as "generally agreed" will be dealt with in a PR which should be linked here.

Below are a list of obsolescent items found and their current status.

ghost commented 7 years ago

I do believe most of the XML configuration can be removed. I think XML removes the compiler from type safety resolution and promotes these problems to the runtime environment. So I would think any XML based config needs to hit the road and go to that XML heaven in the sky.

ghost commented 7 years ago

It also seems like we should get rid of parent/child containers.

https://github.com/castleproject/Windsor/issues/324 https://github.com/castleproject/Windsor/issues/338 https://github.com/castleproject/Windsor/issues/115 https://github.com/castleproject/Windsor/issues/66

I think what you need to do is read @hammett's original post about this:

http://hammett.castleproject.org/?p=296

@kkozmic tried to reason with this implementation but I do believe it creates more problems than what the micokernel already solves:

We should really be using:

ghost commented 7 years ago

I hope that we can treat this as an opportunity to discuss things that should not be here apart from mere obsolescence, parent/child containers appears to be a big after trawling through all the old issues tonight. This is steering users down the wrong path with the wrong expectations. A major version increment is hopefully where we get to discuss things like this and make things easier for people. It is my hope that there is more that people would like see more come out of this that makes Windsor easier to use.

jonorossi commented 7 years ago

parent/child containers appears to be a big after trawling through all the old issues tonight

Yes as you've discovered they are a source of many problems because the rules around them still aren't defined. Reading the comments on @kkozmic blog posts does make me realise that today child containers are really the only solution for some people's requirements. A handler selector doesn't let you register different components without having them always registered, a sub dependency resolver would be a painful way to perform this task, it means if you were doing WinForms (or any long lived app) your root container has common stuff and as you enter a specific module (doesn't even need to be a plugin) of the application its child container gets created for use and destroyed on closing, you couldn't really do that without child containers.

Having a look at Autofac they don't appear to have child containers but creating an Autofac lifetime scope does provide an inner container builder where you can register more types that don't infect the parent scope.

I think we could probably do the same and allow the scoped lifestyle to support registrations, singletons would survive in the container, scoped lifestyle components would be released on scope disposal as they are now, transient is obvious, and all other lifestyles would behave like they were resolved outside the scope. Thinking through many of the requirements in those blog comments this would handle most cases.

Windsor has been used in long lived programs (GUI and server) for a long time and will continue to be, web apps are only one use case. What we really need is for users to get involved so we can learn about people's use cases.

reijerh commented 7 years ago

From that list, we use BasedOn<T> and Where, e.g: Classes.FromAssembly(assembly).BasedOn<IPlugin>().WithServiceBase() and Classes.FromAssemblyNamed("foo").Where(t => t.Name.EndsWith("Repository")).WithServiceAllInterfaces()

I'm a little confused by the deprecation messages on those to be honest, perhaps we've been using them wrong? Are there alternatives?

Now regarding child containers: we're using them in an application that has a plugin style architecture, where each plugin can create and attach its own child container to the main container. A plugin represents a module, and whenever a module is opened, the child container is created and the plugin registers its own components (possibly overriding some main container's defaults). When the module is closed by the user, the child container is disposed and there should be no trace left of any of the module's components.

This works quite nicely as the plugins can re-use defaults from the main container for e.g. UI stuff, but they can modify behavior at the same time.

Regarding #324: this 'bug' is only important to me because we currently have to manually Resolve() some components to accommodate the use of WPF ViewModels in old and new code, and it's not always known when these components should be Released() again. So relying on the module container to dispose stuff was very convenient. This is just the way things go with a huge codebase where 75% is legacy and you have to combine different paradigms. As an alternative I tried to use the scoped lifestyle, but it didn't seem to work correctly when I tried to create a module-lifetime scope using it, probably caused by async/await. I looked into it, and it seems the mechanism it uses (some sort of calling context thingy) doesn't work correctly for async/await scenarios. I rolled my own module-scoped-lifestyle for now.

One other thing that caused some confusion: resolving transients using Resolve() inside a singleton's constructor will stick the resolved transients to the singleton forever, meaning calling Release() for these manually resolved transients later on doesn't actually release them from memory. This can result in memory leaks that are insanely hard to figure out, there's just a couple of posts on the internet that document this, if you know what to look for.

ghost commented 7 years ago

@reijerh

The members you mention are not deprecated and look like they will live on in the FromDescriptor. The BasedOn methods here are in the BasedOnDescriptor to stop the fluent interface from being re-entrant and resetting. I think you might be OK.

@reijerh & @jonorossi

Parent/Child containers look like they need proper rules hashed out. Coming up with a language that could concisely define this would not be an easy task. We should try lead up to it by discussing it more. I have some additional thoughts around singletons. For example, if it is registered in the parent container and already resolved then it should stand to reason they would survive the disposal of the child container however if they are registered in the child container then surely it would make sense that it is not resolvable via the parent and would not survive the child containers disposal. This fundamentally alters how a singleton would behave. Furthermore disposables that are transient's and then consumed by singleton's should probably be disallowed altogether in parent containers, in child containers the singleton would be tracked and are assumed to be shorter lived so this would potentially be viable as the child container would eventually get disposed. Our rules need to factor in where the component was registered, what the intended lifestyle was and how it behaves or is tracked in a child container and then what happens to it when the container is disposed. No easy task! :)

The divergent side of my brain is thinking do we even need this? What we are really after is a way creating shorter lived containers that serve as plugins with re-usable setups. I am a massive fan of using installers. These allow you to create and share common container setups and common installers can be consumed by specialised installers for child scenarios. Simply creating a new container with distinctions between how it is used and how installers are composed together might actually be a simpler solution to the problem. This also avoids sending us down the path of trying to support mutated lifestyles. Much easier!

I still believe we should get rid of this feature, I can solve it by using composition at the installer level.

jonorossi commented 7 years ago

What we are really after is a way creating shorter lived containers that serve as plugins with re-usable setups.

Usage of child containers It rarely about reusing container set up but having access to parent container instances.

ghost commented 7 years ago

Usage of child containers It rarely about reusing container set up but having access to parent container instances.

Still struggling to understand why this is a "must have" feature.

Parent/Child Scenario

ParentForm <- Registered in parent container, instantiated. Click resolves through N possible ChildForm's.

ChildForm <- Registered in child container, to be instantiated but requires a reference to ParentForm.

ParentIWindsorInstaller -> Holds reference to parent container. Preferably protected static named ParentContainer.

ChildIWindsorInstaller -> Inherits ParentIWindsorInstaller and then will have access to both ParentContainer/ChildContainer.

Then add a custom SubDependencyResolver to the child that checks resolution between the parent and child containers because you already have a reference to both.

Might be missing something obvious here but this is potentially how I would do it.

jonorossi commented 7 years ago

Then add a custom SubDependencyResolver to the child that checks resolution between the parent and child containers because you already have a reference to both.

Wouldn't you run into even more lifetime problems doing it that way?

It would be good to hear some more detail from @reijerh how their application works, what type of services they register in the child container.

ghost commented 7 years ago

Wouldn't you run into even more lifetime problems doing it that way?

I am thinking less problems because lifestyles are more deterministic and don't have to mutate their behaviour across containers with blended release policies for tracked items as I mentioned earlier. I am happy to be wrong on this. Just thinking out loud really.

It would be good to hear some more detail from @reijerh how their application works, what type of services they register in the child container.

Yes, I would love to know more and even what I am proposing is possible.

hconceicao commented 7 years ago

Parent/Child containers are used by other framework as a container integration thing. I remember NServiceBus using/doing something like that. But it is a very naive and limited implementation, definitely needs some love.

Imo the whole component burden thing should be removed. Everytime people discover that I was a castle contributor they tell me a war story about a production memory leak caused by it. In the end people set the NoTrackingReleasePolicy and move on not caring about it.

The xml config stuff I thinl that is very usefull for store some app properties. I usually have all the have lifting done by the fluent api, and leave some settings be setup on xml.

And not sure if its the right place for it, but one thing that hammett and I discussed in the last years, is that the openness for modification is maybe a bad thing. Most of people configure the container at the startup, and never changes it. But this assumption (that it can be modified at any time in the app lifetime) feeds a lot of simple mistakes (that could be detected with simple checks).

reijerh commented 7 years ago

I went through each module's installer, and as far as I can tell most registrations could just as well be done in the main container. The only argument I could think of against doing this is that it 'pollutes' the main container for as long as the app lives, even though the module was already closed.

There is one thing that I think would be tricky to do without child containers though: overriding implementations. As an example, we have an IProgressReporter with a default implementation that shows the progress as a popup to the user: PopupProgressReporter. This one is registrered with IsFallback().

Some more modern modules have adapted a different way of showing progress due to a new UI design, and so they implement their own IProgressReporter that visualizes progress in a different way. This ModernProgressReporter overrides the default while the user is in a certain modern module. The override is removed as soon as the user leaves the module.

Repositories and ViewModels are other examples of things we override in modules.

So I'm guessing this scenario might be implemented using some other Windsor extension point?

BTW, about burdens: I had to use managedExternally: true together with UsingFactoryMethod to make sure a transient wasn't pinned to the lifetime of a singleton in whose constructor it was resolved. Hard to find out and not very intuitive, maybe the whole burden thing needs rethinking. This is basically +1 what @hconceicao said.

codekaizen commented 7 years ago

doing WinForms (or any long lived app) your root container has common stuff and as you enter a specific module (doesn't even need to be a plugin) of the application its child container gets created for use and destroyed on closing, you couldn't really do that without child containers.

This is the main use case for us. Root parent container, then child container per "open document" which gets disposed when the document is closed. This pattern takes care of dependency lifetimes very nicely. We've tried using scoped lifetimes for this, but there are limitations in this mechanism which aren't present in parent/child containers, one which @reijerh mentioned: no override (which we use on some document types). We've also extended parent/child containers to handle this case more flexibly and robustly than vanilla Castle.

I'd be interested in helping work through lifetime scope issues to get a solid answer for v5.0.

ghost commented 7 years ago

This is the main use case for us. Root parent container, then child container per "open document" which gets disposed when the document is closed. This pattern takes care of dependency lifetimes very nicely.

@codekaizen - I am interested to know more about your lifestyle configuration here, can you shed more light on how these are setup? We just had a case with transient disposables registered in the parent were not being disposed when resolved through the child container and then disposing the child container in #324. @jonorossi has already said this breaks the resolve/release rule but burden tracking should perhaps be able to do this? @hconceicao has already stated that burden tracking is probably wrong and should be removed. Would be good to hear about a formula that works :)

ghost commented 7 years ago

If anyone is interested, I have an implementation of a plugin architecture which use a subdependency resolver and by completely avoiding the parent/child api's of windsor. It uses a child container per view approach and from what I can see things are releasing and disposing perfectly. Will warn you it is a little raw and only took 3.5 hours to implement.

https://github.com/fir3pho3nixx/Windsor.Plugins

Look forward to your feedback.

IanYates commented 7 years ago

@fir3pho3nixx - interesting.

My main app is a web app, but we're starting to have some things being done on the client-side on a Windows PC. Our bridge for that is registering a protocol handler of the form app://{http-server}/{clientAction}/{actionInstanceId}

Multiple {clientAction}s are implemented. We have a single EXE registered client-side. One example is to open a Word doc and prepare a mail merge, watch for the user to make their changes & close Word, then send the doc back to the server. Another example is to drive a local scanner.

Rather than launch a new instance of the EXE every time I'd like to make a long-lived but simple winforms/wpf app that sits in the taskbar. Each invocation from the server launches a stub exe which finds the running client app and gives it the command.

So... I'll have one root Windsor container in that app for things like registering a logger, a factory to give me a HttpClient instance and some other useful things. But each invocation - sometimes of the same action type - should have its own "container" scope much like you were describing your view models. I occasionally would need to override something that the parent container would normally provide. I'd also like to dispose of everything in the child container as a sort of 'unit of work' when the user action is done.

I suspect I might be able to do some of this with some sort of bound & scoped lifetimes but it's just something I've never quite got my head around as the child container approach seems more straightforward. The child container, as far as I'm concerned, can be a form of handle which defines the scope and lifetime. Registering things in there is the extra bit which your plugin thing fixes, as well as making the concept more simple.

Am I on the right track with my thinking?

I implemented a form of this with Windsor already but ran into trouble with, from memory, this scenario. I had a transient registered in the parent container. The transient took in a dependency. That dependency however wasn't available until I was actually invoking an action - so I'd explicitly register that dependency in the child container (in this case it was a specific instance of an object). I'd then try to resolve that parent-registered transient from the child container. If memory serves correctly, Windsor didn't like that.

ghost commented 7 years ago

@IanYates

Am I on the right track with my thinking?

Yes, the idea here is that things fallback to a vanilla or "well known" understanding of how lifestyles are managed in a single container and removes the complexity of who registered what where and how that cascades and mutates between child/parent resolves. It also leverages object tracking to the best of it's ability for transients/singletons. I have outlined my assumptions in this section.

I implemented a form of this with Windsor already but ran into trouble with, from memory, this scenario. I had a transient registered in the parent container. The transient took in a dependency. That dependency however wasn't available until I was actually invoking an action - so I'd explicitly register that dependency in the child container (in this case it was a specific instance of an object). I'd then try to resolve that parent-registered transient from the child container. If memory serves correctly, Windsor didn't like that.

My pattern "kind of" solves this scenario but I do have a few questions/assumptions to try and understand it better.

The problem:

Given: We want to resolve Transient A from child container.

ParentContainer -> Registers Transient A (requiring Dependency B, not registered) -> Handler.State == WaitingDependency. Therefore always potentially a misconfigured component in parent container.

ChildContainer -> Registers Dependency B -> Handler.State == Valid. Resolvable but cannot contribute to Transient A's constructor because this is in the parent container.

Questions:

In summary, if this is never resolved through the parent container it would benefit greatly from being pushed down the the child containers. You could also probably apply some clever polymorphism in your installers to make this happen by convention.

Resolving Transient A from the parent container is a completely different story, this would simply involve no overriding registrations in your child container.

jonnii commented 7 years ago

I think it's also worth deprecating EnvironmentInfo (https://github.com/Huddle/Castle.Windsor/blob/master/src/Castle.Windsor/IEnvironmentInfo.cs). It's rarely used (even though I have used it extensively in the past!) it's really no longer necessary.

ghost commented 7 years ago

I have updated this issue with some of the topics mentioned here. All marked as in flight. If you feel something should be added please ping me and I will add it.

ghost commented 7 years ago

Linking discussion: https://github.com/castleproject/Windsor/issues/346 and adding it to the issue description.

Liwoj commented 6 years ago

@fir3pho3nixx

I do believe most of the XML configuration can be removed. I think XML removes the compiler from type safety resolution and promotes these problems to the runtime environment. So I would think any XML based config needs to hit the road and go to that XML heaven in the sky

Hi. Im Windsor's long time user and I strongly disagree with this. Fluent registration in code is cool and I use it for most things but XML config still has its use. Its invaluable for storing various setting for components - things which are so low level I don't want to put it into app\web config but still have some way to change it without building and redeploying my app. It can do things app.config cant as arrays etc. Also ability to include another xml file is great (for sharing some settings between different apps for example)

kenegozi commented 6 years ago

TL;DR; I agree with Michal's sentiment, but believe there are other ways to address this without holding Windsor back.

IIRC app.config allow including another files.

Application's configurable knobs (and many other concerns) can be more easily, and/or more efficiently solved, by systems that are not the container's configuration. Even being a (now emeritus) contributor to Castle and Windsor, I'd always try to minimize vendor lock in (even if the vendor is myself). If I need to switch a container (or forgo it altogether) I'd still need environment aware, dynamically loaded configurations. True to a Unix-y (or simply solid) design principle, switching the container should have minimal to no effect on configuration management.

For example and specifically for application's config knobs, I had always avoided relying on xml or app config directly. Main reason was the ability to source configs from other places (like env variables, Azure/Heroku configs, etc). The common form for those is Pair<String,String> so I'd make simple wrappers that expect the value to be directly parseable, or json-deserializeable (e.g. key->String, key->3, key->["array","of","things"]. On top of that, I'd have a handler to allow dynamically changing config values. Nothing here has anything to do with dependency injection, lifecycle management, and other benefits of using a Container.

Ken.

Ken Egozi. http://kenegozi.com/ http://kenegozi.com/blog

On Fri, Mar 2, 2018 at 10:28 AM, Michal Levý notifications@github.com wrote:

@fir3pho3nixx https://github.com/fir3pho3nixx

I do believe most of the XML configuration can be removed. I think XML removes the compiler from type safety resolution and promotes these problems to the runtime environment. So I would think any XML based config needs to hit the road and go to that XML heaven in the sky

Hi. Im Windsor's long time user and I strongly disagree with this. Fluent registration in code is cool and I use it for most things but XML config still has its use. Its invaluable for storing various setting for components - things which are so low level I don't want to put it into app\web config but still have some way to change it without building and redeploying my app. It can do things app.config cant as arrays etc. Also ability to include another xml file is great (for sharing some settings between different apps for example)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/castleproject/Windsor/issues/338#issuecomment-370010252, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAQbrmt-z5LEPAOb0WOeQ6z4PEDqzk9ks5taY9XgaJpZM4PaKU6 .

ghost commented 6 years ago

@Liwoj

The XML config is littered with type definitions, this to me is code. It has no business being composed in XML files.

ghost commented 6 years ago

@Liwoj

You are trading out compiler safety in statically typed language for a run-time exception. I am not sure this the right way to go.

Liwoj commented 6 years ago

@fir3pho3nixx

The XML config is littered with type definitions, this to me is code. It has no business being composed in XML files.

You can use XML just for <parameters> together with component id and do everything else in code. No types in xml.

You are trading out compiler safety in statically typed language for a run-time exception. I am not sure this the right way to go.

Yep but this is the case with any external configuration be it xml, JSON or any other store. Its trade-of between safety (compile time checking) and flexibility (ability to change behavior without compilation and deployment). You can say something similar about reflection and no one argues about the need to remove it from .NET.

@kenegozi Maybe your app needs to change configuration in run-time. Some other app doesn't. Or you are afraid of vendor lock. For me avoiding vendor lock sometimes means breaking DRY and "reinventing the wheel". I don't see the point in cluttering my components (or creating wrappers) with code for parsing\converting configuration values to it's run-time representation.

You've mentioned life-cycle management as one of the container's responsibilities. Well, creation of components and setting up dependencies is part of the life-cycle. In my book, support for setting up dependencies in external configuration, support for powerful type conversions (+ ability to provide your own type converter if needed) is infrastructure concern. Its addressed very well in Windsor and makes my life so much easier because its easy, powerful and "just works". In the past when I was comparing DI implementations in net, it was one of the selling points of Windsor for me and its removal would be huge step back imho

Is there any discussion here about how exactly this part of Windsor is "holding it back"? Or are we just comparing our "holistic views" on how all the apps should be developed ?

jonorossi commented 6 years ago

@Liwoj this discussion is exactly that, a discussion to gauge what sort of stuff we could get rid of because Windsor is huge with so few people contributing to maintaining it, so thanks for joining this discussion as it wasn't specially about XML configuration or any one thing but the fact that there are obsolete things all over the place. @fir3pho3nixx edited the issue description by collating a list from the comments that makes this issue look more like a todo list than just a discussion thread, but he has also included "Not a thing for v5" against "XML config in general".

Between us there are mixed opinions on XML config, but I believe XML config still has a use case (including heaps of history usage that isn't just going to magically rewrite itself) exactly as you describe and I suspect if we do anything in the medium term it'll be to de-emphasise XML config in our docs to encourage usage only when C# can't be used. We also have work to do to make sure all facilities can be configured easily by code, even the just recently resurrected Quartz.NET facility heavily uses XML config and doesn't have a C# interface (other than building the XML configuration object) to configure it, like some other facilities.

@Liwoj I highly recommend watching the Windsor repo to get notifications, it is low volume and we need more users involved in discussions on how to move forward, what should change, what should stay the same, even if you don't have time to contribute code. 9 participants on this thread, surely there are more than 9 Windsor users out there.

jonorossi commented 6 years ago

I should add that I don't see XML config as holding Windsor back like a bunch of other areas where things are sort of broken and will needing breaking changes to fix, areas where users are running into defect after defect like factory facility and child containers.

In regards to XML config, what we really need to establish first is an alternative to XML config at minimum via samples showing how users can do that before considering deprecating it. I feel that removing XML config would cut Windsor at it's knees for many users since it is such a long standing core feature from day one, these users will struggle to migrate big apps and likely just stay with an old version of the library and then never contribute. I agree with @kenegozi that environment variables are the way most applications are configured today, not a big XML document (even though it can resolve environment variables).

Most of the other features we've been removing have been long marked obsolete or thought of that way for a long time, so I think we should give users the courtesy of some time if this is to happen here, i.e. deprecate, then later remove.

kenegozi commented 6 years ago

@jonorossi : Indeed, by holding back I refer to the added complexity of maintaining an ever growing matrix of features that need to play nicely together. Even in a 100% funded project it is not an easy task, let alone volunteer run OSS.

And removing the support for such a feature is definitely a breaking change. Question is how many people how are users of XML config are also likely to upgrade to a possible no-xml-anymore vNext version of Windsor.

There could still be made a layer by which a library, external to Windsor's core, can read the XML config and call windsor's well published registration APIs. This would inverse the coupling, and declare programmatic configuration as core and XML configuration as wrapper.

@Liwoj Can't agree more. Lifecycle management and general extensibility is what sets Windsor apart from other .NET containers. What I argue is that bundling general-purpose config into the container is mixing up concerns where they should not belong. Don't reinvent the wheel, pick up a library for reading configuration if you must. However taking a dependency on something is exactly that - for the benefit of reducing duplication you create an unneeded dependency. DRY is not gospel, in my book it says "Do Repeat Yourself unless there's a very good reason not to :)". Passing a string to JSON.Deserialize is hardly a complicated error prone logic that worth re-using.

ghost commented 6 years ago

@jonorossi - sorry I edited the issue, was trying to create a quick reference for agreed items. Avoiding having to read through the entire issue.

An example where xml config has been used inappropriately is in type factories. Component registrations plug things in, only so interceptors can reference it later. Not sure this is a good thing.

jonorossi commented 6 years ago

There could still be made a layer by which a library, external to Windsor's core, can read the XML config and call windsor's well published registration APIs. This would inverse the coupling, and declare programmatic configuration as core and XML configuration as wrapper.

@kenegozi exactly, we first need to make sure everything we are supporting actually has a programmatic interface so we can flip the direction. I guess since external configuration was one of the ticket features of Windsor over MicroKernel it has really driven a lot of the past design decisions.

I'm definitely not against moving away from XML config, but in the context of this issue I don't think version 5 is the place as we aren't at the point. Windsor's external configuration is always more powerful than most people realise at first, there are a lot of extended features (conditionals, include, etc) in there so not always just a quick change to C#.

What do you guys think of a Windsor external configuration compat library (another assembly/package we maintain) that would force use of the public API to perform registration? Most/all of Castle.Windsor.Configuration including XmlInterpreter would move there and the internals of the container couldn't reference this assembly as the dependency is the other way around. I think it would be the safest way to move forward to reduce size of the core of Windsor while trying to keep each major version not so big of a jump. I'm not sure yet how this would work for facilities outside Windsor.dll.

sorry I edited the issue, was trying to create a quick reference for agreed items. Avoiding having to read through the entire issue.

@fir3pho3nixx no problem, I was just pointing out this isn't a formal todo list even if it appeared a little like that.

ghost commented 6 years ago

@jonorossi

How much of this do we want to action before close?

AllTypes/AllTypesOf has been addressed. https://github.com/castleproject/Windsor/blob/master/CHANGELOG.md

I would still like to address: https://github.com/castleproject/Windsor/issues/338#issuecomment-331751143.

ghost commented 6 years ago

Sorry @hconceicao and @jnm2.

I think you guys raised some hot issues. You are not being excluded, I personally feel your pain because I run this container in production everywhere I go. We just don't have the time to build this into V5.

Please raise new issues for anything we missed here. Thank you for your input. Kindest respect.

jonorossi commented 6 years ago

How much of this do we want to action before close?

Since this is just a discussion thread rather than a specific action item, maybe we just rename the title to remove v5, there is still quite a lot of info we've discussed that would be good to capture in proper separate issues in the short to medium term.

jnm2 commented 6 years ago

@fir3pho3nixx Sure, no problem! The only issue I remember bringing up right now already has an open issue, https://github.com/castleproject/Windsor/issues/346.

ghost commented 6 years ago

Since this is just a discussion thread rather than a specific action item, maybe we just rename the title to remove v5, there is still quite a lot of info we've discussed that would be good to capture in proper separate issues in the short to medium term.

I am also cool with that.

thuannguy commented 5 years ago

Hi. Im Windsor's long time user and I strongly disagree with this. Fluent registration in code is cool and I use it for most things but XML config still has its use. Its invaluable for storing various setting for components - things which are so low level I don't want to put it into app\web config but still have some way to change it without building and redeploying my app. It can do things app.config cant as arrays etc. Also ability to include another xml file is great (for sharing some settings between different apps for example)

I second this. My solution uses a mix of fluent registration in code and XML registration. Fluent registration using convention is cool because I can add implement new services and components and they are registered automatically for me. However, XML registration is cool too. The ability to easily register items of an array in the specific order is useful. In addition, my customers appreciate the fact that they can plug in some custom implementations themselves: "hey, you need to implement this interface and then you 'just' need to add it to that xml file!"

Btw, my solution also uses child containers because it needs to register components that vary on a request by request basis.

jonnii commented 5 years ago

We have also relied on xml configuration for a long time as a means to configure our applications at deployment time but we have migrated to a very simple json based config as a transition towards using the asp.net core config stuff. Some of these may be subjective but our reasons for doing so are:

  1. xml config is, relatively, more verbose and harder to read
  2. xml config is hard to validate at run time, you need to rely on windsor to give you a meaningful error message
  3. Our tooling (octopus deploy) has native support for json
  4. The future of .net config is json based
  5. Windsor xml config is too permissive to exposure to a user

I'm in favor of deprecating all of the xml config stuff because moving to a custom json config was really easy, gave us a lot more flexibility and reduced the number of dimensions for app configuration to only those that we wanted to support. My preference would be an easy integration path for asp.net core json config so you can get all the other great benefits of using that such as easy environment variable support.

thuannguy commented 5 years ago

For god's sake, please keep XML config support. It IS working well and being permissive is a great thing for advanced users. Some can favor json-based, but that doesn't mean json is anything better than XML. The greatest thing about Windsor is that it is very matured with so many features, and that also means it has a large user bases, some of them are enterprise applications that heavily use every features that Windsor can offer. Before saying that moving from XML to JSON is easy, please consider the fact that it might not always be easy for everyone, depending on how advanced one uses XML configuration. "Really easy" might mean hundreds of hours of coding and testing. While I undertand the hidden desire of all developers to catch up with what is trending, I'd argue that it is not always a good thing.

Today I tried to upgrade my application to a latest release and found out that LifestyleType.PerWebRequest was removed (Yup, I'm a bit late to the party, but my case could represent what some developers of enterprise applications have to do.) The lifestyle was removed and it is recommended that LifeStyle.PerWebRequest() should be used. What doesn't work for me is that my code can run in two different hosting environments, and one uses Transient/Singleton while the other uses PerWebRequest. Thus, I passed a LifestyleType to the installer which is obviously broken now because the PerWebRequest lifestyle disappeared. Maybe the one you removed it thought changing to PerWebRequest() could be easy?

`

public class ControllersWindsorInstaller : IWindsorInstaller
{
    private readonly LifestyleType lifestyle;

    public ControllersWindsorInstaller(LifestyleType lifestyle)
    {        
        this.lifestyle = lifestyle;
    }

    public void Install(IWindsorContainer container, IConfigurationStore store)
    {
            container.Register(Classes
                .FromAssembly(plugInAssembly)
                .BasedOn<Controller>()
                .Configure(reg => reg
                    .LifeStyle.Is(lifestyle);
    }
}

`

jonorossi commented 5 years ago

@thuannguy Firstly this issue is just a discussion to get an idea of where users sit, it is literally tagged and titled discussion, couldn't be any clearer. Windsor might have a lot of features but who maintains all those features as platforms/runtimes change as the years go by. The discussion around XML config was heading towards having it as an optional add on in the way the MicroKernel was extended with XML config to create Windsor (they used to be 2 separate projects many years ago), not being removed completely. Windsor has been built by volunteer contributors, and it is severely lacking them in recent years.

Regarding LifestyleType.PerWebRequest, it has been discussed heaps before removal (where you were free to provide input) and the changelog discusses this breaking change for System.Web functionality. There are so many ways to do web applications today that it no longer made sense for Windsor to have web request functionality baked into main library, and the implementation changed from its own lifestyle to building on top of Windor's scoped lifestyle. It should be a fairly trivial to work around this change, you could change your installer to look for Custom or Scoped lifestyle enum member and set the lifestyle based on that instead, or change your installer to accept a boolean if in scoped "web mode". One of the biggest drivers of the change was to remove the need for code to care about "web mode" as it uses scoped lifestyle instead. Windsor sometimes needs to move forward even if it sometimes requires breaking changes in major versions, that is software, if you don't want change then don't upgrade.

thuannguy commented 5 years ago

@jonorossi I'm all in for making it optional, as long as existing XML features will still be working well. Some of the comments suggested to deprecate it which is a dangerous move from my point of view. Therefore, I had to raise my concern.

Regarding LifestyleType.PerWebRequest, yep, as you said it is the reason I felt the need to provide input here because I don't want the same thing happens for such an important and powerful feature. My example is all about things might not be so easy for other people.

While I can understand the problem of lacking of contributors. I'm not here to criticize anyone because my cheese was moved! Instead, I'm grateful to you guys for maintaining the library that has benefited me a lot. So thank you for that 😄

I don't buy the "if you don't want change then don't upgrade" advice. Changes are one thing, being left behind because of a road-blocker change is another thing.

jonorossi commented 5 years ago

@thuannguy I'd strongly recommend watching the Windsor repo on GitHub and participating in discussions including reviewing pull requests to ensure you like the direction of the project. It is an extremely low traffic project at the moment and the only way to make sure you voice is heard is to be involved.

thuannguy commented 5 years ago

@jonorossi I subscribed to the watch list as you recommended. I will try to get involved as much as possible.

phillip-haydon commented 5 years ago

With:

Parameters ServiceOverrides

Now removed, what are the alternatives? Or should I just stick to v4 for this legacy app?

jonorossi commented 5 years ago

@phillip-haydon those methods that were removed had all been marked obsolete with alternatives in the reason text for many years, DependsOn is generally the replacement. If the alternative mentioned isn't suitable for you open a new issue and we'll make sure you can upgrade, best not to get stuck with old versions.

See https://github.com/castleproject/Windsor/commit/3455b17538731d216d2dbcc8e81497d5cdba5948#diff-5a5f8f4974d2f9047452d0e470abc15a

phillip-haydon commented 5 years ago

@jonorossi - I know, I'm not complaining, just wanting to know what direction I need to go in. This is in a legacy app that's compiled and touched once every couple of months, someone decided that it would be a good idea to write a wrapper and a service locator around Windsor.

So today I dived in to do a dependency bump and came to realise the APIs were deprecated, so looked at some of the documentation.

https://github.com/castleproject/Windsor/blob/master/docs/registering-components-one-by-one.md#supplying-the-component-for-a-dependency-to-use-service-override

This section seems to be out of date. So I found this thread, and decided to ask :)

DependsOn is working fine though, I've moved from the old APIs to DependsOn and it works without issue.

Thanks.

jonorossi commented 5 years ago

@phillip-haydon good to hear you sorted it out, don't we all love legacy apps 😉. Don't worry I didn't think you were complaining, I agree the docs can sometimes be lacking, it looks like you found one that got missed, any chance for a pull request to fix that page.

phillip-haydon commented 5 years ago

Will sort a PR out tomorrow :D already turned computer off for the night.