Ajax ils offline message

[lahmann]

ILS->getOfflineMode() is currently only implemented for the NoILS driver but is pretty useful to check whether an ILS is actually online in order to improve user experience (i.e. reporting if the ILS is currently offline). We implemented connection-checks in our custom DAIA/PAIA driver which might also be a good addition to the current DAIA and the pending PAIA driver. The current implementation in the templates though might lead to long loading times if the ILS is offline and the check i.e. has to wait for a timeout. This is especially critical for the VuFind homepage which will hang until ILS->getOfflineMode() returns anything.
This pull request proposes to call ILS->getOfflineMode() on the homepage via Ajax to make sure loading times are as fast as possible (and do not rely on availability of remote services). Other possible candidates for Ajax-calls would be myresearch/cataloglogin.phtml and myresearch/login.phtml. RecordTab/holdingsils.phtml is mostly depending on the ILS so checking for ILS-availability via Ajax does not add any benefit while trying to obtain holdings data from the ILS during view rendering. Assuming that getRealTimeHoldings() always depends on the ILS this call should be safe to skip if the ILS is reported to be offline, saving another call to an ILS reported as offline.

The integration of the javascript in search/home.phtml can probably be improved and/or moved to the common.js allowing to call the check from the above mentioned templates as well.
I also moved the ils-offline message into a single template for code reuse and better maintenance.

[crhallberg]

The only way I can think to improve the JS is if we had a function in common.js that looks for a class or id (#ils-offline for example). If an element with that class is present, launch the AJAX and put the response in the element. That way, we can add a div to every template we want those messages to appear in.

If we don't worry about non-JS users, we could replace all of the other calls to getOfflineMode() with just a div.

[lahmann]

Your suggestion of moving the JS into common.js and searching for #ils-offline sounds good to me.

I'm not sure if it's a good idea to replace all calls to getOfflineMode() with js-calls: e.g. for holdings the ILS will be contacted for holding-information anyway and getOfflineMode() would rather be used as a check for ILS availability before firing all the holding queries.

[demiankatz]

The reason that only NoILS implements getOfflineMode is that there is an assumption that users who desire this functionality will use the "loadNoILSOnFailure" configuration setting. See this code:

vufind/module/VuFind/src/VuFind/ILS/Connection.php

Line 121 in 0c08509

// If we're configured to fail over to the NoILS driver, we need

If the ILS driver throws an exception during initialization, then NoILS gets loaded instead.

So rather than implementing getOfflineMode in the DAIA driver, it might make sense to have DAIA's initialization routine throw an exception when the ILS is offline.

This approach is not without disadvantages, though -- it currently incurs a cost, since we have to initialize the ILS driver -- an expensive operation in many cases -- on every page load in order to be able to handle things like hiding login when the ILS is offline. I'm certainly open to better implementations. I'm just not sure what the best approach might be.

Note that there are already problems with the MultiBackend driver not being able to detect offline status effectively (at least, not without doing an inappropriate amount of initialization). There's also another open pull request that touches on this -- #736, which improves Voyager driver performance by deferring database initialization, but which probably breaks offline ILS detection in the process.

One thought for a more complicated mechanism that might be more effective than the current approach: rather than checking the ILS availability every time, dedicate a specific exception to the "ILS Offline" scenario. If that exception gets thrown, catch it very high in the code, set a flag in the session indicating that the ILS is offline, and refresh the page. When the session flag is set, put VuFind into offline mode, but retry the ILS connection every X minutes so that you can clear the flag if necessary. This seems awfully convoluted, but it might greatly improve day-to-day performance.

Regarding other aspects of this PR, I definitely like the idea of moving the offline message to a helper template to cut back on redundancy. I'm not sure why you chose to load that by AJAX on search/home, though -- I suppose in theory that might reduce load on the ILS, but as currently implemented, I think the only real effect it has is to prevent non-JS users (if such creatures exist) from seeing the message.

Perhaps we can try to move forward on merging some of the non-controversial parts of this PR and then continue discussing the bigger issues. What do you think?

[demiankatz]

See also #768 for more conversation that may be relevant to this work.

[lahmann]

Thanks for pointing me to #768 - this definitely touches the topic of this pull request. Regarding your previous comments:

1.)

If the ILS driver throws an exception during initialization, then NoILS gets loaded instead.

So rather than implementing getOfflineMode in the DAIA driver, it might make sense to have DAIA's initialization routine throw an exception when the ILS is offline.

My first implementation did exactly this - the problem here is, that ILS driver init() is done every time during runtime. If the ILS is offline for whatever reason or the route fails in worst case the test in init() will last until timeout of Zend/Http is hit (and throws an exception eventually). This behaviour is especially critical if the ILS is not essential for the page being requested (e.g. search/home) as it will be not responsive for the user. For this reason I figured it would be better to decide at which point to actively trigger a status check of the ILS and to decouple any potentially page blocking ILS calls through Ajax-calls.

