search

fcrepo-exts / fcrepo-import-export

Apache License 2.0
15 stars 19 forks source link

Updated importer to add contained resources before storing container … #107

Closed mikedurbin closed 7 years ago

mikedurbin commented 7 years ago

…RDF.

https://jira.duraspace.org/browse/FCREPO-2518

mikedurbin commented 7 years ago

Thanks for testing. I think I fixed the error associated with binary children.

dbernstein commented 7 years ago

Cool I will try it again.

On Thu, Jun 22, 2017 at 2:02 PM, Michael Durbin notifications@github.com wrote:

Thanks for testing. I think I fixed the error associated with binary children.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fcrepo4-labs/fcrepo-import-export/pull/107#issuecomment-310501697, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVDa29mRPQZP8eGiWYAFY76RWKkpp9Aks5sGtZggaJpZM4OCnew .

mikedurbin commented 7 years ago

This is easy enough to fix, but it looks like we went to some lengths to intentionally prevent importing the root container's triples.

https://github.com/fcrepo4-labs/fcrepo-import-export/blob/master/src/main/java/org/fcrepo/importexport/importer/Importer.java#L360-L362

By removing that test and fixing up some other bits of code to accommodate it, the tests pass and I imagine the behavior is fixed for you. I don't see any real problem with it, but I worry we might have had a good reason to intentionally prohibit importing the repository root, even if we didn't find a way to capture that reasoning in our integration tests. The git blame doesn't help, as it implicates me for introducing that behavior and I don't remember why. @escowles, do you recall any reasons why we wouldn't want to import the repository root?

escowles commented 7 years ago

@mikedurbin I took a look at git-blame to see when we inserted that logic to stop importing the root node, and it looks like you are the one who did it:

https://github.com/fcrepo4-labs/fcrepo-import-export/blame/master/src/main/java/org/fcrepo/importexport/importer/Importer.java#L360

Looking at those tickets, I don't see anything about the root node specifically, just general import failures. So if you don't remember anything about why we did that, I'm fine with removing it.