[DS-1443] Export structure with structure-builder #1257

mwoodiupui · 2016-01-29T14:40:57Z

https://jira.duraspace.org/browse/DS-1443

Export the current Community/Collection structure in a form suitable for re-importation with the command-line structure-builder.

rradillen · 2016-01-30T16:11:18Z

this would be very practical

helix84 · 2016-01-31T15:37:43Z

I tested this PR and it works. Just some minor notes here:

Is there any need for the -e option it to be required for export? It currently is required.
/dspace/bin/dspace structure-builder -e admin@example.com -x -o dspace-structure.xml
The import requires the root element to be import_structure and produces imported_structure (note the difference). The export produces imported_structure, thus leading to "-There are no top level communities in the source document" error when trying to re-import the exported xml (or re-import the import output xml).
This one actually concerns import, but this would be a good opportunity to address it - import always creates new communities/collections, even if the identifier attribute is specified (which is the case in the import output xml and export xml).
a) During import, we could attempt to find out whether all the specified handles are free and if they are, use them. If not, for those objects which already exist, offer to create new handles or skip their creation.
b) Alternatively, if the identifier attribute is present, warn that new handles will be created.

mwoodiupui · 2016-03-21T19:43:28Z

All of these points are good ones. I'm addressing each. I took the cheap way out on (3) and just issue a warning when 'identifier' attributes are present.

I wonder why we have two different root element names at all.

mwoodiupui · 2016-07-01T19:09:58Z

Rebased on current master.

mwoodiupui · 2017-08-17T17:02:35Z

Rebased again on current master.

…lize it

…ommand function a separate method for clarity.

… for identifier.

…able; warn when importing containers with 'identifier' attributes

mwoodiupui · 2018-03-22T15:12:19Z

Rebased again on current master.

…ferences.

tdonohue

👍 Code looks good, but I haven't had a chance to test it. This seems like a logical feature to add in though, as we already have a corresponding import.

abollini

By code inspection it looks good +1
I have made only a small suggestion/comment inline in the code to be (eventually) addressed.
It will be nice to provide IT for this feature, import a structure and check that the output match with the import file will be great

abollini · 2018-08-16T13:22:43Z

dspace-api/src/main/java/org/dspace/administer/StructBuilder.java

@@ -142,13 +171,52 @@ public static void main(String[] argv)
        // set the context
        context.setCurrentUser(ePersonService.findByEmail(context, eperson));

+        // Export? Import?
+        if (line.hasOption('x')) {
+            exportStructure(context, output);


as you don't have a logged-in user I guess that you can hit auth issue with communities or collections without Anonymous READ

The user is logged in by line 172. But it is possible that 'String eperson' contains a name that matches no EPerson, and in that case the context will be logged out. We could find the EPerson during command analysis, and exit with an error if -e was given but its value is not a known account. Probably that should be a separate issue. The code doesn't do anything wrong, but the resulting error message is less helpful than it might be. We ought to check for this problem throughout the CLI.

Oh, I see: you perhaps mean that export should require -e as import does. That also can be pushed up to command analysis, so that execution can assume that it has a valid identity to set into the Context.
However, it is still possible to give an identity which is not authorized to access some of the repository structure.

pnbecker

Looks fine by code review. Will give it a test run asap.

pnbecker · 2018-08-22T20:09:11Z

I think this can be merged as it is. I would like to see one further improvement, which could be done in a separate pull request. We can create Communities with new identifiers by using communityService.create(parent, context) or we can create Communities using specified identifiers by using communityService.create(null, context, handle). If an identifier does exist in the input document, why don't we try to reuse it?

pnbecker · 2018-08-22T21:26:21Z

dspace-api/src/main/java/org/dspace/administer/StructBuilder.java

-            BufferedWriter out = new BufferedWriter(new FileWriter(output));
-            out.write(new XMLOutputter().outputString(xmlOutput));
-            out.close();
+            new XMLOutputter().output(xmlOutput, output);


In the export method we use Format.getPrettyFormat() to indent the XML. Why don't we use this for the xml output of the import method?

That sounds like a good idea, but it seems out-of-scope for the present patch and issue which are focused on the new export feature. I think that a new issue and a small patch would be welcome.

tdonohue · 2018-08-22T21:44:09Z

@mwoodiupui : Just to clarify, are you working on Integration tests for this feature? I recall you asking about how to do so on Slack. If you are working on ITs, then I'd suggest we wait to merge for ITs, and then merge immediately after ITs are added.

pnbecker · 2018-08-22T22:48:13Z

@mwoodiupui I just send you a PR realizing my idea to import Communities and Collections reusing their old handles if the import file contains those: mwoodiupui#13. Can we include it in this PR or shall I create a separate ticket and PR?

pnbecker · 2018-08-22T23:12:35Z

While extending the Code, I tested it. It works great. :-)

…tput.

mwoodiupui · 2018-08-27T15:40:47Z

@tdonohue I am working (slowly) on ITs for the feature.

pnbecker · 2018-09-05T10:07:17Z

