Signposting on Dataverse 5.9 #8424

vicding-mi · 2022-02-15T12:03:08Z

What this PR does / why we need it:
This PR adds signposting to Dataverse. Signposting enables easier machine access to datasets and hence increases accessibility.
Which issue(s) this PR closes:
N/A
Closes #5962
N/A
Special notes for your reviewer:

Suggestions on how to test this:
This PR does not change any human readable part of Dataverse.

curl -I http://linkto.dataverse/dataset to see the signposting L1 header

Link to L2 linkset can be found in L1 header and can be directly viewed in browser

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
in folder doc/release-notes/8424-signposting.md
Additional documentation:

…ring)

…nposting

* uses external js * adding signposting configuration setting * signposting poc from eko * error fix * move the signposting location in code * Signposting with MicroSettings cleanup v1 xhtml revert to vanilla * clean up * bug fix and add default file type; TODO: Bug on the baseurl (empty string) * hide type when do not use default * set default when no config is set for signposting * modification according to reviews * move long json string from code to bunddle * allow empty config on the level 2 profile Co-authored-by: Kevin Condon <[email protected]> Co-authored-by: ekoi <[email protected]>

vicding-mi · 2022-03-07T08:26:16Z

hey guys, not sure how to carry on the process/discussion. Can @qqmyers please give some guidance? I changed according to the review some of the points, while for the other points I tried to answers the review questions and ask for suggestions. Many thanks!

coveralls · 2022-03-08T19:23:08Z

Coverage decreased (-0.05%) to 18.816% when pulling 5fe6a8a on vicding-mi:signposting-5.9 into 0d853b7 on IQSS:develop.

qqmyers

Tried to add some feedback but it looks like there are still some open questions/change requests too. I'll contact Herbert about which metadata file(s) should be referenced.
Most of the discussion is about what should be configurable and if it's worth refactoring some parts. Neither are things that I'd hold this up for so I think when the other issues are resolved.

qqmyers · 2022-03-08T19:31:34Z

doc/release-notes/8424-signposting.md

+    "CC0": "https://creativecommons.org/licenses/cc0/"
+  },
+  "describedby": {
+    "doi": "https://doi.org/",


The GlobalId class provides a toURL method that uses public strings and will assign the doi or handle version according the the ID type.
So I think you could just use dataset.getGlobalIdentifier().toUrl() where you need the url.

qqmyers · 2022-03-08T19:56:56Z

doc/release-notes/8424-signposting.md

+  "defaultFileTypeValue": "https://schema.org/Dataset",
+  "maxItems": 5,
+  "maxAuthors": 5
+}


With the :Signposting setting you're giving people a very easy way to change all of these parameters (or to put '{}' as the value - not sure if that just turns off signposting or breaks the dataset page).

In general, I'm OK with leaving in any parameters you think are needed. My concern is really that by exposing these here you're not just making it possible for Herbert to change them but everyone else as well. If I as a Dataverse installation admin change the type from application/vnd.citationstyles.csl+json or use a different filetypevalue than schema.org/Dataset, would I still be 'Signposting-compliant'? The ones that make the most sense to me are the maxItems/maxAuthor ones - a local admin might want to show 10 authors which would still be valid. The rest seem more like things that should be constants in the code, or only read from the properties file if there has to be a way to change them without recompiling code.

