You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tuscany.apache.org by Simon Laws <si...@googlemail.com> on 2008/03/10 11:27:31 UTC

Re: svn commit: r635435 - in /incubator/tuscany/java/sca/modules: assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/ assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/ core-spring/src/main/java/org/apache/tuscany/s

On Mon, Mar 10, 2008 at 5:43 AM, <js...@apache.org> wrote:

> Author: jsdelfino
> Date: Sun Mar  9 22:43:19 2008
> New Revision: 635435
>
> URL: http://svn.apache.org/viewvc?rev=635435&view=rev
> Log:
> Fixed algorithm in CompositeConfigurationBuilder to produce correct URIs,
> in particular avoid adding binding name to itself, and consider binding URI
> when specified in the composite in the single service case too. Integrated
> CompositeConfigurationBuilder in the main build() now that it covers all
> cases. Adjusted the domain-impl, the callable reference resolution and the
> core-spring reference resolution code to the new URI form.
>
> Modified:
>
>  incubator/tuscany/java/sca/modules/assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/CompositeConfigurationBuilderImpl.java
>
>  incubator/tuscany/java/sca/modules/assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/CalculateBindingURITestCase.java
>
>  incubator/tuscany/java/sca/modules/core-spring/src/main/java/org/apache/tuscany/sca/core/spring/assembly/impl/BeanReferenceImpl.java
>
>  incubator/tuscany/java/sca/modules/core/src/main/java/org/apache/tuscany/sca/core/context/CallableReferenceImpl.java
>
>  incubator/tuscany/java/sca/modules/domain-impl/src/main/java/org/apache/tuscany/sca/domain/impl/SCADomainImpl.java
>
>  incubator/tuscany/java/sca/modules/workspace-admin/src/main/java/org/apache/tuscany/sca/workspace/admin/impl/DeployableCollectionImpl.java
>
> Modified:
> incubator/tuscany/java/sca/modules/assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/CompositeConfigurationBuilderImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/CompositeConfigurationBuilderImpl.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/CompositeConfigurationBuilderImpl.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/CompositeConfigurationBuilderImpl.java
> Sun Mar  9 22:43:19 2008
> @@ -52,9 +52,9 @@
>  import org.apache.tuscany.sca.policy.PolicySetAttachPoint;
>
>  public class CompositeConfigurationBuilderImpl {
> -    String SCA10_NS = "http://www.osoa.org/xmlns/sca/1.0";
> -    String BINDING_SCA = "binding.sca";
> -    QName BINDING_SCA_QNAME = new QName(SCA10_NS, BINDING_SCA);
> +    private final static String SCA10_NS = "
> http://www.osoa.org/xmlns/sca/1.0";
> +    private final static String BINDING_SCA = "binding.sca";
> +    private final static QName BINDING_SCA_QNAME = new QName(SCA10_NS,
> BINDING_SCA);
>
>     private AssemblyFactory assemblyFactory;
>     private SCABindingFactory scaBindingFactory;
> @@ -81,9 +81,10 @@
>      * @param composite
>      * @param problems
>      */
> -    public void configureComponents(Composite composite) {
> +    public void configureComponents(Composite composite) throws
> CompositeBuilderException {
>         configureComponents(composite, null);
>         configureSourcedProperties(composite, null);
> +        configureBindingURIs(composite, null, null);
>     }
>
>     /**
> @@ -124,8 +125,6 @@
>             // Create default SCA binding
>             if (service.getBindings().isEmpty()) {
>                 SCABinding scaBinding = createSCABinding();
> -
> -
>                 service.getBindings().add(scaBinding);
>             }
>
> @@ -136,33 +135,6 @@
>                 if (binding.getName() == null) {
>                     binding.setName(service.getName());
>                 }
> -
> -                String bindingURI;
> -                if (binding.getURI() == null) {
> -                    if (compositeServices.size() > 1) {
> -                        // Binding URI defaults to parent URI / binding
> name
> -                        bindingURI = String.valueOf(binding.getName());
> -                        if (parentURI != null) {
> -                            bindingURI = URI.create(parentURI +
> '/').resolve(bindingURI).toString();
> -                        }
> -                    } else {
> -                        // If there's only one service then binding URI
> defaults
> -                        // to the parent URI
> -                        if (parentURI != null) {
> -                            bindingURI = parentURI;
> -                        } else {
> -                            bindingURI = String.valueOf(binding.getName
> ());
> -                        }
> -                    }
> -                } else {
> -                    // Combine the specified binding URI with the
> component URI
> -                    bindingURI = binding.getURI();
> -                    if (parentURI != null) {
> -                        bindingURI = URI.create(parentURI +
> '/').resolve(bindingURI).toString();
> -                    }
> -                }
> -
> -                binding.setURI(bindingURI);
>             }
>
>             if (service.getCallback() != null) {
> @@ -272,40 +244,13 @@
>                     componentService.getBindings().add(scaBinding);
>                 }
>
> -                // Set binding names and URIs
> +                // Set binding names
>                 for (Binding binding : componentService.getBindings()) {
>
>                     // Binding name defaults to the service name
>                     if (binding.getName() == null) {
>                         binding.setName(componentService.getName());
>                     }
> -
> -                    String bindingURI;
> -                    if (binding.getURI() == null) {
> -                        //if (componentServices.size() > 1) {
> -                        if (component.getServices().size() > 1) {
> -                            // Binding URI defaults to component URI /
> binding name
> -                            bindingURI = String.valueOf(binding.getName
> ());
> -                            bindingURI = URI.create(component.getURI() +
> '/').resolve(bindingURI).toString();
> -                        } else {
> -                            // If there's only one service then binding
> URI defaults
> -                            // to the component URI
> -                            bindingURI = component.getURI();
> -                        }
> -                    } else {
> -                        // Combine the specified binding URI with the
> component URI
> -                        bindingURI = binding.getURI();
> -
> -                        // if this binding comes from a composite service
> then the URI will already be set
> -                        // otherwise we need to set it relative to the
> component URI. We can tell by looking
> -                        // if this binding is the same as the one on the
> component type service
> -                        if ((componentService.getService() != null)&&
> -
>  (!componentService.getService().getBindings().contains(binding))){
> -                            bindingURI = URI.create(component.getURI() +
> '/').resolve(bindingURI).toString();
> -                        }
> -                    }
> -
> -                    binding.setURI(bindingURI);
>                 }
>                 if (componentService.getCallback() != null) {
>                     for (Binding binding : componentService.getCallback().getBindings())
> {
> @@ -1053,7 +998,7 @@
>      * @param compositeService
>      * @return
>      */
> -    static Component getPromotedComponent(CompositeService
> compositeService) {
> +    private static Component getPromotedComponent(CompositeService
> compositeService) {
>         ComponentService componentService =
> compositeService.getPromotedService();
>         if (componentService != null) {
>             Service service = componentService.getService();
> @@ -1105,11 +1050,11 @@
>       *       a composite is actually build in a node the node default
> information is currently
>       *       available
>       *
> -      * @param defaultBindings list of default bindings (from the node
> configuration composite)
>       * @param composite the composite to be configured
>       * @param uri the path to the composite provided through any nested
> composite component implementations
> +      * @param defaultBindings list of default binding configurations
>       */
> -    public void calculateBindingURIs(List<Binding> defaultBindings,
> Composite composite, String uri) throws CompositeBuilderException {
> +    public void configureBindingURIs(Composite composite, String uri,
> List<Binding> defaultBindings) throws CompositeBuilderException {
>
>         String parentComponentURI = uri;
>
> @@ -1129,7 +1074,7 @@
>             if (implementation instanceof Composite) {
>
>                 // Process nested composite
> -                calculateBindingURIs(defaultBindings,
> (Composite)implementation, componentURI);
> +                configureBindingURIs((Composite)implementation,
> componentURI, defaultBindings);
>             }
>         }
>
> @@ -1227,12 +1172,11 @@
>                 continue;
>             }
>             if (binding.getName().equals(otherBinding.getName())) {
> -
> -                throw new CompositeBuilderException("Multiple bindings
> for service " +
> +                warning("Multiple bindings for service " +
>                                                     service.getName() +
>                                                     " have the same
> binding type and name " +
>                                                     binding.getName() +
> -                                                    ". Tuscany SCA can't
> create unique URIs to differentiate these bindings ");
> +                                                    ". Tuscany SCA can't
> create unique URIs to differentiate these bindings ", binding);
>             }
>         }
>     }
> @@ -1253,7 +1197,8 @@
>     throws CompositeBuilderException{
>         // This is a composite service so there is no component to provide
> a component URI
>         // The path to this composite (through nested composites) is used.
> -        constructBindingURI(parentComponentURI, service, binding,
> composite.getServices().size() > 1, defaultBindings);
> +        boolean includeBindingName = composite.getServices().size() != 1;
> +        constructBindingURI(parentComponentURI, service, binding,
> includeBindingName, defaultBindings);
>     }
>
>      /**
> @@ -1268,7 +1213,8 @@
>       */
>     private void constructBindingURI(Component component, Service service,
> Binding binding, List<Binding> defaultBindings)
>     throws CompositeBuilderException{
> -        constructBindingURI(component.getURI(), service, binding,
> component.getServices().size() > 1, defaultBindings);
> +        boolean includeBindingName = component.getServices().size() != 1;
> +        constructBindingURI(component.getURI(), service, binding,
> includeBindingName, defaultBindings);
>     }
>
>     /**
> @@ -1277,32 +1223,38 @@
>      * @param componentURIString the string version of the URI part that
> comes from the component name
>      * @param service the service in question
>      * @param binding the binding for which the URI is being constructed
> -     * @param includeServiceBindingURI when set true the
> serviceBindingURI part should be used
> +     * @param includeBindingName when set true the serviceBindingURI part
> should be used
>      * @param defaultBindings the list of default binding configurations
>      * @throws CompositeBuilderException
>      */
> -    private void constructBindingURI(String componentURIString, Service
> service, Binding binding, boolean includeServiceBindingURI, List<Binding>
> defaultBindings)
> +    private void constructBindingURI(String componentURIString, Service
> service, Binding binding, boolean includeBindingName, List<Binding>
> defaultBindings)
>       throws CompositeBuilderException{
>
>         try {
> -            URI baseURI = null;
> -            URI componentURI = null;
> -            URI serviceBindingURI = null;
> -
>             // calculate the service binding URI
> -            if (binding.getURI() == null){
> -                serviceBindingURI = new URI(binding.getName());
> +            URI bindingURI;
> +            if (binding.getURI() != null){
> +                bindingURI = new URI(binding.getURI());
> +
> +                // if the user has provided an absolute binding URI then
> use it
> +                if (bindingURI.isAbsolute()){
> +                    binding.setURI(bindingURI.toString());
> +                    return;
> +                }
>             } else {
> -                serviceBindingURI = new URI(binding.getURI());
> -            }
> +                bindingURI = null;
> +            }
>
> -            // if the user has provided an absolute binding URI then use
> it
> -            if (serviceBindingURI != null && serviceBindingURI.isAbsolute
> ()){
> -                binding.setURI(serviceBindingURI.toString());
> -                return;
> +            // Get the service binding name
> +            URI bindingName;
> +            if (binding.getName() != null) {
> +                bindingName = new URI(binding.getName());
> +            } else {
> +                bindingName = new URI("");
>             }
>
>             // calculate the component URI
> +            URI componentURI;
>             if (componentURIString != null) {
>                 componentURI = new
> URI(addSlashToPath(componentURIString));
>             } else {
> @@ -1311,7 +1263,7 @@
>
>             // if the user has provided an absolute component URI then use
> it
>             if (componentURI != null && componentURI.isAbsolute()){
> -                binding.setURI(concatenateModelURI(null, componentURI,
> serviceBindingURI, includeServiceBindingURI));
> +                binding.setURI(constructBindingURI(null, componentURI,
> bindingURI, includeBindingName, bindingName));
>                 return;
>             }
>
> @@ -1338,18 +1290,19 @@
>             }
>  */
>             // as a simpler alternative to the above commented out code.
> -            baseURI = null;
> +            URI baseURI = null;
>             if (defaultBindings != null) {
>                 for (Binding defaultBinding : defaultBindings){
>                     if (binding.getClass() == defaultBinding.getClass()){
>                         baseURI = new URI(addSlashToPath(
> defaultBinding.getURI()));
> +                        break;
>                     }
>                 }
>             }
>
> -            binding.setURI(concatenateModelURI(baseURI, componentURI,
> serviceBindingURI,includeServiceBindingURI));
> -        } catch (URISyntaxException ex){
> -            throw new CompositeBuilderException("URLSyntaxException when
> creating binding URI at component " +
> +            binding.setURI(constructBindingURI(baseURI, componentURI,
> bindingURI, includeBindingName, bindingName));
> +        } catch (URISyntaxException ex) {
> +            warning("URLSyntaxException when creating binding URI at
> component " +
>                                                 componentURIString +
>                                                 " service " +
>                                                 service.getName() +
> @@ -1380,36 +1333,52 @@
>      *
>      * @param baseURI the base of the binding URI
>      * @param componentURI the middle part of the binding uri derived from
> the component name
> -     * @param serviceBindingURI the end part of the binding uri derived
> from the service name
> -     * @param includeServiceBindingURI when set true the
> serviceBindingURI part should be used
> +     * @param bindingURI the end part of the binding uri
> +     * @param includeBindingName when set true the binding name part
> should be used
> +     * @param bindingName the binding name
>      * @return the resulting URI as a string
>      */
> -    private String concatenateModelURI(URI baseURI, URI componentURI, URI
> serviceBindingURI, boolean includeServiceBindingURI){
> -
> +    private String constructBindingURI(URI baseURI, URI componentURI, URI
> bindingURI, boolean includeBindingName, URI bindingName){
>         String uriString;
>
> -        if (baseURI == null){
> +        if (baseURI == null) {
>             if (componentURI == null){
> -                uriString = serviceBindingURI.toString();
> +                if (bindingURI != null ) {
> +                    uriString = bindingURI.toString();
> +                } else {
> +                    uriString = bindingName.toString();
> +                }
>             } else {
> -                if (includeServiceBindingURI){
> -                    uriString = componentURI.resolve
> (serviceBindingURI).toString();
> +                if (bindingURI != null ) {
> +                    uriString = componentURI.resolve
> (bindingURI).toString();
>                 } else {
> -                    uriString = componentURI.toString();
> +                    if (includeBindingName) {
> +                        uriString = componentURI.resolve
> (bindingName).toString();
> +                    } else {
> +                        uriString = componentURI.toString();
> +                    }
>                 }
>             }
>         } else {
>             if (componentURI == null){
> -                if (includeServiceBindingURI){
> -                    uriString = baseURI.resolve
> (serviceBindingURI).toString();
> +                if (bindingURI != null ) {
> +                    uriString = baseURI.resolve(bindingURI).toString();
>                 } else {
> -                    uriString = baseURI.toString();
> -                }
> +                    if (includeBindingName) {
> +                        uriString = baseURI.resolve
> (bindingName).toString();
> +                    } else {
> +                        uriString = baseURI.toString();
> +                    }
> +                }
>             } else {
> -                if (includeServiceBindingURI){
> -                    uriString = baseURI.resolve
> (componentURI).resolve(serviceBindingURI).toString();
> +                if (bindingURI != null ) {
> +                    uriString = baseURI.resolve
> (componentURI).resolve(bindingURI).toString();
>                 } else {
> -                    uriString = baseURI.resolve(componentURI).toString();
> +                    if (includeBindingName) {
> +                        uriString = baseURI.resolve
> (componentURI).resolve(bindingName).toString();
> +                    } else {
> +                        uriString = baseURI.resolve
> (componentURI).toString();
> +                    }
>                 }
>             }
>         }
> @@ -1419,6 +1388,10 @@
>             uriString = uriString.substring(0, uriString.length()-1);
>         }
>
> -        return uriString;
> +        URI uri = URI.create(uriString);
> +        if (!uri.isAbsolute()) {
> +            uri = URI.create("/").resolve(uri);
> +        }
> +        return uri.toString();
>     }
>  }
>
> Modified:
> incubator/tuscany/java/sca/modules/assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/CalculateBindingURITestCase.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/CalculateBindingURITestCase.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/CalculateBindingURITestCase.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/CalculateBindingURITestCase.java
> Sun Mar  9 22:43:19 2008
> @@ -229,7 +229,7 @@
>         Binding b = composite.getComponents
> ().get(0).getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1", b.getURI());
>         } catch(Exception ex){
> @@ -243,7 +243,7 @@
>         Binding b = composite.getComponents
> ().get(0).getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/s1", b.getURI());
>         } catch(Exception ex){
> @@ -258,7 +258,7 @@
>         b.setName("n");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/n", b.getURI());
>         } catch(Exception ex){
> @@ -274,7 +274,7 @@
>         b.setURI("b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/b", b.getURI());
>         } catch(Exception ex){
> @@ -290,7 +290,7 @@
>         b.setURI("http://myhost:8080/b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/b", b.getURI());
>         } catch(Exception ex){
> @@ -306,7 +306,7 @@
>         b.setURI("../../b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/b", b.getURI());
>         } catch(Exception ex){
> @@ -323,7 +323,7 @@
>         Binding b = composite.getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root", b.getURI());
>         } catch(Exception ex){
> @@ -337,7 +337,7 @@
>         Binding b = composite.getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/s1", b.getURI());
>         } catch(Exception ex){
> @@ -352,7 +352,7 @@
>         b.setName("n");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/n", b.getURI());
>         } catch(Exception ex){
> @@ -368,7 +368,7 @@
>         b.setURI("b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/b", b.getURI());
>         } catch(Exception ex){
> @@ -384,7 +384,7 @@
>         b.setURI("http://myhost:8080/b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/b", b.getURI());
>         } catch(Exception ex){
> @@ -401,7 +401,7 @@
>         Binding b =
> ((Composite)composite.getComponents().get(0).getImplementation()).getComponents().get(0).getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/c2", b.getURI());
>         } catch(Exception ex){
> @@ -415,7 +415,7 @@
>         Binding b =
> ((Composite)composite.getComponents().get(0).getImplementation()).getComponents().get(0).getServices().get(0).getBindings().get(0);
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/c2/s1", b.getURI());
>         } catch(Exception ex){
> @@ -430,7 +430,7 @@
>         b.setName("n");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/c2/n", b.getURI());
>         } catch(Exception ex){
> @@ -446,7 +446,7 @@
>         b.setURI("b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/root/c1/c2/b", b.getURI());
>         } catch(Exception ex){
> @@ -462,7 +462,7 @@
>         b.setURI("http://myhost:8080/b");
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>
>             assertEquals("http://myhost:8080/b", b.getURI());
>         } catch(Exception ex){
> @@ -473,7 +473,10 @@
>
>     // component service binding name error tests
>
> -    public void testComponentServiceBindingNameError1() {
> +    //FIXME Need to find a better way to test these error cases as
> +    // the composite builder now (intentionally) logs warnings instead of
> +    // throwing exceptions
> +    public void FIXMEtestComponentServiceBindingNameError1() {
>         Composite composite = createComponentServiceBinding();
>         Binding b1 = composite.getComponents
> ().get(0).getServices().get(0).getBindings().get(0);
>         Binding b2 = new TestBindingImpl();
> @@ -481,14 +484,17 @@
>
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>             fail();
>         } catch(Exception ex){
>             //System.out.println(ex.toString());
>         }
>     }
>
> -    public void testComponentServiceBindingNameError2() {
> +    //FIXME Need to find a better way to test these error cases as
> +    // the composite builder now (intentionally) logs warnings instead of
> +    // throwing exceptions
> +    public void FIXMEtestComponentServiceBindingNameError2() {
>         Composite composite = createComponentServiceBinding();
>         Binding b1 = composite.getComponents
> ().get(0).getServices().get(0).getBindings().get(0);
>         Binding b2 = new TestBindingImpl();
> @@ -499,7 +505,7 @@
>
>
>         try {
> -            configurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +            configurationBuilder.configureBindingURIs(composite, null,
> defaultBindings);
>             fail();
>         } catch(Exception ex){
>             System.out.println(ex.toString());
>
> Modified:
> incubator/tuscany/java/sca/modules/core-spring/src/main/java/org/apache/tuscany/sca/core/spring/assembly/impl/BeanReferenceImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/core-spring/src/main/java/org/apache/tuscany/sca/core/spring/assembly/impl/BeanReferenceImpl.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/core-spring/src/main/java/org/apache/tuscany/sca/core/spring/assembly/impl/BeanReferenceImpl.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/core-spring/src/main/java/org/apache/tuscany/sca/core/spring/assembly/impl/BeanReferenceImpl.java
> Sun Mar  9 22:43:19 2008
> @@ -40,6 +40,9 @@
>     public String getBeanName() {
>         SCABinding binding = reference.getBinding(SCABinding.class);
>         String name = binding.getURI();
> +        if (name.startsWith("/")) {
> +            name = name.substring(1);
> +        }
>         int s = name.lastIndexOf('/');
>         if (s != -1) {
>             name = name.substring(0, s);
>
> Modified:
> incubator/tuscany/java/sca/modules/core/src/main/java/org/apache/tuscany/sca/core/context/CallableReferenceImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/core/src/main/java/org/apache/tuscany/sca/core/context/CallableReferenceImpl.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/core/src/main/java/org/apache/tuscany/sca/core/context/CallableReferenceImpl.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/core/src/main/java/org/apache/tuscany/sca/core/context/CallableReferenceImpl.java
> Sun Mar  9 22:43:19 2008
> @@ -248,19 +248,22 @@
>                 for (Binding binding : reference.getBindings()) {
>                     if (binding instanceof OptimizableBinding) {
>                         String targetURI = binding.getURI();
> +                        if (targetURI.startsWith("/")) {
> +                            targetURI = targetURI.substring(1);
> +                        }
>                         int index = targetURI.lastIndexOf('/');
>                         String serviceName = "";
>                         if (index > -1) {
>                             serviceName = targetURI.substring(index + 1);
>                             targetURI = targetURI.substring(0, index);
>                         }
> -                        Component targetComponet =
> compositeActivator.resolve(targetURI);
> +                        Component targetComponent =
> compositeActivator.resolve(targetURI);
>                         ComponentService targetService = null;
> -                        if (targetComponet != null) {
> +                        if (targetComponent != null) {
>                             if ("".equals(serviceName)) {
> -                                targetService =
> ComponentContextHelper.getSingleService(targetComponet);
> +                                targetService =
> ComponentContextHelper.getSingleService(targetComponent);
>                             } else {
> -                                for (ComponentService service :
> targetComponet.getServices()) {
> +                                for (ComponentService service :
> targetComponent.getServices()) {
>                                     if (service.getName().equals(serviceName))
> {
>                                         targetService = service;
>                                         break;
> @@ -269,7 +272,7 @@
>                             }
>                         }
>                         OptimizableBinding optimizableBinding =
> (OptimizableBinding)binding;
> -                        optimizableBinding.setTargetComponent
> (targetComponet);
> +                        optimizableBinding.setTargetComponent
> (targetComponent);
>                         optimizableBinding.setTargetComponentService
> (targetService);
>                         if (targetService != null) {
>                             for (Binding serviceBinding :
> targetService.getBindings()) {
>
> Modified:
> incubator/tuscany/java/sca/modules/domain-impl/src/main/java/org/apache/tuscany/sca/domain/impl/SCADomainImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/domain-impl/src/main/java/org/apache/tuscany/sca/domain/impl/SCADomainImpl.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/domain-impl/src/main/java/org/apache/tuscany/sca/domain/impl/SCADomainImpl.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/domain-impl/src/main/java/org/apache/tuscany/sca/domain/impl/SCADomainImpl.java
> Sun Mar  9 22:43:19 2008
> @@ -670,6 +670,9 @@
>                                "]");
>
>         String url = SERVICE_NOT_REGISTERED;
> +        if (serviceName.startsWith("/")) {
> +            serviceName = serviceName.substring(1);
> +        }
>         String serviceKey = serviceName + bindingName;
>
>         for (NodeModel node : domainModel.getNodes().values()){
>
> Modified:
> incubator/tuscany/java/sca/modules/workspace-admin/src/main/java/org/apache/tuscany/sca/workspace/admin/impl/DeployableCollectionImpl.java
> URL:
> http://svn.apache.org/viewvc/incubator/tuscany/java/sca/modules/workspace-admin/src/main/java/org/apache/tuscany/sca/workspace/admin/impl/DeployableCollectionImpl.java?rev=635435&r1=635434&r2=635435&view=diff
>
> ==============================================================================
> ---
> incubator/tuscany/java/sca/modules/workspace-admin/src/main/java/org/apache/tuscany/sca/workspace/admin/impl/DeployableCollectionImpl.java
> (original)
> +++
> incubator/tuscany/java/sca/modules/workspace-admin/src/main/java/org/apache/tuscany/sca/workspace/admin/impl/DeployableCollectionImpl.java
> Sun Mar  9 22:43:19 2008
> @@ -404,25 +404,30 @@
>         }
>
>         // configure the endpoints for each composite in the domain
> -        for (Composite composite : domainComposite.getIncludes()) {
> -            List<Binding> defaultBindings = null;
> -
> -            // find the node that will run this composite
> +        List<Composite> domainIncludes = domainComposite.getIncludes();
> +        for (int i = 0, n =domainIncludes.size(); i < n; i++) {
> +            Composite composite = domainIncludes.get(i);
>             QName compositeName = composite.getName();
> -            String contributionURI = composite.getURI();
> +            String contributionURI = uri(domainEntries[i].getKey());
> +
> +            // find the node that will run this composite and the default
> +            // bindings that it configures
> +            Component node = null;
>             for (Composite cloudComposite : cloudsComposite.getIncludes())
> {
> -                for (Component node : cloudComposite.getComponents()) {
> -                    NodeImplementation nodeImplementation =
> (NodeImplementation)node.getImplementation();
> +                for (Component nc : cloudComposite.getComponents()) {
> +                    NodeImplementation nodeImplementation =
> (NodeImplementation)nc.getImplementation();
>                     if (nodeImplementation.getComposite().getName().equals(compositeName)
> &&
>                         nodeImplementation.getComposite().getURI().equals(contributionURI))
> {
> -                        defaultBindings = node.getServices
> ().get(0).getBindings();
> +                        node = nc;
> +                        break;
>                     }
>                 }
>             }
>
> -            if (defaultBindings != null) {
> +            if (node != null) {
>                 try {
> -                    compositeConfigurationBuilder.calculateBindingURIs(defaultBindings,
> composite, null);
> +                    List<Binding> defaultBindings = node.getServices
> ().get(0).getBindings();
> +                    compositeConfigurationBuilder.configureBindingURIs(composite,
> null, defaultBindings);
>                 } catch (CompositeBuilderException e) {
>                     throw new ServletException(e);
>                 }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tuscany-commits-unsubscribe@ws.apache.org
> For additional commands, e-mail: tuscany-commits-help@ws.apache.org
>
> Hi

There is now a test in the binding name creation code that checks that two
service bing types are the same before testing for duplicate names. I don't
think the binding type is relevant here. We should actually be testing that
the scheme that the bindings intend to use are the same but that information
is not easily available. With the current change we can say

<binding.ws name="fred"/>
<binding.jsonrpc name="fred"/>

and I don't believe the code will complain.

Also can you give a quick example the problems you found relating to "avoid
adding binding name to itself, and consider binding URI when specified in
the composite in the single service case too" so I can understand the other
code changes.

Thanks

Simon

Re: svn commit: r635435 - in /incubator/tuscany/java/sca/modules: assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/ assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/ core-spring/src/main/java/org/apache/tuscany/s

Posted by Jean-Sebastien Delfino <js...@apache.org>.
Simon Laws wrote:
> Comments inline....
> 
> snip...
> 
>> and that's OK I think :) as binding.ws and binding.jsonrpc could end up
>> on different port numbers for example (depending on their node
>> configuration).
> 
> 
> And they might not.
> 
> 
>> Actually, I was struggling to understand why we needed this test for
>> duplicate names at all.
>>
>> Does the spec forbids two bindings with the same name?
> 
> 
> No but it says that if  the name is used as the default binding URI then
> only one binding for a service can use this default.
> 
> 
>> By trying hard to detect duplicate names early on (without having all
>> the info about what the binding URIs will end up to be), won't we raise
>> errors  even for cases that should work?
>>
> 
> I agree that we don't have all of the information required to carry out this
> test. Hence the TODO in the code.
> 
> 
>> One example of the issues I was running into:
>> <service...>
>>   <binding.jsonrpc/>
>>   <binding.ws uri="/whatever">
>> </service>
>> throwing a duplicate name exception as the two bindings had the same name.
>>
>> On a fun note, I think that:
>> <service name="aservice">
>>   <binding.jsonrpc name="whatever"/>
>>   <binding.ws uri="/whatever">
>> </service>
>> would probably not have thrown an exception, but should have :)
>>
> 
> Hmmm. not sure but you could be right.
> 
> 
>> So my recommendation would be to not try too hard to detect these
>> duplicates early on based on binding names (as the detection algorithm
>> is too fragile at that point), instead detect duplicates at the very end
>> when we see that we end up with duplicate URIs.
> 
> 
> +1. I'll add a new method to scan the model for duplicate URI that can be
> called independently.
> 

+1 this will be simpler and more reliable.

Eventually all the code that calculates binding URIs (in the 
Axis2ServiceBindingProvider for example) will have to move to the model 
processing phase as well, as already discussed.

>>
>>> Also can you give a quick example the problems you found relating to
>> "avoid
>>> adding binding name to itself, and consider binding URI when specified
>> in
>>> the composite in the single service case too" so I can understand the
>> other
>>> code changes.
>> One issue was that the code was not considering the binding URI when
>> there was only one service on a component, for example IIRC:
>> <component name="Store">
>>   <service name="Widget">
>>     <binding.http uri="/ui"/>
>>   </service>
>> </component>
>> was bound to /Store (the component URI) instead of /ui.
>>
>> I understand that the binding name should be omitted from the computed
>> URI in the single service case, but the binding URI should be considered
>> if specified, leading to the requirement to distinguish between binding
>> name (always know, not always considered) and binding URI (not always
>> specified, always considered when specified).
> 
> 
> Ok. Thank you. I had interpreted the spec incorrectly here. I had mistakenly
> read assembly spec line 2375 as "Where a component has only a single
> service, the value of the Service Binding URI is null" instead of what it
> actually says, which is "Where a component has only a single service, the *
> default* value of the Service Binding URI is null".
> 
> 
>> About the "adding the binding name to itself" part, this is a little
>> complicated. IIRC the issue was that CompositeConfigurationBuilder would
>> first convert:
>> <component name="Foo">
>>   <service...>
>>     <binding.sca/>
>>   </service>
>> </component>
>>
>> into:
>> <component name="Foo">
>>   <service...>
>>     <binding.sca uri="Foo"/>
>>   </service>
>> </component>
>>
>> Then when an SCANode would consume that, CompositeBuilder.build would
>> turn it into:
>> <component name="Foo">
>>   <service...>
>>     <binding.sca uri="Foo/Foo"/>
>>   </service>
>> </component>
>>
>> as I think the old CompositeConfigurationBuilder code used in that case
>> used to concatenate the component URI and the binding URI.
> 
> 
> OK, I see. So this was a combination of the old and the new causing
> problems. I see that you now call the old after the new has run so am
> assuming that your changes are making this behave now. Let me know if not.
> 

Yes the latest code handles that correctly now.

> 
>>> Thanks
>>>
>>> Simon
>>>

-- 
Jean-Sebastien

---------------------------------------------------------------------
To unsubscribe, e-mail: tuscany-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: tuscany-dev-help@ws.apache.org


Re: svn commit: r635435 - in /incubator/tuscany/java/sca/modules: assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/ assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/ core-spring/src/main/java/org/apache/tuscany/s

Posted by Simon Laws <si...@googlemail.com>.
Comments inline....

snip...

>
> and that's OK I think :) as binding.ws and binding.jsonrpc could end up
> on different port numbers for example (depending on their node
> configuration).


And they might not.


>
> Actually, I was struggling to understand why we needed this test for
> duplicate names at all.
>
> Does the spec forbids two bindings with the same name?


No but it says that if  the name is used as the default binding URI then
only one binding for a service can use this default.


>
> By trying hard to detect duplicate names early on (without having all
> the info about what the binding URIs will end up to be), won't we raise
> errors  even for cases that should work?
>

I agree that we don't have all of the information required to carry out this
test. Hence the TODO in the code.


> One example of the issues I was running into:
> <service...>
>   <binding.jsonrpc/>
>   <binding.ws uri="/whatever">
> </service>
> throwing a duplicate name exception as the two bindings had the same name.
>
> On a fun note, I think that:
> <service name="aservice">
>   <binding.jsonrpc name="whatever"/>
>   <binding.ws uri="/whatever">
> </service>
> would probably not have thrown an exception, but should have :)
>

Hmmm. not sure but you could be right.


> So my recommendation would be to not try too hard to detect these
> duplicates early on based on binding names (as the detection algorithm
> is too fragile at that point), instead detect duplicates at the very end
> when we see that we end up with duplicate URIs.


+1. I'll add a new method to scan the model for duplicate URI that can be
called independently.


>
>
> >
> > Also can you give a quick example the problems you found relating to
> "avoid
> > adding binding name to itself, and consider binding URI when specified
> in
> > the composite in the single service case too" so I can understand the
> other
> > code changes.
>
> One issue was that the code was not considering the binding URI when
> there was only one service on a component, for example IIRC:
> <component name="Store">
>   <service name="Widget">
>     <binding.http uri="/ui"/>
>   </service>
> </component>
> was bound to /Store (the component URI) instead of /ui.
>
> I understand that the binding name should be omitted from the computed
> URI in the single service case, but the binding URI should be considered
> if specified, leading to the requirement to distinguish between binding
> name (always know, not always considered) and binding URI (not always
> specified, always considered when specified).


Ok. Thank you. I had interpreted the spec incorrectly here. I had mistakenly
read assembly spec line 2375 as "Where a component has only a single
service, the value of the Service Binding URI is null" instead of what it
actually says, which is "Where a component has only a single service, the *
default* value of the Service Binding URI is null".


>
> About the "adding the binding name to itself" part, this is a little
> complicated. IIRC the issue was that CompositeConfigurationBuilder would
> first convert:
> <component name="Foo">
>   <service...>
>     <binding.sca/>
>   </service>
> </component>
>
> into:
> <component name="Foo">
>   <service...>
>     <binding.sca uri="Foo"/>
>   </service>
> </component>
>
> Then when an SCANode would consume that, CompositeBuilder.build would
> turn it into:
> <component name="Foo">
>   <service...>
>     <binding.sca uri="Foo/Foo"/>
>   </service>
> </component>
>
> as I think the old CompositeConfigurationBuilder code used in that case
> used to concatenate the component URI and the binding URI.


OK, I see. So this was a combination of the old and the new causing
problems. I see that you now call the old after the new has run so am
assuming that your changes are making this behave now. Let me know if not.


>
> >
> > Thanks
> >
> > Simon
> >
>
> --
> Jean-Sebastien
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tuscany-dev-unsubscribe@ws.apache.org
> For additional commands, e-mail: tuscany-dev-help@ws.apache.org
>
>

Re: svn commit: r635435 - in /incubator/tuscany/java/sca/modules: assembly/src/main/java/org/apache/tuscany/sca/assembly/builder/impl/ assembly/src/test/java/org/apache/tuscany/sca/assembly/builder/impl/ core-spring/src/main/java/org/apache/tuscany/s

Posted by Jean-Sebastien Delfino <js...@apache.org>.
Simon Laws wrote:
>>
>> Hi
> 
> There is now a test in the binding name creation code that checks that two
> service bing types are the same before testing for duplicate names. I don't
> think the binding type is relevant here. We should actually be testing that
> the scheme that the bindings intend to use are the same but that information
> is not easily available. With the current change we can say
> 
> <binding.ws name="fred"/>
> <binding.jsonrpc name="fred"/>
> 
> and I don't believe the code will complain.

and that's OK I think :) as binding.ws and binding.jsonrpc could end up 
on different port numbers for example (depending on their node 
configuration).

Actually, I was struggling to understand why we needed this test for 
duplicate names at all.

Does the spec forbids two bindings with the same name?

By trying hard to detect duplicate names early on (without having all 
the info about what the binding URIs will end up to be), won't we raise 
errors  even for cases that should work?

One example of the issues I was running into:
<service...>
   <binding.jsonrpc/>
   <binding.ws uri="/whatever">
</service>
throwing a duplicate name exception as the two bindings had the same name.

On a fun note, I think that:
<service name="aservice">
   <binding.jsonrpc name="whatever"/>
   <binding.ws uri="/whatever">
</service>
would probably not have thrown an exception, but should have :)

So my recommendation would be to not try too hard to detect these 
duplicates early on based on binding names (as the detection algorithm 
is too fragile at that point), instead detect duplicates at the very end 
when we see that we end up with duplicate URIs.

> 
> Also can you give a quick example the problems you found relating to "avoid
> adding binding name to itself, and consider binding URI when specified in
> the composite in the single service case too" so I can understand the other
> code changes.

One issue was that the code was not considering the binding URI when 
there was only one service on a component, for example IIRC:
<component name="Store">
   <service name="Widget">
     <binding.http uri="/ui"/>
   </service>
</component>
was bound to /Store (the component URI) instead of /ui.

I understand that the binding name should be omitted from the computed 
URI in the single service case, but the binding URI should be considered 
if specified, leading to the requirement to distinguish between binding 
name (always know, not always considered) and binding URI (not always 
specified, always considered when specified).

About the "adding the binding name to itself" part, this is a little 
complicated. IIRC the issue was that CompositeConfigurationBuilder would 
first convert:
<component name="Foo">
   <service...>
     <binding.sca/>
   </service>
</component>

into:
<component name="Foo">
   <service...>
     <binding.sca uri="Foo"/>
   </service>
</component>

Then when an SCANode would consume that, CompositeBuilder.build would 
turn it into:
<component name="Foo">
   <service...>
     <binding.sca uri="Foo/Foo"/>
   </service>
</component>

as I think the old CompositeConfigurationBuilder code used in that case 
used to concatenate the component URI and the binding URI.

> 
> Thanks
> 
> Simon
> 

-- 
Jean-Sebastien

---------------------------------------------------------------------
To unsubscribe, e-mail: tuscany-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: tuscany-dev-help@ws.apache.org