search

opendatatrentino / jackan

Java client to access CKAN repositories
http://opendatatrentino.github.io/jackan
Other
19 stars 13 forks source link

Error reading records from demo.ckan.org #41

Open pandzel-zz opened 7 years ago

pandzel-zz commented 7 years ago

I have an issue reading certain records from demo.ckan.org which have "others" property populated with a simple string.

Example of such an record is: http://demo.ckan.org/api/3/action/package_search?q=id:69c59a57-9ff3-43ff-915a-3a1c04bc4d80

You can see that there is a property others: "qui1hfv3qtm85lg2iokob6lg1m" within the very first resource entry.

It appears that Jackan fails to load this resource because it attempts to load a simple string into the private Map<String, Object> others bean property as defined in CkanResourceBase class.

DavidLeoni commented 7 years ago

Hi Piotr, thanks for the report.

In Jackan we 'invented' the field others to insert unexpected fields, but evidently I didn't think about possibile collisions with custom schemas that already have the others field. These days I don't have much time to look into it, first quick fix could be to rename others in the source code to something unique like jackanOthers, or check how to configure Jackson to prevent such collisions.

pandzel-zz commented 7 years ago

David,

Thank you for responding so quickly. Studying your code I can tell that this is not a trivial problem. Perhaps it is possible to avoid collisions like that to some extent by creating own deserializer. Technically, it's not that hard.

But here is a real problem: Jackan "others" and CKAN "others" are semantically completely different values even regardless the actual type in Java code. What I mean is the meaning of those two fields are different, therefore it really make sense to rename "others" into "jackanOthers" or even drop it completely.

The twist comes when you realize that public methods getOthers(), setOthers() and putOthers() are (willingly or unwillingly) part of your API, and you can not change it without breaking somebody's code who may be using Jackan library already.

Anyway, great library and great and mature design. I hope, you can tackle "others" issue some thay in the future.

DavidLeoni commented 7 years ago

Hi Piotr, thanks for the positive comments!

I gave a better look at the problem.

Looking at the official CKAN schema, there is no others field. But there is the option to implement custom schemas (which I hate), so basically each catalog can have its own unexpected fields. You can get the 'others' field only if you're unlucky enough to find a catalog with this particular invented field. In this case, dataset you point to was automatically generated who-knows-how. On demo.ckan.org in particular you can actually find a lot of garbage data, including stuff generated by Jackan test suite :-)

Anyway when I find the time I'm going to fix the problem. I will try to avoid breaking the current api.