That might suggest that just having a :Signposting setting that lets you change maxItems/maxAuthors (defaulting to 5 if you can't parse) and doesn't otherwise let you mess-up/disable Signposting overall might be better.

As far as I'm concerned, it's up to you whether this makes sense - I doubt many people will even touch the setting since it works by default anyway.

qqmyers · 2022-03-08T19:59:38Z

doc/release-notes/8424-signposting.md

+ * `maxAuthors` Same with `maxItems`, `maxAuthors` sets the max number of authors to be shown in `level 1` profile. 
+If amount of authors exceeds this value, no link of authors will be shown in `level 1` profile. 
+
+Note: Authors without author link will not be counted nor shown in any profile/linkset. 


Hopefully @pdurbin has a suggestion as to where in the guides would be best.

One thing I'll note about the above is that it seems to mix things I can change (e.g. maxAuthors) with info about signposting and fields like AboutPage that I can't change. It might be worthwhile to just give an example level 1 header and then just talk about the parts you might want to change, or at least indicate what is changeable when you list fields.

qqmyers · 2022-03-08T21:20:21Z

src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java

+            } else {
+                //TODO: if a user has access to this file, they should be given the swift url
+                // perhaps even we could use this as the "private url"
+                fileDownloadUrl = swiftIO.getTemporarySwiftUrl();


I think URLs like http://localhost:8080/api/access/datafile/15 would be best practice. I don't think you get that from the swiftIO.getTemporarySwiftUrl() call. That download URL - /api/access/datafile - comes from the api/Access.java class where I think you could add a header. (Similarly, I think you could respond to a HEAD instead of GET there as well).

pdurbin

@vicding-mi thanks for this pull request! I'm excited for Signposting support in Dataverse. I haven't had a chance to do a proper review yet but I'm at least leaving a comment about the config that's been on my mind.

pdurbin · 2022-03-09T18:17:15Z

doc/release-notes/8424-signposting.md

+ * The level 2 linkset can be fetched by visiting the dedicated linkset page for 
+   that artifact. The link can be seen in level 1 links with key name `rel="linkset"`.
+
+The configuration is stored as JSON string in the `Bundle.properties` file, key name is


Why is the config in Bundle.properties? I see elsewhere that @qqmyers has suggested a database setting called :Signposting, which sounds like a better approach to me.

vicding-mi · 2022-03-10T08:48:24Z

Hi @pdurbin and @qqmyers, many thanks for your comment.
bundle file is the solution I got from the proof of concept, so I sticked to it. Since db is a better place for storing config, I will change it and update you guys.

@qqmyers , thanks for your review, will revise accordingly.

pdurbin · 2022-03-10T12:26:09Z

@vicding-mi great! Thanks! Please feel free to create as many database settings as you need. I think the record is all the "PV" settings: :PVMinLength :PVMaxLength :PVNumberOfConsecutiveDigitsAllowed :PVCharacterRules :PVNumberOfCharacteristics :PVDictionaries :PVGoodStrength :PVCustomPasswordResetAlertMessage 😄

pdurbin

A few comments and some doc suggestions in the form of a pull request.

pdurbin · 2022-03-09T18:17:15Z

doc/release-notes/8424-signposting.md

+ * The level 2 linkset can be fetched by visiting the dedicated linkset page for 
+   that artifact. The link can be seen in level 1 links with key name `rel="linkset"`.
+
+The configuration is stored as JSON string in the `Bundle.properties` file, key name is


Why is the config in Bundle.properties? I see elsewhere that @qqmyers has suggested a database setting called :Signposting, which sounds like a better approach to me.

pdurbin · 2022-03-10T19:41:41Z

doc/release-notes/8424-signposting.md

+
+There are 2 Signposting profile levels, level 1 and level 2. In this implementation, 
+ * level 1 links are shown in 
+HTTP header, which can be fetched by `curl -I https://domain/link-to-article`. 


Would link-to-dataset make more sense than link-to-article?

pdurbin · 2022-03-10T19:42:46Z

doc/release-notes/8424-signposting.md

+ * level 1 links are shown in 
+HTTP header, which can be fetched by `curl -I https://domain/link-to-article`. 
+ * The level 2 linkset can be fetched by visiting the dedicated linkset page for 
+   that artifact. The link can be seen in level 1 links with key name `rel="linkset"`.


The term "artifact" might be confusing here. Are we talking about a dataset?

pdurbin · 2022-03-10T22:05:53Z

doc/release-notes/8424-signposting.md

+ * `maxAuthors` Same with `maxItems`, `maxAuthors` sets the max number of authors to be shown in `level 1` profile. 
+If amount of authors exceeds this value, no link of authors will be shown in `level 1` profile. 
+
+Note: Authors without author link will not be counted nor shown in any profile/linkset. 


There's no FAIR section but I just created vicding-mi#1 to suggest creating a "Discoverability" page in the Admin Guide. @vicding-mi if you like this, please feel free to merge it and add more docs. Thanks.

pdurbin

First, great pull request. I'm excited to have Signposting support in Dataverse!

One thing I want to make clear from the start is that core developers can probably jump in and hack on the code and docs to get this over the finish line. Please just let us know.

As I mentioned at #8424 (comment) I made a pull request (into this one) to suggest where to document Signposting.

I also left some other feedback about a <null> I'm seeing, about tests, agreeing with Jim about the ok method, etc.

pdurbin · 2022-03-11T19:59:52Z

src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java

+     * @param bld
+     * @return HTTP OK response which contains the json structure of linkset
+     */
+    protected Response okLinkset( JsonArrayBuilder bld ) {


Yes, I agree. Instead of a proliferation of "ok" methods like okLinkset, it would be better to delete it from this PR and use ok( JsonObjectBuilder bld ) method like this:

old:

return okLinkset(JsonPrinter.jsonLinkset(new SignpostingResources(systemConfig, dsv, signpostingConf)));

new:

return ok(Json.createObjectBuilder().add("linkset", JsonPrinter.jsonLinkset(new SignpostingResources(systemConfig, dsv, signpostingConf))));

pdurbin · 2022-03-11T20:01:48Z

src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java

+            }
+        }
+
+        logger.info(String.format("identifierSchema: %s", identifierSchema));


