Thats an excellent idea. I think there are still parts of this that could be made significantly easier by altering the SPI but I'll be happy to have them in utility jar for now. I'll go do this.
...ant On 11/9/06, Jeremy Boynes <[EMAIL PROTECTED]> wrote:
How about adding a scripting utility jar? This would be an additional jar that contained utility classes relevant to scripting extensions that they could all refer too, avoiding duplication but not putting too much implementation in the SPI itself (which I think we all agree should be avoided). -- Jeremy On Nov 8, 2006, at 4:04 PM, Jim Marino wrote: > > On Nov 8, 2006, at 2:08 PM, ant elder wrote: > >> This hasn't entirely turned out as I was expecting. This container >> now looks >> remarkably similar to the other existing ones, > I see this as a good sign...to me it means we are arriving at an SPI >> I thought we were trying to >> refactor out the common parts to come up with an easier container >> SPI? > Yes and I couldn't find much to factor into the kernel. However, I > did factor things out that were redundant with the later. >> Lets >> take MissingSideFileException as an example, that could be reused >> by many >> container extensions so could be in a container SPI couldn't it? >> > MissingSideFileException could be moved up. However, I would be > very cautious about moving implementation classes. There is always > a trade-off in SPI design between providing out-of-the-box > implementations versus too tightly coupling extensions to the > containers they are extending. If there are too many implementation > classes in an SPI, it will render the latter inflexible and it will > be difficult to evolve in a backward compatible way in the future. > For example, we can take a few specifics: > > 1. ScriptComponentBuilder. Injecting of properties is specific to > the implementation type. One thing that we could potentially change > is having scope containers passed in. We should think about this. > > 2. ScriptComponentTypeLoader. How a component type side file is > resolved for an implementation type is specific to it. Providing > utility classes for trivial operations such as reading streams will > just complicate the SPI needlessly. > > 3. ScriptImplementationLoader. Pretty much the same comments as for > ScriptComponentTypeLoader. > > One thing that could be simplified in the extension (as I mentioned > previously) is ScriptInstance could be removed altogether. What do > you think? > > Perhaps I'm just not seeing what could be moved up. Could you > elaborate on the specifics? > > Jim > > > >> ...ant >> >> On 11/4/06, Jim Marino < [EMAIL PROTECTED]> wrote: >>> >>> O.K., I committed the refactor to container.script with the follow >>> changes: >>> >>> 1. Fixed the project so it builds again :-) (it is not part of the >>> default build) >>> >>> 2. Removed the need for Async target invokers based on core changes >>> >>> 3. Remove ComponentType hack as that has been fixed in core >>> >>> 4. Changes to better leverage SPI >>> - Removal of all the "helper" classes. I was able to >>> factor these >>> out by using stuff from SPI, class hierarchy simplifications, e.g. >>> ScriptImplementation >>> - Scripts now instantiated in the build phase as opposed to >>> loading >>> - Stronger typing of errors, e.g. MissingSideFileException >>> - Use of ObjectFactory for creating Script instances >>> >>> 5. Fix issue where mutable properties were not thread-safe (i.e. >>> instances were shared across implementation instances). >>> >>> 6. Remove dependency on core impl classes and cleanup some of the >>> unit tests >>> >>> 7. Fix checkstyle warnings and PMD failures >>> >>> 8. Add license file >>> >>> 9 Use a config class for creating a ScriptComponent as that takes a >>> number of constructor parameters >>> >>> >>> There are also some things that could be done as potential >>> improvements: >>> >>> 1. Better test quality. >>> - There are no real tests involving invocations and wires >>> to and >>> from scripts. This could be set up with EasyMock in a pretty >>> straightforward way without the need for SCATestCase >>> - Likewise for properties, we should test accessing >>> properties >>> from within a script. >>> >>> 2. Support for scopes other than module. >>> >>> 3. Some of the class hierarchies seem like they could be simplified, >>> flattened, or removed. For example, ScriptInstance and its >>> implementation could potentially be removed completely and just have >>> the ScriptComponent return the BSF engine (ScriptTargetInvoker would >>> need to be changed for this to carry additional information). >>> >>> >>> Jim >>> >>> On Oct 31, 2006, at 3:05 AM, ant elder wrote: >>> >>> > No go for it, that would be great! >>> > >>> > ...ant >>> > >>> > On 10/31/06, Jim Marino < [EMAIL PROTECTED]> wrote: >>> >> >>> >> So do you mind if I make the changes? >>> >> >>> >> Jim >>> >> >>> >> On Oct 31, 2006, at 1:02 AM, ant elder wrote: >>> >> >>> >> > No, tied up on M2 things, i doubt I'll have time for this >>> till M2 >>> >> > is done. >>> >> > >>> >> > ...ant >>> >> > >>> >> > On 10/31/06, Jim Marino <[EMAIL PROTECTED]> wrote: >>> >> >> >>> >> >> Any luck in making these changes? >>> >> >> >>> >> >> Jim >>> >> >> >>> >> >> On Oct 26, 2006, at 8:30 AM, Jim Marino wrote: >>> >> >> >>> >> >> >> >>> >> >> >> >>> >> >> >> As you don't like the easy SPI extension I've got rid of >>> the >>> >> easy >>> >> >> >> extension >>> >> >> >> dependency of the script container. I've moved the script >>> >> >> >> container into >>> >> >> >> trunk as it was going stale and i want to start using and >>> >> >> >> improving it. It >>> >> >> >> still uses some of the easy classes which are in a helper >>> >> package >>> >> >> >> now, i'll >>> >> >> >> get rid of them as we clean things up. I saw you've done >>> >> some core >>> >> >> >> changes >>> >> >> >> for the componentType problem, thanks, I'll go look at how >>> >> to use >>> >> >> >> that for >>> >> >> >> this (and the other script containers) and change the >>> code as >>> >> >> >> appropriate. >>> >> >> >> You said you'd take on getting the things like the async >>> >> code into >>> >> >> >> the spi >>> >> >> >> to avoid all that duplicate code so would you like to go >>> >> ahead and >>> >> >> >> do that >>> >> >> >> now? (or I can do it if you like). >>> >> >> > O.K. I committed the changes. There is no need to handle >>> >> message id >>> >> >> > correlation as the wiring fabric (specifically >>> >> >> > TargetInvokerExtension) does it automatically. You should be >>> >> able >>> >> >> > to delete AsyncMonitor and the async target invoker. You >>> will >>> >> also >>> >> >> > need to change some of the signatures of the builders to >>> pass >>> >> in a >>> >> >> > WorkContext and ExectionMonitor (these are autowired to >>> >> >> > ComponentBuilderExtension). I made some basic changes the >>> the >>> >> >> > script container to pass tests but you will probably need >>> to do >>> >> >> > some more (limited) refactoring to get it fully >>> operational. I >>> >> >> > fixed the Groovy container so you can use that as an >>> example. >>> >> (BTW, >>> >> >> > as a side note, container.script is not part of the build by >>> >> >> default). >>> >> >> > >>> >> >> > The other changes to make are to create a script >>> >> specialization of >>> >> >> > ComponentType in the loader and use ObjectFactory for >>> creating >>> >> >> > instances. >>> >> >> > >>> >> >> >> Once the componentType and async changes >>> >> >> >> are done I/we can look at the next things to simplify and >>> >> once all >>> >> >> >> that >>> >> >> >> refactoring is done I'll look at what remains in the helper >>> >> >> >> package and see >>> >> >> >> if there are still things I think could be simplified. >>> >> >> >> >>> >> >> > If you can make those changes, we can take another pass >>> over the >>> >> >> > helper classes and see what is left. >>> >> >> > >>> >> >> > Jim >>> >> >> > >>> >> >> >> ...ant >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> >>> >> >>> -------------------------------------------------------------------- >>> - >>> >> >> > To unsubscribe, e-mail: tuscany-dev- >>> [EMAIL PROTECTED] >>> >> >> > For additional commands, e-mail: tuscany-dev- >>> [EMAIL PROTECTED] >>> >> >> > >>> >> >> >>> >> >> >>> >> >> >>> >> >>> -------------------------------------------------------------------- >>> - >>> >> >> To unsubscribe, e-mail: [EMAIL PROTECTED] >>> >> >> For additional commands, e-mail: tuscany-dev- >>> [EMAIL PROTECTED] >>> >> >> >>> >> >> >>> >> >>> >> >>> >> >>> -------------------------------------------------------------------- >>> - >>> >> To unsubscribe, e-mail: [EMAIL PROTECTED] >>> >> For additional commands, e-mail: [EMAIL PROTECTED] >>> >> >>> >> >>> >>> >>> -------------------------------------------------------------------- >>> - >>> To unsubscribe, e-mail: [EMAIL PROTECTED] >>> For additional commands, e-mail: [EMAIL PROTECTED] >>> >>> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
