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-----
From: eclipselink-dev-bounces@xxxxxxxxxxx
[mailto:eclipselink-dev-bounces@xxxxxxxxxxx] On Behalf Of christopher
delahunt
Sent: Friday, February 05, 2010 9:50 AM
To: Dev mailing list for Eclipse Persistence Services
Subject: Re: [eclipselink-dev] 2.1 Design doc for JMS-MDB cache
coordination
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
------------------------------------------------------------------------
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev