Andreas, On Mon, May 24, 2010 at 1:02 AM, Andreas Veithen <[email protected]>wrote:
> On Sun, May 23, 2010 at 20:58, Hiranya Jayathilaka <[email protected]> > wrote: > > Hi Andreas, > > > > On Sun, May 23, 2010 at 11:13 PM, Andreas Veithen < > [email protected] > >> wrote: > > > >> On Sun, May 23, 2010 at 19:14, Hiranya Jayathilaka < > [email protected]> > >> wrote: > >> > Hi Andreas, > >> > > >> > I didn't quite understand the problem caused by adding this > dependency. > >> > Could you please explain it a little more? > >> > >> Having a dependency between the classes in > >> org.apache.axis2.transport.base.datagram and SocketAddress is a > >> violation of the design as would be a dependency between > >> AbstractTransportListener and javax.jms.ConnectionFactory. > >> > > > > I disagree. ConnectionFactory is a JMS specific class. It has no reason > to > > be in a high level interface like AbstractTransportListener. But > > SocketAddress is a more general class that can be used to model any > > transport address. Here's the introduction of the SocketAddress class > from > > the javadocs: > > > > "This class represents a Socket Address with no protocol attachment. As > an > > abstract class, it is meant to be subclassed with a specific, protocol > > dependent, implementation." > The bottom line is, it acts as a template for transport protocol addresses. Isn't that what we want when it comes to sending back responses? We just need to hold on to the protocol address of the source machine, right? > > > > As you can see it is abstract and exists at a very high level. It is not > > bound to any transport protocol implementation. So IMO there is nothing > > wrong in having a dependency to such a high level interface from the base > > transport module. > > You should read the entire Javadoc: > > "It provides an immutable object used by sockets for binding, > connecting, or as returned values." > > "Socket" is a reference to OSI layer 4. Using SocketAddress in > org.apache.axis2.transport.base.datagram would thus result in a design > that makes these classes only appropriate as a basis for transports > that exchange messages directly over an OSI layer 4 protocol. While I agree with you on the theoretical definition of sockets, practically speaking, the SocketAddress class can be subclassed in ways that are not specific to the transport layer in the OSI model. BTW here's how the concept of Socket is defined in the javadocs: "This class implements client sockets (also called just "sockets"). A socket is an endpoint for communication between two machines." So the concept of sockets can be used to model virtually any protocol endpoint in Java. > BTW, it's the first time that I heard somebody suggesting to create > new subclasses of SocketAddress (and that are not usable with > Socket#bind or Socket#connect). Do you know of anybody doing this? > Not that I know of. But if needed it can be done easily. As mentioned in the javadocs it is 'meant' to be subclassed. The SocketAddress class does not contain any methods either. So its implementation could be completely transport specific as desired by the developer. > > >> > SocketAddress is a high level abstract class. The widely used > >> > InetSocketAddress class is derived from that. IMO almost any transport > >> can > >> > make use of that. > >> > >> No, the pipe transport in Synapse is based on > >> org.apache.axis2.transport.base.datagram and for this one, > >> SocketAddress has no meaning. > >> > >> > If not they can simply pass null. > >> > >> Yes, we could also add a javax.jms.ConnectionFactory attribute in > >> AbstractTransportListener and let all transport except the JMS > >> transport set it to null :-) > >> > > > > As I've already mentioned, ConnectionFactory is JMS specific whereas > > SocketAddress is generic. These two cannot be compared with each other. > > SocketAddress is OSI layer 4 specific... > Let's say we really got rid of the SocketAddress references from the base transport. What is the alternative you are suggesting for handling request-response scenarios with transports like UDP? Thanks, Hiranya > >> > >> > Or we can even introduce > >> > a new receive method to the AbstractDatagramTransportListener class > which > >> > does not accept a SocketAddress argument. WDYT? > >> > >> I haven't looked into the reasons for the appearance of SocketAddress > >> in that code, so I don't know yet what is the correct way to get rid > >> of it. > >> > > > > If I understood correct a SocketAddress is used to keep track of the > source > > address for each message/datagram. This address is then used to send > > responses back. So it has a very good reason to be in the base transport. > > Since it is abstract each transport implementation can have their own > > implementations of the SocketAddress to model transport specific source > > addresses (although in UDP transport we seem to be using the default > > InetSocketAddress implementation which works fine in case of UDP). > > > > Given these reasons I don't think we have to completely get rid of this > from > > the base transport. If we must we can add a new receive method which does > > not care about source addresses. This method can be used by transports > like > > the Synapse pipe transport. Thoughts? > > > > Thanks, > > Hiranya > > > > > >> > Thanks, > >> > Hiranya > >> > > >> > On Sun, May 23, 2010 at 8:30 PM, Andreas Veithen (JIRA) < > [email protected] > >> >wrote: > >> > > >> >> Change in r887108 breaks design of > >> org.apache.axis2.transport.base.datagram > >> >> > >> > --------------------------------------------------------------------------- > >> >> > >> >> Key: WSCOMMONS-543 > >> >> URL: > >> https://issues.apache.org/jira/browse/WSCOMMONS-543 > >> >> Project: WS-Commons > >> >> Issue Type: Bug > >> >> Components: Transport > >> >> Reporter: Andreas Veithen > >> >> Priority: Blocker > >> >> Fix For: Transports 1.1 > >> >> > >> >> > >> >> The change in r887108 [1] introduced a dependency between > >> >> AbstractDatagramTransportListener and SocketAddress. This badly > breaks > >> the > >> >> design of the classes in org.apache.axis2.transport.base.datagram, > since > >> >> they are not only used for UDP (which is the only derived transport > that > >> >> uses SocketAddress). See also [2] and [3]. This needs to be fixed for > >> the > >> >> next release. > >> >> > >> >> [1] http://svn.apache.org/viewvc?rev=887108&view=rev > >> >> [2] http://markmail.org/thread/ancuyeuxefi6ttnn > >> >> [3] http://markmail.org/message/nls3mpg2g4ec3uq5 > >> >> > >> >> -- > >> >> This message is automatically generated by JIRA. > >> >> - > >> >> You can reply to this email to add a comment to the issue online. > >> >> > >> >> > >> > > >> > > >> > -- > >> > Hiranya Jayathilaka > >> > Senior Software Engineer; > >> > WSO2 Inc.; http://wso2.org > >> > E-mail: [email protected]; Mobile: +94 77 633 3491 > >> > Blog: http://techfeast-hiranya.blogspot.com > >> > > >> > > > > > > > > -- > > Hiranya Jayathilaka > > Senior Software Engineer; > > WSO2 Inc.; http://wso2.org > > E-mail: [email protected]; Mobile: +94 77 633 3491 > > Blog: http://techfeast-hiranya.blogspot.com > > > -- Hiranya Jayathilaka Senior Software Engineer; WSO2 Inc.; http://wso2.org E-mail: [email protected]; Mobile: +94 77 633 3491 Blog: http://techfeast-hiranya.blogspot.com