This should be lowered to logger.fine.

pdurbin · 2022-03-11T20:07:00Z

src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java

+                visibleAuthorCounter++;
+                // return empty if number of visible author more than max allowed
+                if (visibleAuthorCounter >= maxAuthors) return "";
+                singleAuthorString = "<" + authorURL + ">;rel=\"author\"";


I'm not sure why but I'm getting null for authorURL like this (I'm on 5fe6a8a):

Link: <null>;rel="author", <https://doi.org/...

This is just a local dataset in dev. Here's the curl command:

curl -s -i http://localhost:8080/dataset.xhtml?persistentId=doi:10.5072/FK2/KQRKB8

pdurbin · 2022-03-11T20:08:37Z

src/main/webapp/dataset.xhtml

+                <c:if test="#{not empty DatasetPage.getSignpostingLinkHeader()}">
+                    <f:event type="preRenderView" listener="#{facesContext.externalContext.response.setHeader('Link', DatasetPage.getSignpostingLinkHeader())}" />
+                </c:if>
+                <!-- End add Signposting-->


I'm leaving this comment here at the end because this is where we usually have tests. Can some tests be written for this code?

…; close swiftIO after use

qqmyers · 2022-09-16T15:29:46Z

Closing in favor of #8981 which builds on the commits here.

Vic Ding and others added 30 commits November 4, 2020 12:32

Merge remote-tracking branch 'upstream/develop' into develop

6a45404

Merge remote-tracking branch 'upstream/develop' into develop

770680e

cvm-sp

a38f0df

cvm-sp temp

c48df18

cvm-sp temp

2868228

cvm-sp temp

a55a378

tmp

db89d30

tmp

de12144

tmp

74dd56d

Merge remote-tracking branch 'upstream/develop' into develop

53f25b5

uses external js

673eef0

adding signposting configuration setting

6bc0f17

Merge remote-tracking branch 'upstream/develop' into develop

879f236

Merge remote-tracking branch 'upstream/develop' into develop

18ba37c

signposting poc from eko

3950734

error fix

5abe639

move the signposting location in code

3d644d1

Signposting with MicroSettings cleanup v1

65d69fe

Signposting with MicroSettings cleanup v1 xhtml revert to vanilla

7345f66

clean up

33fcb95

bug fix and add default file type; TODO: Bug on the baseurl (empty st…

a2e5b64

…ring)

hide type when do not use default

ddb8780

set default when no config is set for signposting

f2fb557

Merge branch 'signposting' of github.com:DANS-KNAW/dataverse into sig…

9de4c35

…nposting

modification according to reviews

1c6fde5

move long json string from code to bunddle

9752c6a

allow empty config on the level 2 profile

1c0cbbc

Merge branch 'develop' into signposting

0c40649

revision based on Herbert feedback

b441c65

vicding-mi requested a review from qqmyers February 22, 2022 14:56

pdurbin self-assigned this Mar 7, 2022

qqmyers reviewed Mar 8, 2022

View reviewed changes

pdurbin reviewed Mar 9, 2022

View reviewed changes

pdurbin added a commit that referenced this pull request Mar 10, 2022

stub out Signposting docs #8424

cea043b

pdurbin reviewed Mar 10, 2022

View reviewed changes

pdurbin added a commit that referenced this pull request Mar 11, 2022

emphasize that it's the metadata that gets harvested #8424

addf5cf

pdurbin reviewed Mar 11, 2022

View reviewed changes

pdurbin assigned vicding-mi and unassigned pdurbin Mar 11, 2022

vicding-mi added 3 commits March 27, 2022 13:13

remove pid type from config, use globalid instead; remove null author…

149e45a

…; close swiftIO after use

remove describedby from release doc

4a65b4a

use recommended call to get pid uri

a538289

pdurbin mentioned this pull request Apr 5, 2022

Signposting #7919

Closed

qqmyers pushed a commit to GlobalDataverseCommunityConsortium/dataverse that referenced this pull request Aug 15, 2022

stub out Signposting docs IQSS#8424

2a07f8b

qqmyers pushed a commit to GlobalDataverseCommunityConsortium/dataverse that referenced this pull request Aug 15, 2022

emphasize that it's the metadata that gets harvested IQSS#8424

4b2e44f

qqmyers mentioned this pull request Sep 16, 2022

Gdcc/Signposting #8981

Merged

qqmyers closed this Sep 16, 2022

jggautier mentioned this pull request May 1, 2024

Signposting should include license URIs made available in planned "multi-license" work #7969

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Signposting on Dataverse 5.9 #8424

Signposting on Dataverse 5.9 #8424

vicding-mi commented Feb 15, 2022 •

edited by pdurbin

Loading

vicding-mi commented Mar 7, 2022

coveralls commented Mar 8, 2022

qqmyers left a comment

qqmyers Mar 8, 2022

qqmyers Mar 8, 2022

qqmyers Mar 8, 2022

qqmyers Mar 8, 2022

pdurbin left a comment

pdurbin Mar 9, 2022

vicding-mi commented Mar 10, 2022

pdurbin commented Mar 10, 2022

pdurbin left a comment

pdurbin Mar 9, 2022

pdurbin Mar 10, 2022

pdurbin Mar 10, 2022

pdurbin Mar 10, 2022

pdurbin left a comment

pdurbin Mar 11, 2022

pdurbin Mar 11, 2022

pdurbin Mar 11, 2022

pdurbin Mar 11, 2022

qqmyers commented Sep 16, 2022

Signposting on Dataverse 5.9 #8424

Signposting on Dataverse 5.9 #8424

Conversation

vicding-mi commented Feb 15, 2022 • edited by pdurbin Loading

vicding-mi commented Mar 7, 2022

coveralls commented Mar 8, 2022

qqmyers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vicding-mi commented Mar 10, 2022

pdurbin commented Mar 10, 2022

pdurbin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qqmyers commented Sep 16, 2022

vicding-mi commented Feb 15, 2022 •

edited by pdurbin

Loading