trellis-ldp / trellis

Trellis is a platform for building scalable Linked Data applications
https://www.trellisldp.org
Apache License 2.0
105 stars 21 forks source link

Redirections from Memento? #287

Closed ajs6f closed 5 years ago

ajs6f commented 6 years ago

I'm getting some very queer failures from MementoResourceIT. I'm really only concerned here with testMementoAcceptDateTimeHeader and testMementoAcceptDateTimeHeader, because I'm not at all sure how my backend could be triggering the emission of redirects. (The others I'm chalking up for now to plain old errors in my backend.)

@acoburn, any thoughts? Is there some condition under which a backend can return a resource that gets translated into a redirect by HTTP? I can't imagine why that would happen... let me know if there's any context or additional info that might clear this up.

[ERROR] Failures:
[ERROR]   LdpBinaryIT.testPatchBinaryDescription:117 Check that the ETag values are the same ==> expected: <"2b31f4da4305d4825fd8ad5281acfa61"> but was: <"30b217e7d59b2c13b25e845c380a6de9">
[ERROR]   MementoBinaryIT.testMementoAcceptDateTimeHeader Check for a successful memento request ==> expected: <SUCCESSFUL> but was: <REDIRECTION>
[ERROR]   MementoBinaryIT.testMementoDateTimeHeader Check that the memento-datetime header is correct ==> expected: <2018-11-07T16:26Z> but was: <2018-11-07T16:25:59Z>
[ERROR]   MementoResourceIT.testMementoAcceptDateTimeHeader Check for a successful memento request ==> expected: <SUCCESSFUL> but was: <REDIRECTION>
[ERROR]   MementoResourceIT.testMementoDateTimeHeader Check that the memento-datetime header is correct ==> expected: <2018-11-07T16:26:06Z> but was: <2018-11-07T16:26:05Z>
ajs6f commented 6 years ago

Just to give a slightly clearer report, I fixed up some of my impl's behavior with binaries based on the understanding I hope to confirm in https://github.com/trellis-ldp/trellis/issues/288 to get what I show below, but I'm still a bit lost on the redirections...

[ERROR] Failures:
[ERROR]   MementoBinaryIT.testMementoAcceptDateTimeHeader Check for a successful memento request ==> expected: <SUCCESSFUL> but was: <REDIRECTION>
[ERROR]   MementoBinaryIT.testMementoDateTimeHeader Check that the memento-datetime header is correct ==> expected: <2018-11-07T17:00:58Z> but was: <2018-11-07T17:00:57Z>
[ERROR]   MementoResourceIT.testMementoAcceptDateTimeHeader Check for a successful memento request ==> expected: <SUCCESSFUL> but was: <REDIRECTION>
[ERROR]   MementoResourceIT.testMementoDateTimeHeader Check that the memento-datetime header is correct ==> expected: <2018-11-07T17:01:01Z> but was: <2018-11-07T17:01Z>
ajs6f commented 6 years ago

On an additional note:

MementoResourceIT.testMementoDateTimeHeader Check that the memento-datetime header is correct ==> expected: <2018-11-07T17:01:01Z> but was: <2018-11-07T17:01Z>

So the difference is less than a second... is this actually tests being a little too picky? I'm going to go read through those tests...

ajs6f commented 6 years ago

After looking at the tests, it looks like you are comparing the dates of Mementos as given in the time map to the date they have in a header when the test actually retrieves them, so something is up here with my impl, it seems. Going back to my code...

acoburn commented 6 years ago

I'm also noticing some periodic errors related to off-by-one-second memento responses. I don't know if it has to do with the RFC 1123-format date to a Java Instant or if there's something else going on, but I'm investigating.

ajs6f commented 5 years ago

Just removed a comment indicating that fewer tests were failing for me-- that was a mistake on my part from a partial build.

ajs6f commented 5 years ago

Ok, I've gone into this with logging to the left and logging to the right, an I am now increasingly convinced that the world is insane. In a nutshell:

The code:

final ZonedDateTime zdt = ZonedDateTime.parse(date, RFC_1123_DATE_TIME);              
log.info("Computed zdt from date in memento link: {}", zdt.toString());
ZonedDateTime headerdate = ZonedDateTime.parse(res.getHeaderString(MEMENTO_DATETIME), RFC_1123_DATE_TIME);
log.info("Computed zdt from header on this memento: {}", zdt.toString());
assertEquals(zdt, headerdate, "Check that the memento-datetime header is correct");

produces (logging-wise):

INFO  16:26:26 Computed zdt from date in memento link: 2018-11-14T16:26:23Z
INFO  16:26:26 Computer zdt from header on this memento: 2018-11-14T16:26:23Z
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 5.507 s <<< FAILURE! - in edu.si.trellis.cassandra.MementoResourceIT
[ERROR] testMementoDateTimeHeader  Time elapsed: 0.112 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: Check that the memento-datetime header is correct ==> expected: <2018-11-14T16:26:23Z> but was: <2018-11-14T16:26:22Z>

That makes zero sense. OTOH, the Link header in question is

<http://localhost:56230/1084c290-ba9f-460e-9b94-9eda36c8589c/ebf2ea6c-1c2f-4c6f-844b-6a5323053fa8?version=1542212782118>; rel="memento"; datetime="Wed, 14 Nov 2018 16:26:23 GMT"

and the

Memento-Datetime : [Wed, 14 Nov 2018 16:26:22 GMT]

So the test is correct that this is bad behavior, but how? How are those headers being generated (from the SAME METADATA)? Presumably, one is reading MementoService::mementos and the other, Resource::getModified, but those two functions are reading the same data for me! (And I would think for any other impl.)

Ok, diving into trellis-http.

ajs6f commented 5 years ago

Another way to put it: how is

Memento-Datetime : [Wed, 14 Nov 2018 16:26:22 GMT]

parsed into:

2018-11-14T16:26:23Z

? Whatever my impl is doing, something doesn't make sense here.

acoburn commented 5 years ago

I think that we need to truncate the Instant values to SECOND precision, but I need to confirm that. I was looking at this last week and saw this sort of off-by-one second issue with date-time parsing and couldn't figure it out; I keep trying. I had a local branch where I could reliably trigger this error, but I need to do some rebasing to bring it up to the current state of the repo.

acoburn commented 5 years ago

I may have found the culprit. I'm running the test suite, and unless that leads to a bunch of other changes, I'll have a PR for you to test in just a sec.

ajs6f commented 5 years ago

The PR works to fix the funky datetime comparisons. The redirects are still there, but they very much seem like something different-- the NonRDFSource description thing.

acoburn commented 5 years ago

This should now be resolved, correct?

ajs6f commented 5 years ago

I'd like to do one final "from scratch" build against master (doing that now), and then yes, and I will close this. Cool?

ajs6f commented 5 years ago

Done.