Simon Laws wrote:
I've started making some changes to the code to move this along a bit.
I'm using ASM_5004 as a test case and have only fixed up the one error
that this test cases exercises in the
ComponentReferenceWireBuilderImpl . This test case does correctly
detect a "Too many targets on reference" error. The output that this
produces is now
22-Jul-2009 21:53:30
org.apache.tuscany.sca.assembly.builder.impl.ComponentReferenceWireBuilderImpl
[{http://tuscany.apache.org/xmlns/sca/1.1}_tempComposite, TestClient,
reference1]
SEVERE: Too many targets on reference: reference1
22-Jul-2009 21:53:30
org.apache.tuscany.sca.assembly.builder.impl.ReferenceConfigurationUtil
[]
WARNING: Component reference reference1 has more than one wires
org.oasisopen.sca.ServiceRuntimeException:
[{http://tuscany.apache.org/xmlns/sca/1.1}_tempComposite, TestClient,
reference1] - Too many targets on reference: reference1
java.lang.IllegalStateException:
org.oasisopen.sca.ServiceRuntimeException:
[{http://tuscany.apache.org/xmlns/sca/1.1}_tempComposite, TestClient,
reference1] - Too many targets on reference: reference1
....
I'm not sure what was reported previously. Looking at this output, there
still seem to be some problems:
1. WARNING reported as well as SEVERE error. These appear to both be
caused by the same user error.
2. Two exceptions (ServiceRuntimeException and IllegalStateException)
are reported in addition to the monitor-derived messages.
Simon
Context:
======
I've added simple interface to the monitor for maintaining context
/**
* Add a context string to the context stack
* @param context the context string to add
*/
public abstract void pushContext(String context);
/**
* Remove the most recent context string from the
* context stack
*/
public abstract void popContext();
At the moment I've added appropriate calls to the
ComponentReferenceWireBuilderImpl in order to record the composite,
component and reference being processed. The composite doesn't turn
out to be very useful as the composite being processed always starts
with the top level composite and direct children are implicitly
included and hence are thrown away by this stage. We could make this
more useful by going back to Raymond's scheme of building the included
composites separately but we face the same problem with any included
composites further down the stack.
I also had to change the structure of the loop in the code to allow me
to get at the component in question. We have many instances where we
create and index of components, services and references. I wonder if
we still need all of them?
I'm printing out the context in the "method" field of the log
statement. It did make me wonder if the class name is still useful
here.
I am not convinced that this is the best approach. I think it's better
for the runtime code that detects and report the error to maintain
the necessary state and put this in the error message. When I get home
(tomorrow) I will post an example of what I have in mind.
Error Utilities
=========
I've had a go at removing the need to have error reporting utility
code in every module that uses the monitor by putting the utilities on
the monitor itself. So you report an error as follows.
Monitor.error(monitor,
this,
"assembly-validation-messages",
"TooManyReferenceTargets",
componentReference.getName());
+1 for this change.
Would be nice to get rid of the need to pass the bundle name in. Any ideas?
Stopping on Error
=============
Haven't done anything explicitly awaiting further discussion. However
we do need to check every error path back and make sure the context
path it unwound properly. This does give us the chance to look at how
defensive our coding is.
See my other post on this.
All of this starts to build a set of rules for using the monitor.
- if the monitor is passed in then use it and don't throw exceptions
explicitly. Parts of our code and 3rd party libaries will throw
exceptions so we need to make sure we catch them
>
+1
- push and pop context in a balanced way being careful of any
exceptions that are thrown
>
See comment above.
- Need to say something about warning/error/fatal.
+1
Simon
Not saying these are the right rules yet but I'd like to have a set at
the end of this discussion
Simon
-