lukefitzwolfgang / icatproject

Automatically exported from code.google.com/p/icatproject
0 stars 0 forks source link

NullPointerException occurring in EntityBaseBean.preparePersist(args) #139

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Component concerned: org.icatproject.core.entity.EntityBaseBean, 
preparePersist(String, EntityManager, GateKeeper) method

Since ICAT server version 4.3.2 I noticed that in a number of situations when 
one-to-many relationship between entities is involved, the code 

List<EntityBaseBean> collection = (List<EntityBaseBean>) m.invoke(this);
if (!collection.isEmpty()) {
  Method rev = r.getInverseSetter();
  for (EntityBaseBean bean : collection) {
    bean.preparePersist(modId, manager, gateKeeper);
    rev.invoke(bean, this);
  }
}

throws NullPointerException in the if condition, because the previous 
m.invoke(this) _may_ return null.

I don't have the access to the latest source (v4.3.3), but comparing the 
bytecode of v4.3.2 and the latest snapshot of v4.3.3 
(icat.core-4.3.3-20140626.100202-17.jar), I can note that they both contain the 
following:

 99  invokevirtual java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) : java.lang.Object [21]
102  checkcast java.util.List [22]
105  astore 10 [collection]
107  aload 10 [collection]
109  invokeinterface java.util.List.isEmpty() : boolean [23] [nargs: 1]
114  ifne 182

while with a non-null check that should be

 99  invokevirtual java.lang.reflect.Method.invoke(java.lang.Object, java.lang.Object[]) : java.lang.Object [21]
102  checkcast java.util.List [22]
105  astore 10 [collection]
107  aload 10 [collection]
--> 109  ifnull 187
112  aload 10 [collection]
114  invokeinterface java.util.List.isEmpty() : boolean [23] [nargs: 1]
119  ifne 187

Please, fix this and change the condition to 

if (collection != null && !collection.isEmpty()) {
// no changes in the block
}

Otherwise it's not very clean to recompile and use the icat.core dist that I 
fixed locally, as I'm actually doing...

Original issue reported on code.google.com by anton.te...@gmail.com on 8 Jul 2014 at 8:15

GoogleCodeExporter commented 9 years ago
I don't check for this being not null because all the collections are meant to 
be initialised to an empty ArrayList. It may be that I have missed one. Can you 
tell me which relationship is giving the problem?

Steve

Original comment by dr.s.m.f...@gmail.com on 8 Jul 2014 at 10:07

GoogleCodeExporter commented 9 years ago
Strangely, this occurs when any one-two-many entity relationship is parsed, if 
an Entity is _not_ associated with more than 0 items on the other side of the 
relation.

If, in your context, the exception never occurs, then I think that the reason 
may be somewhere in my JAX-WS generated proxy code used for SOAP requests.

For example, the relation between a Dataset and Datafiles is generated as 
follows (extra code omitted):

public class Dataset extends EntityBaseBean {

  @XmlElement(nillable = true)
  protected List<Datafile> datafiles;

  // no public no-args constructor generated: on default init datafiles = null !

  public List<Datafile> getDatafiles() {
    if (datafiles == null) {
      datafiles = new ArrayList<Datafile>();
    }
    return this.datafiles;
  }

  // and there is no generated setter!  

}

Of course, this code is very ugly: I won't speak here about the SRP violation 
in the getter, due to the lazy init, nor about the weird "protected" access to 
the field. And I still don't see how it can be worked around, as it is JAX-WS 
generated (am I missing some config?).

I suppose that somewhere deep behind, the exception may be related with the 
access to the objects using java.lang.reflect: the fields are null by default.

Would you please be so kind as to add the non-null check to the condition? It 
does not make the routine heavier, it's just a matter of a single ifnull goto 
in the bytecode afterwards...

Original comment by anton.te...@gmail.com on 8 Jul 2014 at 10:38

GoogleCodeExporter commented 9 years ago
If you are generating your own client from the WSDL then you need to hack the 
wsdl first to get decent code out of it. My code for this is:

#!/bin/sh

url=$1

wget --no-proxy --no-check-certificate "$url"/ICATService/ICAT?wsdl -O ICAT.wsdl
wget --no-proxy --no-check-certificate "$url"/ICATService/ICAT?xsd=1 -O ICAT.xsd

sed s#"$url"/ICATService/ICAT?xsd=1#ICAT.xsd# ICAT.wsdl > ICAT.wsdl.new && mv 
ICAT.wsdl.new ICAT.wsdl
sed 's#ref="tns:\([a-z]*\)"#name="\1" type="tns:\1"#' ICAT.xsd > ICAT.xsd.new 
&& mv ICAT.xsd.new ICAT.xsd

however I suggest you just use the client that I generate.

This is the ICAT entity on the client side. The failure you are getting is on 
the server side with objects generated from the SOAP XML generated by your 
client. The SOAP generator can only call getDatafiles() which can never return 
null

Given that this code works fine for others when invoked from C#, C++, Python 
and Java clients I think you must be generating a bad SOAP message. I don't 
want to add code which should never be invoked but prefer to find out what is 
actually going wrong.

Please give my client a try and see if the problem goes away.

Steve

Original comment by dr.s.m.f...@gmail.com on 8 Jul 2014 at 12:38

GoogleCodeExporter commented 9 years ago
In the absence of further replies I am closing this one.

Steve

Original comment by dr.s.m.f...@gmail.com on 11 Dec 2014 at 3:30