This pull request certainly is no alternative to #768 - more consistent exception handling and ILS-offline reporting would be welcome - but it addresses in my opinion an issue that will always rise by design: any ILS-health-check might result in a connection timeout in worst case (unless we define a limit - but than we have to find the golden balance between enough response time for the ILS and site-responsiveness for the user). Through checking the ILS asynchronously we can minimize the nuisance for the user.
Views that entirely depend on the ILS still have to rely on proper ILS-offline behavior (as being worked on in #768).

2.) Multibackends: I haven't thought about those - maybe one would have to implement an appropriate method in MultiBackend driver to handle differing statuses from the configured ILSes. On the other hand as long as ILS->getOfflineMode() is not implemented it will default to false and causes no harm.

3.)

I'm not sure why you chose to load that by AJAX on search/home, though -- I suppose in theory that might reduce load on the ILS, but as currently implemented, I think the only real effect it has is to prevent non-JS users (if such creatures exist) from seeing the message.

If the configured ILS has implemented getOfflineMode() doing a health check for the ILS, calling getOfflineMode() during template rendering an actually offline ILS will block the page. We have had situations when the ILS was offline causing search/home to hang and preventing the user from searching. The usage of Ajax solves this problem.
I agree that non-JS users won't see the status message, but they will when trying to login. In this case I think that keeping the catalog's landing page operational is more favorable than to provide the user with information on the status of the ILS even before she decides to interact with it.

I'm happy to split the pull request - to get the ils-offline template into master straight away - if the above discussed topics are still controversial. Let me know what you think!

[demiankatz]

André,

Reviewing this again, it looks to me like there are three parts to this pull request:

1.) Refactor offline message to a separate template.

2.) Don't bother doing AJAX status lookups when we're in offline mode.

3.) Load offline status via AJAX on home page.

I think parts 1 and 2 are uncontroversial, and we might as well merge them.

I think part 3 might actually be part of the solution to the problem I describe in my comments on #768 where I talk about the deferred loading preventing the offline message from displaying until later in the login process. If we have an AJAX health check, then at least for JS-enabled users, we have a greater chance of successfully displaying a warning message on the home page, but without sacrificing the speed of the initial page load. However, I'm not sure if this will work exactly as-is in combination with the new mechanism -- I suspect we may need to add some additional tests to the AJAX routine (and possibly a new ILS driver method) to trigger a health check.

My proposal: let me merge parts 1 and 2 to master, move part 3 over to #768, and then close this PR. We can continue discussion of the ideal health check mechanism on #768, and the other changes will help lay the groundwork for our final approach to the problem.

If you agree that this sounds reasonable, I can take care of the necessary merging in the next day or two. Let me know what you think!

[lahmann]

Demian, your proposal sounds very reasonable! Let's get 1+2 merged into master and continue with 3 in #768. Let me know if you need any assistance.

[demiankatz]

@lahmann, in taking a closer look at this to prepare for merging, I realized there are actually some problems:

1.) The template refactoring does not account for the fact that we use different messages in different contexts (there's ils_offline_home_message, ils_offline_holdings_message and ils_offline_login_message used in different contexts in the master code -- see https://github.com/vufind-org/vufind/blob/master/languages/en.ini#L494; this branch reverts everything to ils_offline_home_message). I'm not sure if this variation is an argument against the refactoring, but at very least we need to make the template capable of accepting a $message variable so that we can show the right message in each place. Let me know what you think, and whether you'd like me to make that change.

2.) Disabling AJAX status when in offline mode is actually a problem, because the NoILS driver can be configured to respond to AJAX requests using offline data (so, for example, it can still fetch call numbers from MARC records even if the ILS is offline). If we disable that, we lose the functionality. Should we simply drop this change, or does it serve a critical need for you? If it's a critical need, can we implement differently? (In theory, if getOfflineMode returns anything other than false, it means we're using NoILS, so making that AJAX call should not be expensive... unless you're using getOfflineMode in a different way in your own driver).

Sorry I didn't catch these things earlier! Please let me know how you think we should proceed.

[lahmann]

Demian, thanks for your comments and no problems - I should have spotted the slight difference in ils offline message in the first place. I just pushed a commit that fixes that issue.

Regarding disabling AJAX status I agree that the useful functionality of the NoILS driver would get lost so disabling the AJAX status when getOfflineMode returns anything than false is not quite optimal. With the new code in #768 this should get handled more gracefully and keep the NoILS functionality. I removed the code with the latest commit.

If you think the pull request is ready to merge we should probably continue discussion on checking the ILS on the home page in #768.

[demiankatz]

The template refactoring has been merged to master and the AJAX logic has been moved into #768, so I am closing this PR. Thanks again!