Skip to content

Fixing ror v2 api access for cvoc #11762

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Fixing ror v2 api access for cvoc #11762

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d24e09
integer detection for array access
ffritze Aug 21, 2025
894d3d5
first check if it's a number
ffritze Aug 22, 2025
69e5a34
JsonArray fix
ffritze Aug 22, 2025
9de4547
Merge branch 'develop' of https://github.com/IQSS/dataverse into fix_…
ffritze Oct 7, 2025
3cf657f
@qqmyers additional suggestion
ffritze Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/main/java/edu/harvard/iq/dataverse/DatasetFieldServiceBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import jakarta.persistence.criteria.*;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.http.HttpResponse;
import org.apache.http.HttpResponseInterceptor;
import org.apache.http.client.methods.HttpGet;
Expand Down Expand Up @@ -764,7 +765,17 @@ Object processPathSegment(int index, String[] pathParts, JsonValue curPath, Stri
}

} else {
curPath = ((JsonObject) curPath).get(pathParts[index]);
if (NumberUtils.isCreatable(pathParts[index])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth handling the case where "0" is returned as a string too? This code assumes anything that can be a number is one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say a number is a number. If you construct a JSON object then you shouldn't use a number as a string value for a key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me rephrase - the code currently assumes that if the value is interpretable as a number, we must be parsing an array, whereas if the value is a json object, the keys would have to be strings. I don't know how often we'll encounter such things (objects with keys "1", "2", "3"), but it seems like it might be cleaner/safer to see if the value can be a number && the curPath is a json array, you're new code is invoked, otherwise it tries to deal with it as an object with a string key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ffritze - apologies for letting this sit. I'm suggesting the following change: 5cd4ecf . If that's acceptable to you, can you include it, and update your branch from IQSS/develop (as done in develop...GlobalDataverseCommunityConsortium:dataverse:fix_ror-v2_api_cvoc) so our build system will be able to run it and we can plan on including it in Dataverse 6.9? (FWIW: I wasn't able to make a commit or PR against your branch)

Copy link
Contributor Author

@ffritze ffritze Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qqmyers Thanks a lot. I will test it in our infrastructure. Currently we have a maintenance phase so I will postpone it until the beginning of next week.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qqmyers I can now confirm that your additional code in this branch is working as expected. This pull request is ready for production

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ffritze great! We were leaving you assigned to this PR until you confirmed this. I'll remove you. Have a good weekend!

try {
int indexNumber = Integer.parseInt(pathParts[index]);
curPath = ((JsonArray) curPath).get(indexNumber);
} catch (NumberFormatException nfe) {
logger.fine("Please provide a valid integer number " + pathParts[index]);
}
} else {
curPath = ((JsonObject) curPath).get(pathParts[index]);
}
// curPath = ((JsonObject) curPath).get(pathParts[index]);
logger.fine("Found next Path object " + curPath.toString());
return processPathSegment(index + 1, pathParts, curPath, termUri);
}
Expand Down