Finishing The Catalyst Bread::Board Branch
As part of a long-term quest to migrate built-in features of Catalyst to independently managed components, the core team has been endeavoring to integrate Bread::Board in replacement of the existing service discovery code. The advantages are numerous, and include the fact that Bread::Board has a more extensive feature set, would allow better integration of code external to the web MVC and would reduce the amount of code the core team needs to personally maintain. It is in keeping with the core Perl and Catalyst ethic of creating broadly reusable code and choosing to integrate rather than roll our own, even if this means some increase in external dependencies.
Disadvantages so far include some difficulty in performing this ‘code swap’ and in determining how best to expose the improved feature set. Additionally, some compatibility hacks are onerous.
The goal of this document is to outline remaining the remaining tasks leading to ‘The Definition of Done’ for this project.
The following is a list of existing task that need to be completed before we can release a TRIAL version. This list is not necessarily exhaustive, but represents a best guess at this point in the project. It may be the case that additional tasks will emerge.
The current core Catalyst supports singleton style (one instance per application run) for components, which can be changed to some sort per request factor or per request instance via ACCEPT_CONTEXT and various code around this like Catalyst::Component::InstancePerContext or stuff in the Catalyst::Model::Adaptor code. We should support the idea of both singleton and per request lifecycles for components. We might want to think about the difference between ‘one per request’ and ‘one every time we ask’. I think that latter is already built into Bread::Board.
To complete this, we need a test case for PerRequest lifecycle and feel that we are able to replace what CMA:PerRequest and C::InstancePerContext does. We might have to write compatibility shims in CMA:PerRequest and C::IPC at some point as well, or at least mark them as not compatibility with Catalyst v6 and offer suggestions.
Sugar Syntax, or the Bread::Board DSL for Catalyst. In order to make it easier to use Bread::Board in Catalyst, we’ve written a DSL for it. However there are not a lot of good test cases in the suite to make sure we got the code correct. “we need singleton model depending on singleton model, perrequest model depending on singleton model, and perrequest model depending on perrequest model depending on singleton model.”
Also, it would be ideal to make sure that generic Bread::Board containers (stuff not inside Catalyst) can correctly be integrated via the DSL.
In current Catalyst you can call ->get_all_components to return a list of all singleton components (useful for stuff like the debug screen in debugging mode). In Bread::Board this is not the case anymore. There are two important places where this method is used. One is the mentioned debug output mode, this is important. There should be a method called ->get_all_catalyst_component_services which returns the services (not actual components) that we can introspect to build a new and better debug screen).
The second use of ->get_all_components is to power $c->components which t0m says should be “ref($c) ? $c->container->get_all_singleton_lifecycle_components : $c->container->get_all_components($c)”. Also, we need to deprecate calling $c->components as a way to add components (such as $c->components('Model::Foo' => $bar)). This should result in a deprecation warning, but I guess we should somehow make it work, at least for a time.
“t0m says the second use issue here is less important than other things”
For this to be considered done we need a new method called “->get_all_catalyst_component_services” which returns the service providers (not instances) with a test case. The debug table output code should use that new method. We also need a “get_all_singleton_lifecycle_components” and “get_all_components” methods to support $c->components, and test cases to cover that, as well as deprecating calling $c->components with arguments.
Note: “get_all_catalyst_component_services” is a list of the services, but “get_all_singleton_lifecycle_components” is actual component instances.
This would allow more careful use of the namespace
Having CI rely on application_name would make the work to split app/ctx harder.
The current name is ambiguous.
The following issues are things we can’t really solve until we finish existing issues. Some things here we may choose to leave undone for a TRIAL release.
In catalyst, a class is considered a component if it defines a ->COMPONENT method. It takes $c (i.e. $app), @args and should return an instance.
Many times people will use $c to call other components for the purposes of depending on them in a decoupled manner.
However with Bread::Board, this is no longer the best way for a component to depend on other components.
^^ This doesn't make any sense? You can't do this at the moment as load order is random..
Since there is nothing actionable for this issue it will be deferred in conversation for now, although it is likely that as long as we still support lookups like $c->model(…) things will work. We may wish to find out what if any important parts of the ecosystem are doing this so that we can update them.
The core issue is that model does its own funky stuff to insert model namespaces into the way component lookup is done. This is what allows one to say $c->model(“DBICSchema::User”) instead of $c->model(“DBICSchema”)->resultset(“User”). At this point this feature is highly engrained in the way people think about using DBIC with Catalyst (for better or worse) and making something like this work is going to be a requirement.
One possible solution is to detect if Catalyst is a new version using Bread Board and then use Bread::Board’s add additional component ability to mimic how the existing code currently works.
“t0m says, regarding backward compatibility with edgy code like this, we should defer to the end when we have a better idea of how everything will work”
In the current version of Catalyst, “->setup_components”, causes everything to be built, including the components themselves. However with Bread::Board it would be possible to defer building all the components, at least in an outside catalyst context. This could improve startup time (at the expense of a hit the first time a component is called).
Although we are deferred working on this for TRIAL, we need to answer the question if such a feature could be added in with a later point release without more breaking code or onerous hacks.
- Test cases for extending the container in an application.
- Using the sugar added in the previous item
- Test when Model::Foo depends_on Model::Bar
- Test for component Foo => ( lifecycle => 'Singleton', class => 'My::External::Class', dependencies => { config => depends_on("config") } )
- Fix ^^ so that you can get your component's namespaced config nicely.
- Tests for using the container outside of Catalyst
- Custom container which adds some (very simple) services which are initialized from the application config file (note plain services, not components)
- Depend on (and test) these inside Catalyst
- Test loading container outside Catalyst, and these services working
- Test Catalyst / MyApp is not loaded
There is a call in Catalyst::Controller to catalyst_component_name class accessor (which doesn't exist anymore). Not sure what the solution here is.
This is not an exhaustive list but these are things that are edgy enough that might might choose to ignore and just document as removed features, or things that have much better ways to solve now that we have Bread::Board. We will have to at the minimum document and communicate.
- expand_component_module
- $instance->expand_modules
- search_extra
- what to do when a user overrides the locate_components method?
This is breaking components which build other components (e.g. Model::DBIC::Schema).
We’ve also identified the follow success requirement that must be met before releasing a TRIAL
- Pass all existing core tests.
- Catalyst Model Adaptor and “Catalyst::ContextPerInstance” should be marked as obsolete. Are there other things on CPAN that this code would obsolete and what if any type of transition code or backcompat hacks should we supply?
- Someone should run through the existing Catalyst tutorial and make sure it still works.
- Essential parts of the greater Catalyst code ecosystem must pass all tests (such as the Sessioning code, DBIC integration code, etc. Possibly everything in Task::Catalyst should pass.
- We should also make sure example Catalyst applications on CPAN (such as Gitalist) also pass all tests and function as expected.
- We should identify an external company heavily invested in Catalyst who would be willing to test their website on the new code as we approach TRIAL status.
- As part of branding Catalyst version 6, we’d like to core “Catalyst::Plugin::Unicode::Encoding”, or something similar that is perhaps written to modern code standards. Is there anything else we want to core or rewrite and core?
- Documentation and advocacy. Should we build some interesting example code around the new feature set enabled by this conversion? Would that help people cope with the possible backward compatibility hits?
- If we are going to call this Catalyst version 6, which implies we are willing to take some backward compatibility hits, what other long pending deprecated features would we like to remove? In addition, are there any other big things we’d like to change, or mark as deprecated (and scheduled for removal in the version 7 timeframe?)
- How might this relate to cleaning up anything left over from the Moose migration or the PSGI migration?