karishma-tirthani / odata4j

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

InMemoryProducer toOEntity fails in 0.7.0 due to findEntityInfoForClass usage #213

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
When I upgrade my producer app to use odata4j 0.7.0 I see the following error 
when I attempt to access any of my entities in OData Explorer (similar problem 
in Sesame).

Internal Server Error

[HttpWebRequest_WebException_RemoteServer]
Arguments: NotFound
Debugging resource strings are unavailable. Often the key and arguments provide 
sufficient information to diagnose the problem. See 
http://go.microsoft.com/fwlink/?linkid=106663&Version=5.1.10411.0&File=System.Wi
ndows.dll&Key=HttpWebRequest_WebException_RemoteServer
   at System.Net.Browser.ClientHttpWebRequest.EndGetResponse(IAsyncResult asyncResult)
   at SL_OData_Explorer.Views.ODataExplorer.<>c__DisplayClass1e.<>c__DisplayClass20.<DownloadRawContent>b__1d()

The same producer code works fine when I switch back to 0.6.0.
Is anyone else seeing this?

Here is some code from very simple producer example:

public class APMProducerFactory implements ODataProducerFactory
{
    @Override
    public ODataProducer create(Properties properties)
    {
        InMemoryProducer producer = new InMemoryProducer("example", 25);
        producer.register(Entry.class, "SystemProperties", new Func<Iterable<Entry>>()
        {
            public Iterable<Entry> apply()
            {
                return (Iterable<Entry>) (Object)
                        System.getProperties().entrySet();
            }
        }, "Key");
        return producer;
   }
}

What is the expected output? What do you see instead?
Expected to be able to access entities registered with InMemoryProducer.  
Instead getting a NPE on server.

What version of the product are you using? On what operating system?
Problem started when I moved from 0.6.0 to 0.7.0.
Running on Windows 7 - but it doesn't seem to matter.

Please provide any additional information below.
I tracked this issue down to the following code in InMemoryProducer.java:

  protected OEntity toOEntity(EdmEntitySet ees, Object obj, PropertyPathHelper pathHelper) {
    InMemoryEntityInfo<?> ei = this.findEntityInfoForClass(obj.getClass()); //  eis.get(ees.getName());

In 0.6.0 ei was initialized using the commented out code - it was changed to 
use findEntityInfoForClass() in 0.7.0.
This is failing to find the entity in the example code because it is looking 
for object of type class java.util.Hashtable$Entry but the entity info was 
saved as type interface java.util.Map$Entry.  So entity info is not found and 
null is returned.

I suggest the following change:

    protected OEntity toOEntity(EdmEntitySet ees, Object obj, PropertyPathHelper pathHelper) {
        InMemoryEntityInfo<?> ei = findEntityInfoForEntitySet(ees.getName());

To find via the class just seems plain wrong...  There could be multiple entity 
sets using same class - using the name instead seems correct approach.  I will 
enter an issue on this.

Original issue reported on code.google.com by mark.los...@ca.com on 6 Sep 2012 at 7:55

GoogleCodeExporter commented 8 years ago
Just took a look and agree this is simply wrong.

Original comment by john.spurlock on 10 Nov 2012 at 6:26

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 89d319f4af50.

Original comment by john.spurlock on 10 Nov 2012 at 6:29

GoogleCodeExporter commented 8 years ago
"simply wrong" may be a little harsh.  Couple of things:
1.  I tried the example producer attached to this defect with the original 
findEntityInfoForClass implementation and it works just fine (this is obvious 
from looking at the code:  Map.Entry isAssignableFrom HashMap.Entry.
2.  Here is a comment that should have been placed at the top of toOEntity:

 // find the type info for the object we are converting.  Note that we don't
 // do this lookup by entity set name:  The EDM type of an entity set defines
 // the *base* type of entities found in the set.  If we are dealing with a 
 // object of a sub-type here, we wan't to convert it according to its actual
 // type.

Given that, how do you guys propose we solve this issue?

Personally I think it is a reasonable assumption to make that an 
InMemoryProducer only ever has a single Class<?> -to- InMemoryEntityInfo 
mapping.  This does not preclude having multiple entity sets that expose the 
same EDM entity type.

Original comment by tony.ro...@gmail.com on 13 Nov 2012 at 5:00