| RE: [eclipselink-dev] 2.1 Design doc for JMS-MDB cache coordination |
|
Everything is possible if we modify the release certified
EclipseLink jar, or copy-paste too many lines of code. The week after your new
code is release we want to avoid to have to copy/paste it to do what we already
know we need to do. We want to avoid: - modify/override more than one class, when only one
class is involved in the logic modified. When concrete class are hardcoded we
are forced to modify many class for modifying one line of code. - copy-paste the code of a long method to change one line.
That had maintenance risk to need to paste code. Some time the pasted code try
to access private member that is not accessible from the derived class. You can "test" the customization flexibility,
by trying to meet what was described here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=254287 The short solution is no hard coding of concrete class outside
method factory, and small methods. Let's walk through it: The goal is to replace: TopicConnection topicConnection =
connectionFactory.createTopicConnection(); With: TopicConnection topicConnection =
connectionFactory.createTopicConnection(String userName, String password); We have it 2 places below. First thing to remark is that
the method where it is found has some copy/paste (next line after
createTopConnection is always the same) and different block that could be
refactored in their own methods. Knowing that there is an interest to customize
createTopicConnection, it should be in a method by itself. Then, what is
involved of overriding JMSTopicRemoteConnection? Does [new
JMSTopicRemoteConnection] is hardcoded somewhere, is it a long method?
What is involved to specify a derived class? Do we end-up with cascade
modifications of factories? public JMSTopicRemoteConnection(RemoteCommandManager
rcm, TopicConnectionFactory topicConnectionFactory, Topic topic, boolean
isLocalConnectionBeingCreated, boolean reuseJMSTopicPublisher) throws
JMSException { super(rcm); + this.topicConnectionFactory =
topicConnectionFactory; this.topic = topic; this.isLocal = isLocalConnectionBeingCreated;
rcm.logDebug("creating_broadcast_connection", getInfo()); try { if(isLocalConnectionBeingCreated) { // it's a local connection +
this.topicConnection = topicConnectionFactory.createTopicConnection(); + this.topicSession
= topicConnection.createTopicSession(false,
javax.jms.Session.AUTO_ACKNOWLEDGE); this.subscriber =
topicSession.createSubscriber(topic); + topicConnection.start();
rcm.logDebug("broadcast_connection_created", getInfo()); rcm.getServerPlatform().launchContainerRunnable(this); + } else if (reuseJMSTopicPublisher) { + // it's an external connection and is
set to reuse the TopicPublisher (legacy) + this.topicConnection
= topicConnectionFactory.createTopicConnection(); +
this.topicSession = topicConnection.createTopicSession(false,
javax.jms.Session.AUTO_ACKNOWLEDGE); this.publisher =
topicSession.createPublisher(topic);
rcm.logDebug("broadcast_connection_created", getInfo()); + } //else bug214534: it's an external
connection, with TopicConnections, sessions and publishers created as needed } catch (JMSException ex) {
rcm.logDebug("failed_to_create_broadcast_connection", getInfo()); close(); throw ex; } } We also need flexibility on injecting our own CommandProcessor. -----Original Message----- Hello Sebastien, I am not sure what you would need to make it easier to
customize, other than more documentation, which is always needed.
TransportManager is abstract rather than an interface, but should still be
easy to extend it or any of its subclasses with your own implementations,
and the configuration (Sessions.xml or JPA properties) already
allows specifying a user defined class to be used as the TransportManager. While it might be beyond the scope of the current
feature, please let me know if there are any specific changes or details that
are missing. I would like to check in and shift to working on something
else, but I'll try to work them in, or file a new bug/feature get them
into EclipseLink later on. Best Regards, Chris Sebastien Tardif wrote: > > Thanks for the improvement. > > > > However, I still don't see any attempt to allow easy
customization via > overriding, like mentioned here: > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=254287 > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=244209 > > > > I'm not asking for anything fancy. Using Factory
Method pattern like > mentioned in ["Design Pattern", 1994],
would be a big improvement. > > > > -----Original Message----- > From: eclipselink-dev-bounces@xxxxxxxxxxx > [mailto:eclipselink-dev-bounces@xxxxxxxxxxx] On
Behalf Of christopher > delahunt > Sent: Thursday, February 04, 2010 3:15 PM > To: Dev mailing list for Eclipse Persistence
Services > Subject: Re: [eclipselink-dev] 2.1 Design doc for
JMS-MDB cache > coordination > > > > New patch uploaded for > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=214534 > > that incorporates all the feedback received. > > > > Please review and provide any additional feedback. > > > > thanks in advance, > > Chris > > > > christopher delahunt wrote: > > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=214534 > > > Patch uploaded for review and feedback > > > > > > > > > Best Regards, > > > Chris > > > > > > christopher delahunt wrote: > > >> Design documentation can be found here: > > >> > > >> >
http://wiki.eclipse.org/EclipseLink/Development/2.1/JMSCacheCoordinationUsingMDB
> > > >> >
<http://wiki.eclipse.org/EclipseLink/Development/JPA_2.0/undelimited_identifiers>
> > > >> > > >> > > >> Please provide feedback either by replying
to this email or using the > > >> Wiki discussion page. > > >> > > >> Thanks, > > >> Chris > > >>
_______________________________________________ > > >> eclipselink-dev mailing list > > >> eclipselink-dev@xxxxxxxxxxx > > >>
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev > > > _______________________________________________ > > > eclipselink-dev mailing list > > > eclipselink-dev@xxxxxxxxxxx > > >
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev > > _______________________________________________ > > eclipselink-dev mailing list > > eclipselink-dev@xxxxxxxxxxx > >
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev > >
------------------------------------------------------------------------ > > _______________________________________________ > eclipselink-dev mailing list > eclipselink-dev@xxxxxxxxxxx >
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev > _______________________________________________ eclipselink-dev mailing list eclipselink-dev@xxxxxxxxxxx https://dev.eclipse.org/mailman/listinfo/eclipselink-dev |