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 2009/05/20 23:30:54 UTC

Re: svn commit: r775672 - in /tuscany/sandbox/travelsample: contributions/introducing-client-contribution/src/main/java/scatours/client/ contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ launchers/introducing-l

On Wed, May 20, 2009 at 1:20 PM, Simon Nash <na...@apache.org> wrote:
> I don't agree with the change to introducing-launcher in this commit
> that puts the client code inline and eliminates the use of the
> introducing-client-contribution.
>
> Having the client code as part of the launcher is acceptable for
> jumpstart-launcher to keep this sample very simple, but for the other
> samples I think the pattern of having a separate contribution for
> test client code is a good pattern that we should use.
>
>  Simon
>
> slaws@apache.org wrote:
>>
>> Author: slaws
>> Date: Sun May 17 15:23:10 2009
>> New Revision: 775672
>>
>> URL: http://svn.apache.org/viewvc?rev=775672&view=rev
>> Log:
>> Add a little more function to the simple travel booking sample and
>> simplify the command line launcher contribution configuration.
>> Added:
>>
>>  tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>>   (with props)
>>
>>  tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>>   (with props)
>> Modified:
>>
>>  tuscany/sandbox/travelsample/contributions/introducing-client-contribution/src/main/java/scatours/client/TestClient.java
>>
>>  tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/Checkout.java
>>
>>  tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ShoppingCart.java
>>
>>  tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/LaunchNode.java
>>
>>  tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/resources/scatours.composite
>>
>> Modified:
>> tuscany/sandbox/travelsample/contributions/introducing-client-contribution/src/main/java/scatours/client/TestClient.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/contributions/introducing-client-contribution/src/main/java/scatours/client/TestClient.java?rev=775672&r1=775671&r2=775672&view=diff
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/contributions/introducing-client-contribution/src/main/java/scatours/client/TestClient.java
>> (original)
>> +++
>> tuscany/sandbox/travelsample/contributions/introducing-client-contribution/src/main/java/scatours/client/TestClient.java
>> Sun May 17 15:23:10 2009
>> @@ -19,6 +19,8 @@
>>   package scatours.client;
>>  +import java.math.BigDecimal;
>> +
>>  import com.tuscanyscatours.Bookings;
>>  import com.tuscanyscatours.Checkout;
>>  @@ -40,7 +42,6 @@
>>         String bookingCode = bookings.newBooking("FS1APR4", 1);
>>         System.out.println("Booking code is " + bookingCode);
>>  -        checkout.makePayment(1995.00, "1234567843218765 10/10");
>> -        System.out.println("Paid $1995.00");
>> +        checkout.makePayment(new BigDecimal("1995.00"), "1234567843218765
>> 10/10");
>>     }
>>  }
>>
>> Modified:
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/Checkout.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/Checkout.java?rev=775672&r1=775671&r2=775672&view=diff
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/Checkout.java
>> (original)
>> +++
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/Checkout.java
>> Sun May 17 15:23:10 2009
>> @@ -18,9 +18,11 @@
>>  */
>>  package com.tuscanyscatours;
>>  +import java.math.BigDecimal;
>> +
>>  import org.osoa.sca.annotations.Remotable;
>>   @Remotable
>>  public interface Checkout {
>> -    void makePayment(double amount, String cardInfo);
>> +    void makePayment(BigDecimal amount, String cardInfo);
>>  }
>>
>> Modified:
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ShoppingCart.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ShoppingCart.java?rev=775672&r1=775671&r2=775672&view=diff
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ShoppingCart.java
>> (original)
>> +++
>> tuscany/sandbox/travelsample/contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ShoppingCart.java
>> Sun May 17 15:23:10 2009
>> @@ -18,11 +18,22 @@
>>  */
>>  package com.tuscanyscatours;
>>  +import java.math.BigDecimal;
>> +import java.util.ArrayList;
>> +import java.util.List;
>> +
>>  public class ShoppingCart implements Checkout, Updates {
>> -    public void makePayment(double amount, String cardInfo) {
>> -        // make payment for trips in cart giving card details
>> +       private static List<String> bookedTrips = new ArrayList<String>();
>> +
>> +    public void makePayment(BigDecimal amount, String cardInfo) {
>> +       System.out.print("Charged $" + amount + " to card " + cardInfo + "
>> for trips" );
>> +       for (String trip : bookedTrips){
>> +               System.out.print(" " + trip);
>> +       }
>> +       System.out.println();
>> +       bookedTrips.clear();
>>     }
>>     public void addTrip(String resCode) {
>> -        // add the booked trip to the cart
>> +        bookedTrips.add(resCode);
>>     }
>>  }
>>
>> Added:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java?rev=775672&view=auto
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>> (added)
>> +++
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>> Sun May 17 15:23:10 2009
>> @@ -0,0 +1,26 @@
>> +/*
>> + * Licensed to the Apache Software Foundation (ASF) under one
>> + * or more contributor license agreements.  See the NOTICE file
>> + * distributed with this work for additional information
>> + * regarding copyright ownership.  The ASF licenses this file
>> + * to you under the Apache License, Version 2.0 (the
>> + * "License"); you may not use this file except in compliance
>> + * with the License.  You may obtain a copy of the License at
>> + * + *   http://www.apache.org/licenses/LICENSE-2.0
>> + * + * Unless required by applicable law or agreed to in writing,
>> + * software distributed under the License is distributed on an
>> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
>> + * KIND, either express or implied.  See the License for the
>> + * specific language governing permissions and limitations
>> + * under the License.    + */
>> +package scatours;
>> +
>> +import org.osoa.sca.annotations.Remotable;
>> +
>> +@Remotable
>> +public interface Bookings {
>> +    String newBooking(String trip, int people);
>> +}
>>
>> Propchange:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>>
>> ------------------------------------------------------------------------------
>>    svn:eol-style = native
>>
>> Propchange:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Bookings.java
>>
>> ------------------------------------------------------------------------------
>>    svn:keywords = Rev Date
>>
>> Added:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java?rev=775672&view=auto
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>> (added)
>> +++
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>> Sun May 17 15:23:10 2009
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Licensed to the Apache Software Foundation (ASF) under one
>> + * or more contributor license agreements.  See the NOTICE file
>> + * distributed with this work for additional information
>> + * regarding copyright ownership.  The ASF licenses this file
>> + * to you under the Apache License, Version 2.0 (the
>> + * "License"); you may not use this file except in compliance
>> + * with the License.  You may obtain a copy of the License at
>> + * + *   http://www.apache.org/licenses/LICENSE-2.0
>> + * + * Unless required by applicable law or agreed to in writing,
>> + * software distributed under the License is distributed on an
>> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
>> + * KIND, either express or implied.  See the License for the
>> + * specific language governing permissions and limitations
>> + * under the License.    + */
>> +package scatours;
>> +
>> +import java.math.BigDecimal;
>> +
>> +import org.osoa.sca.annotations.Remotable;
>> +
>> +@Remotable
>> +public interface Checkout {
>> +    void makePayment(BigDecimal amount, String cardInfo);
>> +}
>>
>> Propchange:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>>
>> ------------------------------------------------------------------------------
>>    svn:eol-style = native
>>
>> Propchange:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/Checkout.java
>>
>> ------------------------------------------------------------------------------
>>    svn:keywords = Rev Date
>>
>> Modified:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/LaunchNode.java
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/LaunchNode.java?rev=775672&r1=775671&r2=775672&view=diff
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/LaunchNode.java
>> (original)
>> +++
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/java/scatours/LaunchNode.java
>> Sun May 17 15:23:10 2009
>> @@ -19,6 +19,9 @@
>>   package scatours;
>>  +import java.io.IOException;
>> +import java.math.BigDecimal;
>> +
>>  import org.apache.tuscany.sca.node.SCAClient;
>>  import org.apache.tuscany.sca.node.SCAContribution;
>>  import org.apache.tuscany.sca.node.SCANode;
>> @@ -33,16 +36,24 @@
>>     // OK for development but you must launch the node from this module
>>   public static void launchFromFileSystemDir(){
>>         try {
>> -            SCANode node =
>> SCANodeFactory.newInstance().createSCANode("scatours.composite", +
>>  SCANode node = SCANodeFactory.newInstance().createSCANode(null,
>>     new SCAContribution("goodvaluetrips",
>> "../../contributions/introducing-goodvaluetrips-contribution/target/classes"),
>> -                new SCAContribution("tuscanyscatours",
>> "../../contributions/introducing-tuscanyscatours-contribution/target/classes"),
>> -                new SCAContribution("client",
>> "../../contributions/introducing-client-contribution/target/classes"),
>> -                new SCAContribution("launcher", "./target/classes"));
>> +                new SCAContribution("tuscanyscatours",
>> "../../contributions/introducing-tuscanyscatours-contribution/target/classes"));
>> +
>>             node.start();
>>  -            Runnable runner =
>> ((SCAClient)node).getService(Runnable.class, "TestClient/Runnable");
>> -            runner.run();
>> +            Bookings bookings =
>> ((SCAClient)node).getService(Bookings.class, +
>>                               "TripBooking/Bookings");
>>  +            System.out.println("Trip boooking code = " + +
>>                    bookings.newBooking("FS1APR4", 1));
>> +            +            Checkout checkout =
>> ((SCAClient)node).getService(Checkout.class, +
>>                               "ShoppingCart/Checkout");
>> +            +            checkout.makePayment(new BigDecimal("1995.00"),
>> "1234567843218765 10/10");
>> +            +                         node.stop();
>>                     } catch (Throwable th) {
>>
>> Modified:
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/resources/scatours.composite
>> URL:
>> http://svn.apache.org/viewvc/tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/resources/scatours.composite?rev=775672&r1=775671&r2=775672&view=diff
>>
>> ==============================================================================
>> ---
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/resources/scatours.composite
>> (original)
>> +++
>> tuscany/sandbox/travelsample/launchers/introducing-launcher/src/main/resources/scatours.composite
>> Sun May 17 15:23:10 2009
>> @@ -23,7 +23,7 @@
>>     xmlns:tours="http://tuscanyscatours.com/"
>>     xmlns:trips="http://goodvaluetrips.com/"
>>     name="scatours">
>> -
>> +         <include name="client:Client" />
>>     <include name="tours:Tours" />
>>     <include name="trips:Trips" />
>>
>>
>>
>>
>
>
>

I'm trying to reduce the number of moving parts. As it stood to test
the combination of

introducing-goodvaluetrips-contribution
introducing-tuscanyscatours-contribution

required

LaunchNode
introducing-launcher as a contribution
introducing-client-contribution

which to me says Tuscany is hard to use and requires many artifacts to
get it up and running.

If we have a generic launcher then I could see the benefit of a client
contribution. But in this case we don't as the launcher is specific to
this sample so it might as well contain the test code directly.

Now if your argument is that we want to test remote bindings on the
tuscanyscatours service the that is a good justification. We don't in
this case but I could buy it from the "consistency with other samples"
point of view. I would however suggest that, if a client contribution
is required, we add it to the launcher module by extending the
launcher contribution to remove the requirement to have as many test
contributions are there are contributions to test.

Simon

Re: svn commit: r775672 - in /tuscany/sandbox/travelsample: contributions/introducing-client-contribution/src/main/java/scatours/client/ contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ launchers/introducing-l

Posted by Simon Nash <na...@apache.org>.
Simon Nash wrote:
> Simon Laws wrote:
>>> In the r775672 version of this code there is no launcher contribution
>>> passed to createSCANode(), so I'm not sure what you mean by "extending
>>> the launcher contribution".  For the reasons I gave above, I think
>>> client test code needs to be in its own contribution.  I can think of
>>> a few options for where these test contributions would be located:
>>>
>>> 1. Leave test contributions in the main contributions module, with a
>>>   naming convention to distinguish them from provider contributions.
>>>
>>> 2. Create a new module for test contributions.  This doesn't feel right
>>>   from a modularity perspective.
>>>
>>> 3. Put test contributions into launcher modules.  If we do this, I think
>>>   we should rename the current "xxx-launcher" modules to something else
>>>   like "xxx-test" to reflect their extended role.
>>>
>>> I would be OK with 1 or 3.
>>
>> Ok, if we reduce this to the one contribution that holds the test code
>> I'm +1 for that. Of the options here then, 1 seems most
>> straighforward.
>>
>> Simon
>>
>>
> Thanks, I'll make these changes to introducing-launcher so we can see
> how it looks.
> 
>   Simon
> 
I have made these updates.  The introducing-launcher module is no
longer a contribution, and now only contains two files:
   src/main/java/scatours/IntroducingLauncher.java
   src/test/java/scatours/IntroducingTestCase.java

I also updated build.xml so that it does not create a jar file
containing IntroducingLauncher.class (not very useful with only one
class).  I don't know how to tweak the pom.xml to do this, so the
mvn build is still creating an scatours-introducing-launcher.jar file.

   Simon



Re: svn commit: r775672 - in /tuscany/sandbox/travelsample: contributions/introducing-client-contribution/src/main/java/scatours/client/ contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ launchers/introducing-l

Posted by Simon Nash <na...@apache.org>.
Simon Laws wrote:
>> In the r775672 version of this code there is no launcher contribution
>> passed to createSCANode(), so I'm not sure what you mean by "extending
>> the launcher contribution".  For the reasons I gave above, I think
>> client test code needs to be in its own contribution.  I can think of
>> a few options for where these test contributions would be located:
>>
>> 1. Leave test contributions in the main contributions module, with a
>>   naming convention to distinguish them from provider contributions.
>>
>> 2. Create a new module for test contributions.  This doesn't feel right
>>   from a modularity perspective.
>>
>> 3. Put test contributions into launcher modules.  If we do this, I think
>>   we should rename the current "xxx-launcher" modules to something else
>>   like "xxx-test" to reflect their extended role.
>>
>> I would be OK with 1 or 3.
> 
> Ok, if we reduce this to the one contribution that holds the test code
> I'm +1 for that. Of the options here then, 1 seems most
> straighforward.
> 
> Simon
> 
> 
Thanks, I'll make these changes to introducing-launcher so we can see
how it looks.

   Simon



Re: svn commit: r775672 - in /tuscany/sandbox/travelsample: contributions/introducing-client-contribution/src/main/java/scatours/client/ contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ launchers/introducing-l

Posted by Simon Laws <si...@googlemail.com>.
> In the r775672 version of this code there is no launcher contribution
> passed to createSCANode(), so I'm not sure what you mean by "extending
> the launcher contribution".  For the reasons I gave above, I think
> client test code needs to be in its own contribution.  I can think of
> a few options for where these test contributions would be located:
>
> 1. Leave test contributions in the main contributions module, with a
>   naming convention to distinguish them from provider contributions.
>
> 2. Create a new module for test contributions.  This doesn't feel right
>   from a modularity perspective.
>
> 3. Put test contributions into launcher modules.  If we do this, I think
>   we should rename the current "xxx-launcher" modules to something else
>   like "xxx-test" to reflect their extended role.
>
> I would be OK with 1 or 3.

Ok, if we reduce this to the one contribution that holds the test code
I'm +1 for that. Of the options here then, 1 seems most
straighforward.

Simon

Re: svn commit: r775672 - in /tuscany/sandbox/travelsample: contributions/introducing-client-contribution/src/main/java/scatours/client/ contributions/introducing-tuscanyscatours-contribution/src/main/java/com/tuscanyscatours/ launchers/introducing-l

Posted by Simon Nash <na...@apache.org>.
Simon Laws wrote:
> On Wed, May 20, 2009 at 1:20 PM, Simon Nash <na...@apache.org> wrote:
>> I don't agree with the change to introducing-launcher in this commit
>> that puts the client code inline and eliminates the use of the
>> introducing-client-contribution.
>>
>> Having the client code as part of the launcher is acceptable for
>> jumpstart-launcher to keep this sample very simple, but for the other
>> samples I think the pattern of having a separate contribution for
>> test client code is a good pattern that we should use.
>>
>>  Simon
>>
>> slaws@apache.org wrote:
>>> Author: slaws
>>> Date: Sun May 17 15:23:10 2009
>>> New Revision: 775672
>>>
 >>> (cut)
>>>
>>
>>
> 
> I'm trying to reduce the number of moving parts. As it stood to test
> the combination of
> 
> introducing-goodvaluetrips-contribution
> introducing-tuscanyscatours-contribution
> 
> required
> 
> LaunchNode
> introducing-launcher as a contribution
> introducing-client-contribution
> 
I agree that we should not need to have separate moving parts for
LaunchNode and introducing-launcher as a contribution.

I thought the Node APIs required this separation, but from looking
at the changes in r775672 I see that it is possible to pass null as
the first argument to createSCANode() rather than a composite URI.
This means we can eliminate scatours.composite and we can also
eliminate having introducing-launcher as a separate contribution.
These are both useful simplifications.

> which to me says Tuscany is hard to use and requires many artifacts to
> get it up and running.
> 
> If we have a generic launcher then I could see the benefit of a client
> contribution. But in this case we don't as the launcher is specific to
> this sample so it might as well contain the test code directly.
> 
I don't agree with this.  The launcher should do what it says, i.e.,
launch the sample using the necessary contributions.  It should not
do any more than this.  If client test code is needed, this should
be provided separately from the launcher because it is not logically
part of the launcher.  The travel sample should be illustrating best
practice, and putting client code into the launcher class itself is
very definitely not best practice.  IMO the client code should be a
contribution and not loaded from the classpath, so that imports from
the provider contributions can work correctly.

If we keep client code and launcher code separate, it may be possible
to come up with a generic launcher approach.  I think this would be a
very useful simplification.  This could be part of the travel sample,
or perhaps we could come up with something that could be added to
the Tuscany codebase.

> Now if your argument is that we want to test remote bindings on the
> tuscanyscatours service the that is a good justification. We don't in
> this case but I could buy it from the "consistency with other samples"
> point of view. I would however suggest that, if a client contribution
> is required, we add it to the launcher module by extending the
> launcher contribution to remove the requirement to have as many test
> contributions are there are contributions to test.
> 
In the r775672 version of this code there is no launcher contribution
passed to createSCANode(), so I'm not sure what you mean by "extending
the launcher contribution".  For the reasons I gave above, I think
client test code needs to be in its own contribution.  I can think of
a few options for where these test contributions would be located:

1. Leave test contributions in the main contributions module, with a
    naming convention to distinguish them from provider contributions.

2. Create a new module for test contributions.  This doesn't feel right
    from a modularity perspective.

3. Put test contributions into launcher modules.  If we do this, I think
    we should rename the current "xxx-launcher" modules to something else
    like "xxx-test" to reflect their extended role.

I would be OK with 1 or 3.

   Simon

> Simon
> 
>