Re: [Spacewalk-devel] [PATCH] - 741476 API system.getDmi() method does not return data when SYSTEM field at RHNSERVERDMI table is null

2011-10-06 Thread Jan Pazdziora
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

2011-10-05 Thread Marcelo Moreira de Mello
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

2011-10-05 Thread Marcelo Moreira de Mello
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

2011-10-05 Thread Jan Pazdziora
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

2011-10-04 Thread Marcelo Moreira de Mello
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