Skip to content

Conversation

nupplaphil
Copy link
Collaborator

@nupplaphil nupplaphil commented Dec 22, 2019

Fixes friendica/friendica#7989

Types of hook-calls:

  • Hook::register() just registers hooks, but doesn't add them to the current call
  • Hook::add() just adds hook to the current call, but doesn't register them

Since we want to call the db-definition hook just once during the install process, I switched from Hook::register() to Hook:add() and therefore removed the unregister call as well.

@MrPetovan
Copy link
Collaborator

MrPetovan commented Dec 22, 2019

I think this change is wrong. If the hook isn’t registered, the next time the DB update routine is called, the addon table won’t be added to the definition.

I’d rather the issue be fixed by calling Hook::add to Hook::register instead.

@nupplaphil
Copy link
Collaborator Author

I think tis change is wrong. If the hook isn’t registered, the next time the DB update routine is called, the addon table won’t be added to the definition.

But why do we need it here? The DB-upate routine is called directly here:

DBStructure::update($a->getBasePath(), false, true);

And we don't need it at other places, so why should we store this hook?

I’d rather the issue be fixed by calling Hook::add to Hook::register instead.

This was my first thought, but than I found this description:
https://github.com/friendica/friendica/blob/cc64471e4c4cbda6f79c50ca784d0b927a45a6a5/src/Core/Hook.php#L67

My interpretation is that it is intentionally not added here. And I don't know what side-effects could cause it to other hooks. So I thought better stay at one point where only one addon could get broken instead of brick the whole hook-mechanism ;-)

@MrPetovan
Copy link
Collaborator

MrPetovan commented Dec 23, 2019

And we don't need it at other places, so why should we store this hook?

Imagine if we decide to delete tables absent from the definition? The addon table would be wiped out the next time the DB update routine is called.

A middle of the road approach would be to keep the register call and just add the add call.

@nupplaphil
Copy link
Collaborator Author

And we don't need it at other places, so why should we store this hook?

Imagine if we decide to delete tables absent from the definition? The addon table would be wiped out the next time the DB update routine is called.

A middle of the road approach would be to keep the register call and just add the add call.

Fair enough

@MrPetovan MrPetovan merged commit 98e9cc7 into friendica:2019.12-rc Dec 23, 2019
@nupplaphil nupplaphil deleted the bug/7989-advancedcontfilter branch December 23, 2019 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants