feat: add dataset relations support - #12668
Draft
vera wants to merge 4 commits into
Draft
Conversation
Member
Author
|
Current IT failures: Unsure if related to changes in this PR |
Member
|
We discussed this at tech hours today. We gave this PR the 6.13 milestone, the next release, but @vera is flexible about this. We gave it a size of 50, which is pretty big, but that might be an overestimate since Vera is quite happy to walk any reviewers through the code and docs. As a reminder, she presented the whole idea back in July during a community call. The recording and her slides are available at https://groups.google.com/g/dataverse-community/c/-Ki-MWvvd2c/m/SGa3-GqjAgAJ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Adds first-class dataset relations, allowing datasets to describe relationships with other internal Dataverse datasets or external resources.
Main functionality:
doc/sphinx-guides/source/api/native-api.rst.EditDatasetRelationspermission (by default, given to any role which previously had theEditDatasetpermission).DatasetRelationAlgorithmabstraction for retrieving and deduplicating relations. The current implementation,SqlDirectDatasetRelationAlgorithm, uses native SQL queries. (Alternative implementations could be introduced later without changing the API or service layer.)relatedIdentifierentries when their relation type name matches a supported DataCite relation type.relatedDatasetCountto Solr-indexed dataset records.Special notes for your reviewer:
Some further details:
PUT /api/datasets/{id}/versions/:draft) now performs metadata and relation replacement in one transaction. Invalid relations therefore roll back the entire submitted update. To that end, a new command (UpdateDatasetVersionWithRelationsCommand) was added which encapsulates existing code of the API endpointupdateDraftVersionand calls the two relevant subcommands (CreateDatasetVersionCommand/UpdateDatasetVersionCommandandReplaceDatasetRelationsCommand).Suggestions on how to test this:
Run the relevant unit tests and integration tests, especially the newly added/extended test cases:
which cover the added and changed functionality, including several edge and error cases. I recommend taking a look at the test cases in
DatasetRelationsITto get an overview of how this feature implementation works in practice.To test performance of the feature, benchmark scripts are included (see
tests/performance/relations/README.md). The following section shows some performance measurement results.Performance measurements
Using synthetic benchmark data
The synthetic benchmark seed script creates and publishes a temporary collection and target dataset through the API. It then creates a number (
-n, default: 10,000) of synthetic copies of that dataset in the database (which is faster than creatingnreal datasets through the API, and sufficient for the performance test). Finally, for each of the synthetically created datasets, it creates a relation pointing at the target dataset.Then, a second script measures performance when calling the relation list API for the target dataset, which requests the first page of relations.
Setup:
Results of performance measurement:
Requesting a page size of 10:
Requesting a page size of 50:
Requesting a page size of 150:
Note that the meaningfulness of these numbers is limited, as it is only a relatively simple synthetic benchmark. For more realistic numbers, see the following section.
In real Dataverse instance
For this benchmark, we performed a migration of the relations in our production Dataverse instance to the new model using an SQL script. (The migration script is not generally applicable/included in this PR, since we previously stored relations in our own custom metadata fields, which are used as source data by the migration SQL.)
The following measurements were collected using the relation list API in this real environment.
Overall stats:
Out of all datasets, we selected the one with the most relations for the performance test:
Stats of the tested dataset (latest published version):
Results of performance measurement:
Requesting a page size of 10:
Requesting a page size of 25:
Requesting a page size of 50:
Requesting all relations:
Note that the response is a lot slower when the metadata blocks of all related datasets are included in the response (
includeMetadataBlocks=true):Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No direct user-interface changes are introduced. This PR adds backend APIs and response fields that can be consumed by the frontend.
Is there a release notes update needed for this change?:
Yes. This introduces a substantial new feature and several API changes.
Release notes should mention:
EditDatasetRelationspermission.Additional documentation:
doc/sphinx-guides/source/api/native-api.rstscripts/api/data/relationTypes/relationTypeAllFields.jsontests/performance/relations/README.mdOpen questions:
I have compiled a list of open questions in the DEP:
Currently, a Dataverse installation starts out without any relation types and they must be configured by the administrator. Should some types come pre-installed?
Currently, relations are modeled after metadata, i.e. they are versioned in the same way. For relations from external sources such as DataCite, this might not make sense. Some relations may not be directly associated with/defined as part of a specific dataset version.
Currently, inverse relations are always returned by the API. There's no param to turn that off.
In our UI, we label them with an "Unconfirmed" badge for clarity:
Currently, there's no defined workflow of how to "approve" an incoming relationship.
(In other words, this is about how to turn an "Unconfirmed" relation as shown in the screenshot above into a confirmed one.)
What workflow would be most user-friendly?
Currently, the implementation does not retrieve transitive relations, e.g. if relations like A -> B -> C exist, A and C are transitively related, but this relation is not returned anywhere.
Do we need this? Can the SQL query-based retrieval strategy provide this with reasonable performance?
We have developed JS code for calling the APIs added in this PR, which might be used to integrate the feature into the React SPA.
How could an integration of this feature into React SPA be developed? As long as it is API-only, it's of limited use to users.
As mentioned above, we have performed a test migration from our custom metadata fields to the new relation model using an SQL script. For installations who store their relations in the existing citation block fields such as "Related Publication", "Related Dataset" and "Related Material", no migration path has been implemented or tested yet.
Should a migration path be provided as part of this feature's release? (Probably?)
Currently, only datasets may have relations (to other datasets or external resources).
In theory, the model can be extended to support other types of relation endpoints, if that is wanted.
Related issues: