On 17/11/2012 13:05, Rainer Jung wrote: > Hi, > > I did some experiments about more flexible version mapping for contextes > a while back and was a bit unhappy about the type unsafetyness of the > Mapper. Internally it only uses plain Objects and the code using it has > to cast everything. > > There is a patch available at > > http://people.apache.org/~rjung/patches/mapper-typesafe-trunk.patch
There is a lot to take in there. I am generally in agreement but find myself rather overwhelmed by the patch. May I suggest you go ahead with just the renaming of Host->MappedHost etc and commit that to trunk (should be simple to do in an IDE) and then prepare a patch that makes the mapper type safe. If it would be easier for me to make the name changes and then you could just svn up I'm happy to do so. Let me know. > that makes the Mapper type safe. The only changes outside of the mapper > package is the removal of the casts. The TestMapper class had to be > adjusted to use Host, Context and Wrapper objects instead of Strings. > > The test suite runs without errors. > > One change is potentially problematic: MapperListener previously > registered a new context using the Container returned by > context.getParent(), ignoring whether it is a Host or something else. I > had to cast to Host, because registering a Context for any type of > Container is no longer possible. I doubt that previously it would have > really worked, because all code using the container returned later by > the mapper would have assumed it to be a Host. > > The Context interface mentions in comments, that the parent might be > something else than a Host. Does anyone know a good example? I can't think of a single example. > Would it have worked? No. I see multiple places where the result would be an NPE if Context->Host->Engine is not in place. > What would the expected behaviour of MapperListener have > been (where would it have registered that context in the mapper)? I doubt the start up processes would have got that far. I suspect that the initial design allowed for a different container hierarchy but over time the Wrapper->Context->Host->Engine hierarchy has become embedded in numerous places in the code. I see no reason to unpick that so lets make it explicit and override getParent() for Wrapper, Context and Host for Tomcat 8. It will enable us to remove lots of casting and we may even find a few obscure bugs. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
