Refresh stale bookstats without page reload

[cblanken]

Lute's main page fails to reload bookstats from a cached page.

Currently bookstats are only refreshed when a client requests the main page at /. When a book is opened, it's bookstats are deleted such that when a user navigates to a cached version of the main page (e.g. they use their browser's back button) the necessary bookstats record is not refreshed.

To remediate this, the refresh_stats call has been relocated to a new route at /refresh_stale_stats which is called by the root index.html to refresh stats whether it has been cached or not.

I've done some limited testing in the dev environment with the default books and this solution seems to work well. If there are any concerns with this approach or new tests that should be added please let me know and I'll implement as soon as I can. Thanks.

[cblanken]

A short video of the fix.

https://github.com/user-attachments/assets/9e023501-5335-4ba9-9570-20afc017e11b

[jzohrab]

Hey @cblanken - I get the idea but wonder if it will still have staleness issues. At least it will likely deal with the "back" button problem when you mentioned. Thoughts? All in all, I don't have any objections to your idea, it looks like a decent solution to me :-) Thanks!

[cblanken]

I don't really think there any problems with the "staleness" mechanism. As far as I can tell, an opened book's stats are always deleted when opening so they should be refreshed when the main page is re-opened. The main issue is the caching. Actually thinking about it some more, it may be better to just restrict caching for the main page with an HTTP header (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control). It would basically force a reload on the main page whenever you navigated to it but I think it would fix it without the data-race concerns.

[jzohrab]

I think it would fix it without the data-race concerns.

Ah right maybe that's easiest, if it works. I'm outside of my realm of limited expertise here.

[cblanken]

I think it would fix it without the data-race concerns.

Ah right maybe that's easiest, if it works. I'm outside of my realm of limited expertise here.

A bit out of my wheelhouse too to be honest.

The synchronous fetch accomplishes the same thing though. And this way there's no need to rely on the user's browser to respect the caching headers. Although I'm sure they would.

[cblanken]

I don't know why all those tests failed, I'll have to look into them later.

[jzohrab]

I don't know why all those tests failed, I'll have to look into them later.

Re-ran them manually to check, still failing. Those are acceptance tests failures, run with inv accept. (stop your current running dev lute before running it :-) )

[cblanken]

@jzohrab Okay, so I'm not super familiar with how pytest-bdd works, but I think they're all failing because the tests don't await for the table load. Adding a short _sleep to the given_visit let's all the acceptance tests pass.

@given(parsers.parse('I visit "{p}"'))
def given_visit(luteclient, p):
    "Go to a page."
    luteclient.visit(p)
    _sleep(0.1)

I really don't like that solution, but I don't have time to really dig into refactoring all those failing test to await the table load with a timeout instead of using _sleeps.

[jzohrab]

Adding a sleep is fine, it's a minimal delay. I'll check the final failure.

[jzohrab]

Ah I guess you need to push that commit that adds the delay :wave: -- ping me when it's pushed, @cblanken . Thank you again!

[cblanken]

@jzohrab Sorry, I was mistaken. There appears to be some other issues even with the extra sleep. The selenium/splinter errors are too opaque for me to figure what's going wrong though.

Moving the DataTable initialization out of the .then seems to let all the tests pass though it throws some SQLAlchemy errors into the output too.

I can revert it to my original solution if you want, but trying to pick apart the tests is too much for me at the moment.

[jzohrab]

Cheers @cblanken I’ll check it out. Thanks!

[jzohrab]

Looked briefly, but thought that your idea of not caching the main page might be better, so am trying that out in #468. Let's see how the tests do.

[jzohrab]

@cblanken - the non-caching approach of #468 worked, all tests pass. Seems like a reasonable approach to me -- great idea! -- so I'll merge that in and will close this one. Cheers!