Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error reading records from demo.ckan.org #41

Open
pandzel-zz opened this issue Nov 7, 2016 · 3 comments
Open

Error reading records from demo.ckan.org #41

pandzel-zz opened this issue Nov 7, 2016 · 3 comments

Comments

@pandzel-zz
Copy link

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants