SWORD: if custom terms disabled, report error #8580

[pdurbin]

What this PR does / why we need it:

Right now you get a 500 error if you try to create a dataset with SWORD when you have custom terms disabled (:AllowCustomTermsOfUse).

In this pull request, if custom terms are disabled we now report an error if you try to use dcterms:rights.

Which issue(s) this PR closes:

• Closes SWORD API: Submitting a dataset with terms when custom dataset terms is disabled results in nondescript command line error and null ptr log error. #8580

Special notes for your reviewer:

The null pointer was fixed by passing licenseService to JsonParser.

Suggestions on how to test this:

Make sure this is true: if custom terms are disabled we now report an error if you try to use dcterms:rights

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?:

No, just a minor bug fix.

Additional documentation:

Included (the new error is mentioned).

[scolapasta]

A very small and specifc change request. Additionally, though, you mention in the issue that we should consider removing the constructor without license server. I agree and since this would be good tech debt clean up (even if it increases the scope). Alternatively, we could have that work be in the fix for the issue I'm about to create from an RT ticket we got. I'm in favor of taking care of that now (in case there are even other circumstances).

[pdurbin]

Additionally, though, you mention in the issue that we should consider removing the constructor without license server. I agree and since this would be good tech debt clean up (even if it increases the scope). Alternatively, we could have that work be in the fix for the issue I'm about to create from an RT ticket we got.

Yesterday I did a little investigation into removing the old constructor. This pull request removes one instance and there are five more:

For the bottom two in ImportGenericServiceBean I'd recommend simply deleting both methods where the constructor used. I'm happy to do this in this pull request. It's dead code added in 087b017 before we shipped 4.0 and the app compiles fine without it. One method is an unused variation on importXML and the other is importDCTerms, also unused.

I haven't tried deleting JsonParser with the null, null, null. I guess we can try.

The two in ImportServiceBean give me pause because they're off topic for SWORD. I'm sure doImportHarvestedDataset has to do with harvesting. doImport I played around with a bit but I don't think the import API (/api/batch/import) works at all anymore and it's undocumented. So it would be some effort to get the API working at all before we remove the constructor. And I assume we don't want to document that API. I think it used for DVN 3 migration.

In summary, for this pull request I'm happy to remove the dead code in ImportGenericServiceBean and take a swing at removing the null,null,null JsonParser. Harvesting should probably be handled in a separate effort. /api/batch/import I'm not sure we even want to prioritize fixing.

[scolapasta]

@pdurbin ok. let's also mark the method as @depecated.

[pdurbin]

@scolapasta sure, deprecated in de9ff67

[coveralls]

Coverage increased (+0.009%) to 19.24% when pulling 593213a on 8580-sword-rights into 82cc65e on develop.

[pdurbin]

@scolapasta in 593213a I removed the dead code ( importDCTerms and the other importXML method). I pushed this change directly to this pull request.

Removing the JsonParser constructor with this( null,null,null ) changes code related to the following areas...

• IP Groups
• Mail Groups
• Workflows
• a method called JsonParser.parseFiles

... and I didn't test any of this (apart from checking that mvn package works) so I didn't include it in this pull request. I did go ahead and push it to a different branch and you can see the proposed change at 583466c. If you'd like this commit included in the pull request, please let me know.