Refactor more methods to the SearchService#3775
Conversation
demiankatz
added
the
architecture
| * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License | ||
| * @link https://vufind.org/wiki/development Wiki | ||
| */ | ||
| class SecretCalculator |
There was a problem hiding this comment.
I don't love this class, but the search unsubscribe calculation had to go into a shared location, and it seemed better to create a new class than to overload the existing HMAC class, which seemed like inappropriate scope creep.
There was a problem hiding this comment.
Well at least the name of the class made me smile. 😄
| ); | ||
| return false; | ||
| } | ||
| if (!$user = $s->getUser()) { |
There was a problem hiding this comment.
Due to changes in the way the search and user entities relate to one another, it seemed cleanest to eliminate the user caching logic in order to simplify the code and reduce dependencies. In some situations, this may put slightly higher load on the database, but looking up a user by primary key is not an expensive operation, so I'm not too concerned about it.
| * | ||
| * @return void | ||
| */ | ||
| public function testNotificationsWithMissingUser() |
There was a problem hiding this comment.
Due to changes in user handling in the command, this test no longer applies, so I simply deleted it.
| * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License | ||
| * @link https://vufind.org/wiki/development Wiki | ||
| */ | ||
| class SecretCalculator |
There was a problem hiding this comment.
Well at least the name of the class made me smile. 😄
| * @return void | ||
| */ | ||
| public function testDestroy(): void | ||
| public function ztestDestroy(): void |
There was a problem hiding this comment.
This is how I disable tests, but no idea why I disabled that and forgot to re-enable it. Definitely not intentional; reverted now (and the test is passing :-) ).
2 participants
This fills in more methods of the search service. Note that some of the work here is redundant with #3732 because I had to copy some test refactoring from there to get things passing here. If that PR is reviewed first, it will reduce the diffs here. Also note that I haven't updated the expiration command yet; that will be easier after #3714 is merged.
TODO