Hi Sebastien
Please see my comments inline. Thanks.
- Venkat
On Fri, Mar 14, 2008 at 1:07 AM, Jean-Sebastien Delfino <
[EMAIL PROTECTED]> wrote:
> Venkata Krishnan wrote:
> I set up the targetnamespace for the names of
> > Intents and PolicySets in the SCADefnsProcessor after an Intent or
> PolicySet
> > has been read by a downstream processor.
>
> Given how the policy code is currently organized that seems the simplest
> option: SCADefinitionsProcessor is responsible for handling the
> targetNamespace, and setting it in the qnames of the policy intents and
> policySets that it adds to its lists of policy intents and policySets.
>
> This could be much simplified though, with the following changes:
>
> 1) cosmetic but it'll help make that code more readable, change
>
> if (extension != null) {
> if ( extension instanceof Intent ) {
> ((Intent)extension).setName(new QName(targetNamespace,
>
> ((Intent)extension).getName().getLocalPart()));
>
> to
>
> if (extension instanceof Intent ) {
> Intent intent = (Intent)extension;
> intent.setName(new QName(targetNamespace,
> intent.getName().getLocalPart()));
>
> as the double 'if' is not necessary, and a local variable will help
> avoid repeating the casts.
>
Yes :) this if looks really crazy now that I am looking back at it. Will
correct it.
>
> 2) Unless I'm missing something, I don't think that you need to set the
> targetNamespace of QualifiedIntent.qualifiableIntents, as it looks like
> it's already read as a QName from the XML stream (and this QName does
> not have to be in the current targetNamespace).
First, I think that the Qualifiable intent needs to be in the current
namespace since I I am not sure and the specs does not mention either, on
how we could represent a qualified intent whose qualifiable intent belongs
to a different namespace. So going with the assumption, in this context the
qualifiable intent needs to be assigned the targetnamspace. Only then would
it be correctly resolved during the resolution phase.
>
>
> 3) Finally a bigger change: it seems that you have StAXArtifactProcessor
> extensions for Intent, PolicySet etc... but these elements are not
> extensions, so you didn't have to go through all that, as they are part
> of the SCA core namespace. So, a much simpler approach would be to just
> read them in, directly from SCADefinitionsProcessor. This is similar to
> CompositeProcessor for example, if we had made a separate processor for
> Component, we would have had to pass a lot of context to it.
>
> In short, it looks like you've created a maze of processor extensions
> for things that didn't have to be extensions, and are now wondering how
> to pass context through this maze :)
>
> The solution is simple, just don't make them extensions :) You can
> either move this code to SCADefinitionsProcessor.read() or to private
> methods of SCADefinitionsProcessor to which you'll be able to pass
> whatever context you need.
>
> Hope this helps.
> --
> Jean-Sebastien
>
This is the the first temptation that I went thro i.e. to merge all of this
processing into the SCADefinitionsProcessor similar to the
CompositeProcessor. We did start with all of this in a single module but
somewhere down the line we decided to split them all into different modules
(can't quite remember and pull up that discussion around this). Ok, let me
get them back together for now.
However, I am not sure how far we could go with this. You will agree that
the 'read' and 'resolve' methods of the CompositeProcessor are quite bulky
running to pages and contains quite a bit of state management.
- Venkat
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
