Hi,
we had a downstream bug that boiled down to a failing Channel to
ClonedChannel cast. The cast was failing because Hibernate proxy objects
for subclasses are guaranteed to implement all properties and methods,
so that isClone() correctly returned true, but do not necessarily
respect the class hierarchy: in that case a CGLIB generated class object
was returned and it did not extend ClonedChannel.
This has been documented as a "pitfall 2" in the following article:
http://java-success.blogspot.it/2012/09/understanding-hibernate-proxy-objects.html
While fixing the bug we found some other places in the code where the
same problem could happen, since the implementation assumes some object
can be cast to ClonedChannel. We then purpose this patch to avoid
relying on that mechanism, using a separate method that will work
regardless if the Channel is actually a Hibernate proxy or not.
Any comment is welcome!
Regards,
--
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany
>From e5d4e4a2b5e19524ae3c6b8e8e16a9825d8d28e6 Mon Sep 17 00:00:00 2001
From: Silvio Moioli <[email protected]>
Date: Fri, 7 Jun 2013 16:02:00 +0200
Subject: [PATCH] Avoid relying on types returned by Hibernate
Conflicts:
java/code/src/com/redhat/rhn/domain/channel/Channel.java
---
java/code/src/com/redhat/rhn/domain/channel/Channel.java | 7 +++++++
java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java | 1 +
java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java | 2 +-
.../frontend/action/channel/manage/ChannelPackagesAddAction.java | 2 +-
java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java | 6 ++----
java/code/src/com/redhat/rhn/manager/system/SystemManager.java | 3 +--
6 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/domain/channel/Channel.java b/java/code/src/com/redhat/rhn/domain/channel/Channel.java
index f091476..bcb06fa 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/Channel.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/Channel.java
@@ -836,4 +836,11 @@ public class Channel extends BaseDomainHelper implements Comparable {
}
return checksumType.getLabel();
}
+
+ /**
+ * @return the original Channel the channel was cloned from
+ */
+ public Channel getOriginal() {
+ throw new UnsupportedOperationException();
+ }
}
diff --git a/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java b/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
index 22b7e85..c130dd9 100644
--- a/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
+++ b/java/code/src/com/redhat/rhn/domain/channel/ClonedChannel.java
@@ -27,6 +27,7 @@ public class ClonedChannel extends Channel {
/**
* @return the original Channel the channel was cloned from
*/
+ @Override
public Channel getOriginal() {
return original;
}
diff --git a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
index d4ce27d..78db9fe 100644
--- a/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
+++ b/java/code/src/com/redhat/rhn/domain/errata/ErrataFactory.java
@@ -263,7 +263,7 @@ public class ErrataFactory extends HibernateFactory {
throw new InvalidChannelException("Cloned channel expected: " +
chan.getLabel());
}
- Channel original = ((ClonedChannel) chan).getOriginal();
+ Channel original = chan.getOriginal();
// see BZ 805714, if we are a clone of a clone the 1st clone
// may not have the errata we want
while (original.isCloned() &&
diff --git a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
index b074956..8dcbd5e 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/channel/manage/ChannelPackagesAddAction.java
@@ -116,7 +116,7 @@ public class ChannelPackagesAddAction extends RhnAction {
//If a channel isn't selected, select one smartly
if (selectedChan == null) {
if (chan.isCloned()) {
- selectedChan = ((ClonedChannel) chan).getOriginal().getId().toString();
+ selectedChan = chan.getOriginal().getId().toString();
}
else {
selectedChan = ORPHAN_PACKAGES;
diff --git a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
index 3d8fbef..5c255e7 100644
--- a/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
+++ b/java/code/src/com/redhat/rhn/manager/channel/ChannelManager.java
@@ -2568,8 +2568,7 @@ public class ChannelManager extends BaseManager {
if (c.isCloned()) {
Map params = new HashMap();
params.put("cid", c.getId());
- ClonedChannel cc = (ClonedChannel) c;
- params.put("ocid", cc.getOriginal().getId());
+ params.put("ocid", c.getOriginal().getId());
SelectMode m = ModeFactory.getMode("Errata_queries",
"list_errata_needing_sync");
return m.execute(params);
@@ -2593,8 +2592,7 @@ public class ChannelManager extends BaseManager {
Map params = new HashMap();
params.put("cid", c.getId());
params.put("set_label", setLabel);
- ClonedChannel cc = (ClonedChannel) c;
- params.put("ocid", cc.getOriginal().getId());
+ params.put("ocid", c.getOriginal().getId());
SelectMode m = ModeFactory.getMode("Errata_queries",
"list_packages_needing_sync_from_set");
return m.execute(params);
diff --git a/java/code/src/com/redhat/rhn/manager/system/SystemManager.java b/java/code/src/com/redhat/rhn/manager/system/SystemManager.java
index 3df7720..899ca39 100644
--- a/java/code/src/com/redhat/rhn/manager/system/SystemManager.java
+++ b/java/code/src/com/redhat/rhn/manager/system/SystemManager.java
@@ -1704,8 +1704,7 @@ public class SystemManager extends BaseManager {
Channel base = server.getBaseChannel();
if (base != null && base.isCloned()) {
- ClonedChannel clonedBase = (ClonedChannel) base;
- base = clonedBase.getOriginal();
+ base = base.getOriginal();
}
if ((base != null) &&
--
1.8.1.4
_______________________________________________
Spacewalk-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/spacewalk-devel
