Re: svn commit: r561135 - in /incubator/cxf/trunk: api/src/main/java/org/apache/cxf/headers/ rt/bindings/soap/src/main/java/org/apache/cxf/binding/soap/ rt/bindings/soap/src/main/java/org/apache/cxf/binding/soap/interceptor/ systests/ systests/src/test/jav...

[Daniel Kulp <dk...@apache.org>]

Glen,

Hmm... just realized I spelled your name wrong in my commit.   I 
apologize for that.

On Monday 30 July 2007 20:12, Glen Mazza wrote:
> Can we make the Direction enum above public, so it can be used
> externally--that way, we would not need to define DIRECTION_IN, _OUT,
> _INOUT static integers at the top?  (Or is it that the callee can not
> work with enums--if so, can we just work with the static ints then?)
> Having both enum constants and static integers with the same name is a
> little bit confusing, if we could just work with one of the two I
> think that would be better.

Agreed.   I just took care of that.

> > +    public void handleMessage(SoapMessage message) throws Fault {
> > +        // TODO Auto-generated method stub
> > +        Iterator<Header> iter =  message.getHeaders().iterator();
>
> (I don't know the code here.  Is there any chance the iterator above
> could be null -- i.e., if there are no headers?  If so, the above line
> of code will NPE.)

No.  SoapMessage.getHeaders() creates a new list if one isn't yet 
present.

> > +    public KeystorePasswordCallback() {
> > +        passwords.put("Alice", "abcd!1234");
> > +        passwords.put("alice", "abcd!1234");
> > +        passwords.put("Bob", "abcd!1234");
> > +        passwords.put("bob", "abcd!1234");
> > +    }
> > +
>
> What is the point of adding both Alice and alice here?  Bob and bob? 
> If you're trying to make user entry case-insensitive (not really
> necessary IMHO), I don't think that will work, because people could
> still enter "BOB", "bOB", etc., and the above would fail anyway.

I'll let Ulhas or Fred comment on this.  I have no idea.



> > +    public static void main(String[] args) {
> > +        try {
> > +            Server s = new Server();
> > +            s.start();
> > +        } catch (Exception ex) {
> > +            ex.printStackTrace();
> > +            System.exit(-1);
> > +        } finally {
> > +            System.out.println("done!");
> > +        }
> > +    }
>
> I think this needs fixing.  The purpose of the finally is "run this
> code whether or not there was an exception", but that is circumvented
> with the System.exit(-1) in the catch {} block.  I think either the
> System.exit(-1) line has to be removed, or the finally block removed,
> with its single statement placed at the end of the try{} block.

Hmm...  very good point.   That said, I think a TON of our system tests 
probably have the same problem.  I'm willing to bet this is a copy/paste 
from one of  the others.  :-(   I suppose I could argue that there are 
throwables that aren't exceptions that could invoke the finally (like 
OutOfMemoryErrors and the like), but I'd really just be stretching for 
an excuse.

-- 
J. Daniel Kulp
Principal Engineer
IONA
P: 781-902-8727    C: 508-380-7194
daniel.kulp@iona.com
http://www.dankulp.com/blog