Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relaxed referential integrity rules for RELS-INT. #25

Merged
merged 1 commit into from Jun 25, 2015

Conversation

Projects
None yet
3 participants
@mikedurbin
Copy link
Contributor

commented Jun 15, 2015

@@ -19,7 +19,7 @@
</foxml:datastreamVersion>
</foxml:datastream>
<foxml:datastream ID="DS0" STATE="A" CONTROL_GROUP="X" VERSIONABLE="false">
<foxml:datastreamVersion ID="DS0.0" LABEL="Example Managed binary datastream" CREATED="2015-05-13T19:51:09.487Z" MIMETYPE="text/xml" SIZE="30">
<foxml:datastreamVersion ID="DS0.0" LABEL="Example Managed binary datastream" CREATED="2015-05-13T19:51:09.488Z" MIMETYPE="text/xml" SIZE="30">

This comment has been minimized.

Copy link
@awoods

awoods Jun 16, 2015

Member

Is this update designed to affect the integration tests to demonstrate the issue of a DS not yet being created?

This comment has been minimized.

Copy link
@mikedurbin

mikedurbin Jun 16, 2015

Author Contributor

Yeah. This change duplicates the conditions in which the issue occurs, and without the changes above (or something comparable) the integration test will fail.

This comment has been minimized.

Copy link
@awoods

awoods Jun 16, 2015

Member

Thanks. Can you reach out to @bcail for verification. Then we can merge this.

@awoods

This comment has been minimized.

Copy link
Member

commented Jun 16, 2015

It would be good to get @bcail's verification of this PR.

@bcail

This comment has been minimized.

Copy link

commented Jun 16, 2015

The test looks good to me, although I haven't actually run the new code. I guess for testing the PR I'd need to clone Mike's repo & use the maven system? How often are new jars created?

@awoods

This comment has been minimized.

Copy link
Member

commented Jun 16, 2015

@bcail

This comment has been minimized.

Copy link

commented Jun 16, 2015

I cloned Mike's repo & ran it on my data with the mvn command. I'm getting the following (different) error:
DEBUG 15:26:03.716 (BasicObjectVersionHandler) Considering changed datastream version RELS-INT.0
[WARNING]
java.lang.reflect.InvocationTargetException
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:497)
at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:293)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
at org.fcrepo.migration.handlers.BasicObjectVersionHandler.updateResourceProperties(BasicObjectVersionHandler.java:491)
at org.fcrepo.migration.handlers.BasicObjectVersionHandler.migrateRelsInt(BasicObjectVersionHandler.java:439)
at org.fcrepo.migration.handlers.BasicObjectVersionHandler.processObjectVersions(BasicObjectVersionHandler.java:133)
at org.fcrepo.migration.handlers.VersionAbstractionFedoraObjectHandler.processObject(VersionAbstractionFedoraObjectHandler.java:95)
at org.fcrepo.migration.handlers.ObjectAbstractionStreamingFedoraObjectHandler.completeObject(ObjectAbstractionStreamingFedoraObjectHandler.java:67)
at org.fcrepo.migration.foxml11.Foxml11InputStreamFedoraObjectProcessor.processObject(Foxml11InputStreamFedoraObjectProcessor.java:124)
at org.fcrepo.migration.Migrator.run(Migrator.java:101)
at org.fcrepo.migration.Migrator.main(Migrator.java:37)

@mikedurbin

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2015

Could you copy the line 439 of your BsaicObjectVersionHandler @bcail so I can figure out what's null?

@bcail

This comment has been minimized.

Copy link

commented Jun 17, 2015

439 updateResourceProperties(ds, triplesToRemove, triplesToInsert);

@mikedurbin

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2015

@bcail I'm sorry I can't seem to figure out what's wrong. I'm not sure how that line could result in a NPE, since no objects are dereferenced.

@bcail

This comment has been minimized.

Copy link

commented Jun 24, 2015

@mikedurbin maybe ds is null? In any case, though, there's a unit test - maybe the PR should just be merged. I can try again with the latest master after it's merged...

@awoods

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

@mikedurbin, do we merge this?

@mikedurbin

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2015

It's possible (likely) there's an outstanding bug that results in the NullPointerException, whether it was introduced by this commit or not is unclear. I'm pretty sure this PR addresses the issue described in the ticket.

@mikedurbin

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2015

So yeah... if you're asking me... let's merge it and move on to the next issue people run across.

awoods added a commit that referenced this pull request Jun 25, 2015

Merge pull request #25 from mikedurbin/issue-24
Relaxed referential integrity rules for RELS-INT.

@awoods awoods merged commit 26f49af into fcrepo4-exts:master Jun 25, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.