Re: [Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null
On Wed, Oct 05, 2011 at 05:04:28PM -0300, Marcelo Moreira de Mello wrote: > On 10/05/2011 04:03 AM, Jan Pazdziora wrote: > > Nack. > > > > The first part of your patch reverts change for bug 452956. Unless we > > know *exactly* why that change was done for that bug (in other words, > > reproduce that original bug, without that patch), we risk a regression > > here. > > > > The second part of your patch seems like a noop to me since > > StringUtils.defaultString is defined as > > > > Returns either the passed in String, or if the String is null, > > an empty String (""). > > > > already. > > After reviewing the patch follow attach a better looking patch for > this bug. Doing our tests, this patch does not include a regression for > bug 452956. > .../frontend/xmlrpc/serializer/DmiSerializer.java |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git > a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java > b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java > index 7fb29ad..1710c2d 100644 > --- > a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java > +++ > b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java > @@ -60,7 +60,7 @@ public class DmiSerializer implements > XmlRpcCustomSerializer { > Dmi dmi = (Dmi) value; > > if (dmi.getSystem() == null) { > -return; > +dmi.setSystem(""); > } > > bean.add("vendor", StringUtils.defaultString(dmi.getVendor())); I still don't think the patch is correct. There is no point in dmi.setSystem'ing the system -- the StringUtils.defaultString will do the trick for the output, without modifying the value, which could potentially cause hibernate want to store it back to the database. I believe the correct fix is to remove that if (dmi.getSystem() == null) { test and branch altogether but if you do that, you need to find out what is the alternate fix for the bug 452956 in the first place. It might be the bios thing, it might be something different. -- Jan Pazdziora Principal Software Engineer, Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null
On 10/05/2011 04:03 AM, Jan Pazdziora wrote: > Nack. > > The first part of your patch reverts change for bug 452956. Unless we > know *exactly* why that change was done for that bug (in other words, > reproduce that original bug, without that patch), we risk a regression > here. > > The second part of your patch seems like a noop to me since > StringUtils.defaultString is defined as > > Returns either the passed in String, or if the String is null, > an empty String (""). > > already. Hello, After reviewing the patch follow attach a better looking patch for this bug. Doing our tests, this patch does not include a regression for bug 452956. I created a test package and the API worked as expected. Best, mmello -- Marcelo Moreira de Mello From: Marcelo Moreira de Mello Date: Wed, 5 Oct 2011 16:58:41 -0300 Subject: [PATCH] 741476 - fixed API system.getDmi() method to no abort the operation when system is NULL --- .../frontend/xmlrpc/serializer/DmiSerializer.java |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java index 7fb29ad..1710c2d 100644 --- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java +++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java @@ -60,7 +60,7 @@ public class DmiSerializer implements XmlRpcCustomSerializer { Dmi dmi = (Dmi) value; if (dmi.getSystem() == null) { -return; +dmi.setSystem(""); } bean.add("vendor", StringUtils.defaultString(dmi.getVendor())); -- 1.7.6.4 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null
Hello Jan, Reviewing it.. Hope to provide an update into this ASAP. Thank you for heads up. Working into this. Best, mmello Marcelo Moreira de Mello RHCA RHCSS RHCVA Senior Software Maintenance Engineer - SEG gpg id: 2048/FDB110E5 gpg fingerprint: 3BE7 EF71 4DD7 6812 D309 8F18 BD42 D095 FDB1 10E5 On Oct 5, 2011, at 4:03 AM, Jan Pazdziora wrote: > On Tue, Oct 04, 2011 at 07:13:31PM -0300, Marcelo Moreira de Mello wrote: >> Hello Team, >> >> Follow a patch addressed to BZ at $subject. Created an test package >> and worked as expected. >> >> After applied the patch, the results work as expected. > > Nack. > > The first part of your patch reverts change for bug 452956. Unless we > know *exactly* why that change was done for that bug (in other words, > reproduce that original bug, without that patch), we risk a regression > here. > > The second part of your patch seems like a noop to me since > StringUtils.defaultString is defined as > > Returns either the passed in String, or if the String is null, > an empty String (""). > > already. > > -- > Jan Pazdziora > Principal Software Engineer, Satellite Engineering, Red Hat > > ___ > Spacewalk-devel mailing list > Spacewalk-devel@redhat.com > https://www.redhat.com/mailman/listinfo/spacewalk-devel ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
Re: [Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null
On Tue, Oct 04, 2011 at 07:13:31PM -0300, Marcelo Moreira de Mello wrote: > Hello Team, > > Follow a patch addressed to BZ at $subject. Created an test package > and worked as expected. > >After applied the patch, the results work as expected. Nack. The first part of your patch reverts change for bug 452956. Unless we know *exactly* why that change was done for that bug (in other words, reproduce that original bug, without that patch), we risk a regression here. The second part of your patch seems like a noop to me since StringUtils.defaultString is defined as Returns either the passed in String, or if the String is null, an empty String (""). already. -- Jan Pazdziora Principal Software Engineer, Satellite Engineering, Red Hat ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel
[Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null
Hello Team, Follow a patch addressed to BZ at $subject. Created an test package and worked as expected. After applied the patch, the results work as expected. Best, mmello -- Marcelo Moreira de Mello RHCA RHCSS RHCVA Senior Software Maintenance Engineer/SEG gpg id: 2048R/FDB110E5 gpg fingerprint: 3BE7 EF71 4DD7 6812 D309 8F18 BD42 D095 FDB1 10E5 From: Marcelo Moreira de Mello Date: Tue, 4 Oct 2011 18:36:16 -0300 Subject: [PATCH] 741476 - fixed API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null --- .../frontend/xmlrpc/serializer/DmiSerializer.java | 14 +- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java index 7fb29ad..dcd94a6 100644 --- a/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java +++ b/java/code/src/com/redhat/rhn/frontend/xmlrpc/serializer/DmiSerializer.java @@ -59,15 +59,11 @@ public class DmiSerializer implements XmlRpcCustomSerializer { SerializerHelper bean = new SerializerHelper(builtInSerializer); Dmi dmi = (Dmi) value; -if (dmi.getSystem() == null) { -return; -} - -bean.add("vendor", StringUtils.defaultString(dmi.getVendor())); -bean.add("system", StringUtils.defaultString(dmi.getSystem())); -bean.add("product", StringUtils.defaultString(dmi.getProduct())); -bean.add("asset", StringUtils.defaultString(dmi.getAsset())); -bean.add("board", StringUtils.defaultString(dmi.getBoard())); +bean.add("vendor", StringUtils.defaultString(dmi.getVendor()==null?"":dmi.getVendor())); +bean.add("system", StringUtils.defaultString(dmi.getSystem()==null?"":dmi.getSystem())); +bean.add("product", StringUtils.defaultString(dmi.getProduct()==null?"":dmi.getProduct())); +bean.add("asset", StringUtils.defaultString(dmi.getAsset()==null?"":dmi.getAsset())); +bean.add("board", StringUtils.defaultString(dmi.getBoard()==null?"":dmi.getBoard())); bean.add("bios_release", StringUtils.defaultString(dmi.getBios().getRelease())); bean.add("bios_vendor", StringUtils.defaultString(dmi.getBios().getVendor())); bean.add("bios_version", StringUtils.defaultString(dmi.getBios().getVersion())); -- 1.7.6.4 ___ Spacewalk-devel mailing list Spacewalk-devel@redhat.com https://www.redhat.com/mailman/listinfo/spacewalk-devel