You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2011/09/23 23:57:36 UTC
Re: svn commit: r1174181 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/ha/session/BackupManager.java java/org/apache/catalina/ha/session/ClusterManagerBase.java
java/org/apache/catalina/ha/session/DeltaManager.java webapps/docs/changelog.xm
Reviewing Eclipse warnings I noticed a bug in this commit.
2011/9/22 <rj...@apache.org>:
> Author: rjung
> Date: Thu Sep 22 15:01:08 2011
> New Revision: 1174181
>
> URL: http://svn.apache.org/viewvc?rev=1174181&view=rev
> Log:
> - Pull up members "cluster" and "notifyListenersOnReplication"
> to common base class.
> - Pull up common clone code to base class.
> - Add sessionAttributeFilter to clone method
> - Reduce visibility of notifyListenersOnReplication
>
> Backport of r1173088, r1173090, r1173461 from trunk.
>
> Modified:
> tomcat/tc7.0.x/trunk/ (props changed)
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/BackupManager.java
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/ClusterManagerBase.java
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaManager.java
> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
(..)
> @@ -259,12 +225,9 @@ public class BackupManager extends Clust
> @Override
> public ClusterManager cloneFromTemplate() {
> BackupManager result = new BackupManager();
> + clone(result);
> result.mExpireSessionsOnShutdown = mExpireSessionsOnShutdown;
> - result.name = "Clone-from-"+name;
> - result.cluster = cluster;
> - result.notifyListenersOnReplication = notifyListenersOnReplication;
> result.mapSendOptions = mapSendOptions;
> - result.maxActiveSessions = maxActiveSessions;
> result.rpcTimeout = rpcTimeout;
> return result;
> }
> + protected void clone(ClusterManagerBase copy) {
> + copy.name = "Clone-from-" + getName();
The above assignment is wrong.
It assigns to ManagerBase.name which is a static field.
(Why that field exists and why it is not final is another question).
The old code was assigning the value to BackupManager.name, which is
an instance field.
> + copy.cluster = getCluster();
> + copy.maxActiveSessions = getMaxActiveSessions();
> + copy.notifyListenersOnReplication = isNotifyListenersOnReplication();
> + copy.setSessionAttributeFilter(getSessionAttributeFilter());
> + }
In Eclipse the warning was:
The static field ManagerBase.name should be accessed in a static way
Also there are several missing @Override annotations in the new methods.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1174181 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/ha/session/BackupManager.java
java/org/apache/catalina/ha/session/ClusterManagerBase.java java/org/apache/catalina/ha/session/DeltaManager.java
webapps/docs/changelog.xm
Posted by Rainer Jung <ra...@kippdata.de>.
On 24.09.2011 13:57, Konstantin Kolinko wrote:
> 2011/9/24 Rainer Jung <ra...@kippdata.de>:
>>
>> Fixed also, ported to TC 7, updated TC 6 patch for Override, kept your
>> vote in STATUS.txt.
>>
>
> OK, good. Thank you.
>
> BTW, typo in JavaDoc in the patch.
> @return true is attribute should not be replicated
> @return true if attribute should not be replicated
Will fix when applying.
> There are enough votes to apply that patch to TC6.
I know, thanks for voting, just didn't want to rush and give enough time
to review. Will commit later today.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1174181 - in /tomcat/tc7.0.x/trunk: ./
java/org/apache/catalina/ha/session/BackupManager.java java/org/apache/catalina/ha/session/ClusterManagerBase.java
java/org/apache/catalina/ha/session/DeltaManager.java webapps/docs/changelog.xm
Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/9/24 Rainer Jung <ra...@kippdata.de>:
>
> Fixed also, ported to TC 7, updated TC 6 patch for Override, kept your
> vote in STATUS.txt.
>
OK, good. Thank you.
BTW, typo in JavaDoc in the patch.
@return true is attribute should not be replicated
@return true if attribute should not be replicated
There are enough votes to apply that patch to TC6.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1174181 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/ha/session/BackupManager.java
java/org/apache/catalina/ha/session/ClusterManagerBase.java java/org/apache/catalina/ha/session/DeltaManager.java
webapps/docs/changelog.xm
Posted by Rainer Jung <ra...@kippdata.de>.
On 23.09.2011 23:57, Konstantin Kolinko wrote:
> Reviewing Eclipse warnings I noticed a bug in this commit.
Argh.
>> + protected void clone(ClusterManagerBase copy) {
>> + copy.name = "Clone-from-" + getName();
>
> The above assignment is wrong.
> It assigns to ManagerBase.name which is a static field.
> (Why that field exists and why it is not final is another question).
... which I'mnot going to answer ...
> The old code was assigning the value to BackupManager.name, which is
> an instance field.
OK, I used the setter instead now.
> In Eclipse the warning was:
> The static field ManagerBase.name should be accessed in a static way
>
> Also there are several missing @Override annotations in the new methods.
Fixed also, ported to TC 7, updated TC 6 patch for Override, kept your
vote in STATUS.txt.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org