Integration #55
Integration #55
Conversation
…ema-java � Conflicts: � src/main/java/io/frictionlessdata/tableschema/schema/BeanSchema.java
…lared as @JsonIgnore
There was a problem hiding this comment.
Well done!
I would ideally go a bit further in embracing Jackson's (de)serialization features. I think the state we should aim at is to remove basically all mentions of ArrayNode or ObjectNode in the code, except at the places where the corresponding instances can genuinely hold any json array or object. See my review on frictionlessdata/datapackage-java#35 for more details about this.
That is of course more work than getting rid of the org.json dependency strictly speaking, but this library is not likely to be of much use to us until that is properly cleaned up. That being said, you are of course not responsible for the state this library is in.
| arr.add(obj); | ||
| } | ||
| return arr.toString(); | ||
| return JsonUtil.getInstance().serialize(arr); |
There was a problem hiding this comment.
Couldn't this asJson method be simplified, by annotating the class appropriately for Jackson to serialize it directly?
| : null; | ||
| } | ||
| return field; | ||
| return JsonUtil.getInstance().deserialize(json, Field.class); |
There was a problem hiding this comment.
Nice - that's what we should have everywhere.
| } | ||
|
|
||
| public static Field fromJson (String json) { | ||
| BooleanField field = (BooleanField)Field.fromJson(json); |
There was a problem hiding this comment.
Are you sure about this? Boolean fields in a TableSchema allow for declarations what values should be considered true'ish or false'ish:
The boolean field can be customised with these additional properties:
trueValues: [ "true", "True", "TRUE", "1" ]
falseValues: [ "false", "False", "FALSE", "0" ]
The code block removed here did just that and I don't see where the functionality has moved, but maybe I am missing it?
There was a problem hiding this comment.
Thanks @iSnow, I had to look at this a couple times too =] - SInce it's parsing those fields as String arrays [trueValues, falseValues], jackson will automatically map them in the BooeanField class.
The 'parseValue` function should still validate the truthy/falsey values -
@Override
public Boolean parseValue(String value, String format, Map<String, Object> options)
throws InvalidCastException, ConstraintsException {
if (null != options) {
if (options.containsKey("trueValues")) {
trueValues = new ArrayList<>((Collection) options.get("trueValues"));
}
if (options.containsKey("falseValues")) {
falseValues = new ArrayList<>((Collection) options.get("falseValues"));
}
}
if (trueValues.contains(value.toLowerCase())){
return true;
}else if (falseValues.contains(value.toLowerCase())){
return false;
}else{
throw new TypeInferringException();
}
}
There was a problem hiding this comment.
Thanks. Unfortunately, it seems there is no test for it, and even the master test-data suite from frictionless doesn't seem to have a Schema that uses non-standard true/false values.
As this is not something in the scope of this bounty, I am merging your changes and pushing to master and will craft a test later.
Thanks for your work, I am in awe about your speed switching this to Jackson!
There was a problem hiding this comment.
Thank you @iSnow !!!
Overview
Removes everit and org.json dependencies, and replaces with jackson
Please preserve this line to notify @iSnow (lead of this repository)