Migration of Login Token Service from Laminas to Doctrine.

[padmasreegade]

Migration of Login Token Service from Laminas to Doctrine.

[demiankatz]

Thanks, @padmasreegade! I noticed a few things that look like they may be problems -- see below for details. If all tests are passing, perhaps this is an indication that some tests are not as complete as they should be. Let me know if you need help exercising any particular parts of the code; I may be able to find time to add more tests if necessary.

[demiankatz]

Thanks, @padmasreegade -- I think this is getting very close to being finished, but there are a couple of things that still need adjustments.

Please let me know if you need help reproducing or testing anything. Unfortunately, we don't have automated testing in place for the persistent login code, so it requires some manual testing. That's probably something we should address, but the dependency on browscap data makes it slightly complicated to set up the tests.

[demiankatz]

Thanks, @padmasreegade!

See below for a few ongoing issues (one minor, the other fairly challenging).

Also note that I merged the latest upstream changes into this PR for you, and while I was testing, I also made a couple of small adjustments (see commit 48139a5, which uses objects instead of integers to represent users in queries... I think the other way works too, but this feels more formal).

[demiankatz]

Unfortunately, there appears to be a new problem here.

I logged in with a persistent login, then I closed my browser and reopened it. When I returned to the site, I got a blank white screen. Inspecting my Apache error log revealed this fatal error:

[Fri Nov 15 15:33:30.924739 2024] [php:error] [pid 7079] [client 10.0.2.2:61865] PHP Fatal error:  Uncaught Error: Call to a member function getUser() on array in /usr/local/vufind/module/VuFind/src/VuFind/Db/Service/LoginTokenService.php:110\nStack trace:\n#0 /usr/local/vufind/module/VuFind/src/VuFind/Auth/LoginTokenManager.php(143): VuFind\\Db\\Service\\LoginTokenService->matchToken()\n#1 /usr/local/vufind/module/VuFind/src/VuFind/Auth/Manager.php(558): VuFind\\Auth\\LoginTokenManager->tokenLogin()\n#2 /usr/local/vufind/module/VuFind/src/VuFind/Auth/Manager.php(601): VuFind\\Auth\\Manager->getUserObject()\n#3 /usr/local/vufind/module/VuFind/src/VuFind/Auth/Manager.php(611): VuFind\\Auth\\Manager->getIdentity()\n#4 /usr/local/vufind/module/VuFind/src/VuFind/Auth/ManagerFactory.php(96): VuFind\\Auth\\Manager->checkForExpiredCredentials()\n#5 /usr/local/vufind/vendor/laminas/laminas-servicemanager/src/ServiceManager.php(642): VuFind\\Auth\\ManagerFactory->__invoke()\n#6 /usr/local/vufind/vendor/laminas/laminas-servicemanager/src/ServiceManager.php(264): Laminas\\ServiceManager\\ServiceManager->doCreate()\n#7 /usr/local/vufind/module/VuFind/src/VuFind/RateLimiter/RateLimiterManagerFactory.php(89): Laminas\\ServiceManager\\ServiceManager->get()\n#8 /usr/local/vufind/vendor/laminas/laminas-servicemanager/src/ServiceManager.php(642): VuFind\\RateLimiter\\RateLimiterManagerFactory->__invoke()\n#9 /usr/local/vufind/vendor/laminas/laminas-servicemanager/src/ServiceManager.php(264): Laminas\\ServiceManager\\ServiceManager->doCreate()\n#10 /usr/local/vufind/module/VuFind/src/VuFind/Bootstrapper.php(390): Laminas\\ServiceManager\\ServiceManager->get()\n#11 /usr/local/vufind/vendor/laminas/laminas-eventmanager/src/EventManager.php(319): VuFind\\Bootstrapper->VuFind\\{closure}()\n#12 /usr/local/vufind/vendor/laminas/laminas-eventmanager/src/EventManager.php(177): Laminas\\EventManager\\EventManager->triggerListeners()\n#13 /usr/local/vufind/vendor/laminas/laminas-mvc/src/Application.php(319): Laminas\\EventManager\\EventManager->triggerEventUntil()\n#14 /usr/local/vufind/public/index.php(23): Laminas\\Mvc\\Application->run()\n#15 {main}\n  thrown in /usr/local/vufind/module/VuFind/src/VuFind/Db/Service/LoginTokenService.php on line 110

I think the problem may be that the getBySeries() method is returning an unstructured array instead of an array of entities; it's possible that changing getArrayResult() to getResult() at the end of that method will help, but when I tried that, I got a different fatal error... so it seems that some more digging and troubleshooting is needed.

Sorry to be the bearer of frustrating news! :-)

[demiankatz]

Thanks, @padmasreegade, your latest changes look like a step in the right direction. I just made a small commit to adjust some @return annotations that I don't think needed to be changed. I'll give this more thorough testing when I return from vacation!

[padmasreegade]

[like] Padmasree Gade reacted to your message:

…

________________________________ From: Demian Katz ***@***.***> Sent: Tuesday, November 26, 2024 3:54:24 PM To: demiankatz/vufind ***@***.***> Cc: Padmasree Gade ***@***.***>; Mention ***@***.***> Subject: [EXTERNAL] Re: [demiankatz/vufind] Migration of Login Token Service from Laminas to Doctrine. (PR #35) Thanks, @padmasreegade<https://github.com/padmasreegade>, your latest changes look like a step in the right direction. I just made a small commit to adjust some @return annotations that I don't think needed to be changed. I'll give this more thorough testing when I return from vacation! — Reply to this email directly, view it on GitHub<#35 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BCM24WZTHI7QZVFR6NFACOL2CSKTBAVCNFSM6AAAAABPO3G7P6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBRGIZTAOJUGA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[demiankatz]

Just a couple of minor stylistic suggestions based on another readthrough.

I'm going to do both automated and hands-on testing now, so I may have a follow-up with more feedback depending on how that goes. :-)

[demiankatz]

Looking good, and all tests passing. Thanks, @padmasreegade!