@mwoodiupui Could you please take a look on my additions (mwoodiupui#13)? Would you like to integrate them here or shall I create a separate PR?

mwoodiupui · 2019-05-06T20:21:50Z

This PR was built before DSpace 7 became master, and is rather out-of-sync with the current master. I'm closing it and will submit a new one, rebuilt for v7.

mwoodiupui · 2019-05-07T15:14:00Z

@pnbecker I think that your patch really should be a separate Jira issue.

mwoodiupui added improvement work in progress PR is still being worked on & is not currently ready for review and removed work in progress PR is still being worked on & is not currently ready for review labels Jan 29, 2016

mwoodiupui added the work in progress PR is still being worked on & is not currently ready for review label Mar 21, 2016

mwoodiupui force-pushed the DS-1443 branch from e5d2b1a to 8a96726 Compare July 1, 2016 19:08

mwoodiupui force-pushed the DS-1443 branch from 8a96726 to 694c592 Compare August 17, 2017 17:01

mwoodiupui removed the work in progress PR is still being worked on & is not currently ready for review label Aug 17, 2017

mwoodiupui added 5 commits March 22, 2018 10:34

Clean up some IDE suggestions -- modernize the code a bit

2a9ab78

[DS-1443] Walk the Community/Collection tree, build a Document, seria…

3b390e7

…lize it

[DS-1443] Clean up documentation, whitespace, throwables; make each c…

9e70ad3

…ommand function a separate method for clarity.

[DS-1443] Build DOM tree correctly, make help work, use correct field…

3ec52e8

… for identifier.

[DS-1443] Don't require '-e' on export; make exported document import…

1b1ec55

…able; warn when importing containers with 'identifier' attributes

mwoodiupui force-pushed the DS-1443 branch from 694c592 to 1b1ec55 Compare March 22, 2018 15:11

[DS-1443] Fix a missed checkstyle complaint; finalize some service re…

eb7699b

…ferences.

mwoodiupui added this to the 7.0 milestone Mar 22, 2018

tdonohue approved these changes Mar 22, 2018

View reviewed changes

tdonohue added the quick win Pull request is small in size & should be easy to review and/or merge label Mar 22, 2018

abollini reviewed Aug 16, 2018

View reviewed changes

mwoodiupui added 2 commits August 20, 2018 13:20

[DS-1443] Move file opeing to command analyzer, to make testing easier.

0cb9b33

[DS-1443] Require EPerson for export as well as import.

f4d5498

pnbecker approved these changes Aug 22, 2018

View reviewed changes

pnbecker reviewed Aug 22, 2018

View reviewed changes

tdonohue added the tools: export Related to export of data from the system label Aug 22, 2018

mwoodiupui added 3 commits August 24, 2018 10:55

[DS-1443] Small cleanups.

9ba5884

[DS-1443] Begin integration testing: import.

24b8853

[DS-1443] Instead of ignoring metadata, provide values to match in ou…

29d250d

…tput.

[DS-1443] Test exporting.

c9986d0

mwoodiupui closed this May 6, 2019

mwoodiupui mentioned this pull request May 6, 2019

[DS-1443] Export structure with structure-builder #2421

Merged

tdonohue removed this from the 7.0 milestone Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DS-1443] Export structure with structure-builder #1257

[DS-1443] Export structure with structure-builder #1257

mwoodiupui commented Jan 29, 2016

rradillen commented Jan 30, 2016

helix84 commented Jan 31, 2016

mwoodiupui commented Mar 21, 2016

mwoodiupui commented Jul 1, 2016

mwoodiupui commented Aug 17, 2017

mwoodiupui commented Mar 22, 2018

tdonohue left a comment

abollini left a comment

abollini Aug 16, 2018

mwoodiupui Aug 16, 2018

mwoodiupui Aug 16, 2018 •

edited

pnbecker left a comment

pnbecker commented Aug 22, 2018

pnbecker Aug 22, 2018

mwoodiupui Aug 27, 2018

tdonohue commented Aug 22, 2018

pnbecker commented Aug 22, 2018

pnbecker commented Aug 22, 2018

mwoodiupui commented Aug 27, 2018

pnbecker commented Sep 5, 2018

mwoodiupui commented May 6, 2019

mwoodiupui commented May 7, 2019

[DS-1443] Export structure with structure-builder #1257

[DS-1443] Export structure with structure-builder #1257

Conversation

mwoodiupui commented Jan 29, 2016

rradillen commented Jan 30, 2016

helix84 commented Jan 31, 2016

mwoodiupui commented Mar 21, 2016

mwoodiupui commented Jul 1, 2016

mwoodiupui commented Aug 17, 2017

mwoodiupui commented Mar 22, 2018

tdonohue left a comment

Choose a reason for hiding this comment

abollini left a comment

Choose a reason for hiding this comment

abollini Aug 16, 2018

Choose a reason for hiding this comment

mwoodiupui Aug 16, 2018

Choose a reason for hiding this comment

mwoodiupui Aug 16, 2018 • edited

Choose a reason for hiding this comment

pnbecker left a comment

Choose a reason for hiding this comment

pnbecker commented Aug 22, 2018

pnbecker Aug 22, 2018

Choose a reason for hiding this comment

mwoodiupui Aug 27, 2018

Choose a reason for hiding this comment

tdonohue commented Aug 22, 2018

pnbecker commented Aug 22, 2018

pnbecker commented Aug 22, 2018

mwoodiupui commented Aug 27, 2018

pnbecker commented Sep 5, 2018

mwoodiupui commented May 6, 2019

mwoodiupui commented May 7, 2019

mwoodiupui Aug 16, 2018 •

edited