Skip to content

Commit

Permalink
Fixed property datatype handling
Browse files Browse the repository at this point in the history
* Corrected test case to check for proper datatype IRI
* Corrected code to pass test case
* Added anothe FIXME in JsonConverter and removed a TODO
  • Loading branch information
mkroetzsch committed Mar 19, 2014
1 parent c88ffc7 commit 4a5d936
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.wikidata.wdtk.datamodel.interfaces.Value;
import org.wikidata.wdtk.datamodel.interfaces.ValueSnak;

// TODO logging support
/**
* This class provides methods to convert dump-file JSON objects into
* representations according to the WDTK data model. Since the converted JSON
Expand Down Expand Up @@ -182,6 +181,9 @@ public ItemDocument convertToItemDocument(JSONObject jsonObject,
Map<String, SiteLink> siteLinks = new HashMap<>();

if (jsonObject.has(KEY_LINK)) {
// FIXME This code has problems if linkArray is not null. Probably
// only the inner two lines are needed and the rest should be
// deleted.
JSONArray linkArray = jsonObject.optJSONArray(KEY_LINK);

This comment has been minimized.

Copy link
@fer-rum

fer-rum Mar 21, 2014

Member

There is one case where the links might be an JSONArray: Older dumps write an empty array when no links are available (For what reason ever.)

So for example in item Q285759 it is handled this way: "links":[]

This is the only time I found the value of the "links"-key to be an array so I omitted the else-branch. Should have documented this. The result is the siteLinks-Variable to remain an empty HashMap.

This comment has been minimized.

Copy link
@fer-rum

fer-rum Mar 21, 2014

Member

My suggestion would be to log a debug message. I will process the dump again to see if this happens often or if there is a non-empty array.

if (linkArray == null) {
JSONObject jsonLinks = jsonObject.getJSONObject(KEY_LINK);
Expand Down Expand Up @@ -341,7 +343,7 @@ private Map<String, SiteLink> getSiteLinks(JSONObject jsonObject)
String siteKey = linkIterator.next();

JSONObject currentLink = jsonObject.optJSONObject(siteKey);
if (currentLink != null) { // modern form wiht badges
if (currentLink != null) { // modern form with badges
title = currentLink.getString("name");

JSONArray badgeArray = currentLink.getJSONArray("badges");
Expand Down Expand Up @@ -656,11 +658,43 @@ private ValueSnak getValueSnak(JSONArray jsonValueSnak)
* @param jsonDataTypeId
* the id of the datatype
* @return the corresponding DatatypeIdValue
* @throws JSONException
* if the datatype string is not known
*/
private DatatypeIdValue getDatatypeIdValue(String jsonDataTypeId) {
// FIXME This is not correct. The datatype id in JSON is not the
// datatype that we need to use in the datamodel.
return this.factory.getDatatypeIdValue(jsonDataTypeId);
private DatatypeIdValue getDatatypeIdValue(String jsonDataTypeId)
throws JSONException {
return this.factory.getDatatypeIdValue(getDatatypeIri(jsonDataTypeId));
}

/**
* Returns the IRI of a Wikibase datatype for a given JSON datatype string.
*
* @param jsonDataTypeId
* the id of the datatype
* @return the corresponding datatype IRI
* @throws JSONException
* if the datatype string is not known
*/
private String getDatatypeIri(String jsonDataTypeId) throws JSONException {
switch (jsonDataTypeId) {
case "wikibase-item":
return DatatypeIdValue.DT_ITEM;
case "string":
return DatatypeIdValue.DT_STRING;
case "url":
return DatatypeIdValue.DT_URL;
case "commonsMedia":
return DatatypeIdValue.DT_COMMONS_MEDIA;
case "time":
return DatatypeIdValue.DT_TIME;
case "globe-coordinate":
return DatatypeIdValue.DT_GLOBE_COORDINATES;
case "quantity":
return DatatypeIdValue.DT_QUANTITY;
default:
throw new JSONException("Unknown property datatype "
+ jsonDataTypeId);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void testEmptyProperty() throws JSONException, IOException {
PropertyIdValue propertyId = this.factory.getPropertyIdValue("P1",
BASE_IRI);
DatatypeIdValue datatypeId = this.factory
.getDatatypeIdValue("globe-coordinate");
.getDatatypeIdValue(DatatypeIdValue.DT_GLOBE_COORDINATES);
PropertyDocument emptyPropertyDocument = this.factory
.getPropertyDocument(propertyId,
Collections.<MonolingualTextValue> emptyList(),
Expand Down

0 comments on commit 4a5d936

Please sign in to comment.