tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher K.St.John <...@distributopia.com>
Subject Re: [PATCH] MinimalTomcat, Coupling, Bugs 6669, 6670
Date Thu, 28 Feb 2002 16:35:40 GMT
"Craig R. McClanahan" wrote:
> 
> Internal references from one package to another (say,
> org.apache.catalina.realm to org.apache.catalina.core) are much less
> desireable. 
> ...
> Is it appropriate to go through the exercise of identifying the
> offending cases and defining proposed changes to implement them?
> Yah, it's definitely time for that to happen.
> 

 Currently, AuthenticatorBase depends on StandardContext. It's
not some deep dependency. AuthenticatorBase just wants to set
its debug level to be the same as its Context. There's an
instanceof check before the cast, so AuthenticatorBase doesn't
actually require that its parent be a StandardContext, but
it does require that StandardContext exists. It produces a 
'mystery dependency' that is invisible from the outside. 

 I may be preaching to the converted, but mystery dependencies
are bad. They make the code brittle and hard to understand.
They lead to spaghetti.

 I reported the dependency as a bug (with a patch) and the
bug was changed to a feature request. I suppose it depends
on how you look at it. It's certainly a bug for me. 

 There are a couple of ways to fix the problem:

 1) Delete the offending code. Easiest, and in this case
 probably reasonable. You'll lose the debug-setting 
 propagation, but you can always set that manually. It's for
 debugging so presumably you're messing with the settings
 any case. My patch did this, I'd like to see it applied.

 2) Move setDebug() up to the Context or Container level.
 They all implement setDebug() anyway, right? And it doesn't
 break compatibility with old code. Med/long term, this would
 probably be a good solution.

 3) Create a Debug interface. Pretty much everything in 
 Catalina has set/getDebug() methods, not just Containers.
 But it's overkill. Each new class or interface adds
 conceptual overhead to the system. I don't think it's
 worth it.

 Whether you like the idea of something like MimimalTomcat
or not, I suspect writing it will turn up some problems
(and solutions) that will improve all of the Catalina code.


 While we're on the subject of interfaces, what is
Context.findRoleMapping() for? I've got it stubbed out in
MinimalTomcat, but I'd like to know what I'm stubbing out.
It used to be used in HttpRequestBase.isUserInRole(), but
the call was replaced by a call to Context.findSecurityRole().
 

-- 
Christopher St. John cks@distributopia.com
DistribuTopia http://www.distributopia.com

--
To unsubscribe, e-mail:   <mailto:tomcat-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:tomcat-dev-help@jakarta.apache.org>


Mime
View raw message