6.0.0-git
2018-12-16

[#11124] decode_attribute hook an exceptions
Summary decode_attribute hook an exceptions
Queue Turba
Queue Version 3.0.12
Type Enhancement
State Feedback
Priority 1. Low
Owners
Requester kd (at) tu-cottbus (dot) de
Created 2012-04-02 (2449 days ago)
Due
Updated 2012-04-04 (2447 days ago)
Assigned
Resolved
Milestone
Patch No

History
2012-04-04 13:41:56 kd (at) tu-cottbus (dot) de Comment #11
New Attachment: Object.php[1].patch Download
Reply to this comment
Yet another try:

Change the logic in getValue:
if composite filed - compose it, (don't return)
if hook is defined - call it (don't return)
if image - wrap it into array
return
2012-04-04 08:13:21 kd (at) tu-cottbus (dot) de Comment #10 Reply to this comment
I can imagine a field that requires different inputs from the user, 
i.e. different attributes, but the result should be rendered in a 
single composite field, after passed through the decode hook.
Just for completeness, can you be a bit more specific, i don't understand,
why the composite field is essential here.

For now i have no new arguments/ideas. Returning an array is ok.

Btw. i havn't had a closer look at the opposite side (setValue/encode),
composite fields seem to be a special case here too (not editable in
the contact form)
2012-04-03 20:42:10 Jan Schneider Comment #9 Reply to this comment

[Show Quoted Text - 14 lines]
I can imagine a field that requires different inputs from the user, 
i.e. different attributes, but the result should be rendered in a 
single composite field, after passed through the decode hook.
Well, maybe we could return an array in
Turba 4 instead, with a boolean as the first element, whether the hook
has been called.
Is it possible to do it the old-fashioned way,  pass the value by 
reference, and return a
status code?
No, for that to work, the arguments need to passed by reference to 
Horde::callHook() too.
2012-04-03 20:07:38 kd (at) tu-cottbus (dot) de Comment #8 Reply to this comment
This is debatable, but I don't see a reason to artificially limit 
which fields we allow to pass through the hook, just for code-flow 
reasons.
A composite field is defined in terms of "turba-attributes" (left-hand 
side of the map),
this is different from directly  to "backend attributes" mapped 
"turba-attributes".
What is passed to the hook as the value of a composite field?
I can't imagine a situation, where a composite field *and* a hook for 
that field makes sense.
I'd always use a direct mapping and do the other work in the hook.
Isn't a composite field not some kind of simple inline hook?
Well, maybe we could return an array in Turba 4 instead, with a 
boolean as the first element, whether the hook has been called.
Is it possible to do it the old-fashioned way,  pass the value by 
reference, and return a
status code?

2012-04-03 12:23:29 Jan Schneider Comment #7 Reply to this comment

[Show Quoted Text - 21 lines]
Not easily possible, because the values aren't necessarily scalar values.
- is it really desired, to have composite fields processed by the hook?
They are a combination of  driver values (that should be processed)
This is debatable, but I don't see a reason to artificially limit 
which fields we allow to pass through the hook, just for code-flow 
reasons.
- are images in this sense not some special kind of composite fields?
Not really.
2012-04-03 11:56:47 kd (at) tu-cottbus (dot) de Comment #6 Reply to this comment

[Show Quoted Text - 16 lines]
Just a few thoughts:

- to detect, whether or not an attribute has processed, one could
compare the value before and after the hook
- is it really desired, to have composite fields processed by the hook?
They are a combination of  driver values (that should be processed)
- are images in this sense not some special kind of composite fields?


2012-04-03 08:37:12 Jan Schneider Comment #5 Reply to this comment
We may want to introduce a new Exception class that is thrown to skip 
hook calls and that's not being logged in Horde::callHook().
It's still a performance degradation to throw and catch so many 
exceptions, but I don't see an alternative to reliably skip processing 
of individual attributes. Well, maybe we could return an array in 
Turba 4 instead, with a boolean as the first element, whether the hook 
has been called.
2012-04-03 08:32:12 Jan Schneider Comment #4 Reply to this comment
To get  the pictures in 'localsql' back, i have to throw an 
exception instead of the
default passthrough 'return $value'.
Yes, because that's how we detect if an attribute was *not* processed 
by a hook, and thus continue the further processing of composite and 
image fields further down in getValue(). If you moved the hook calling 
further down, you wouldn't be able to hook into those fields at all.
Now it works as expected, but i have tons of "No decode handler" 
errors in the Logs.
What I'm doing wrong here?
Nothing, though I admit that having the logs spammed with false 
warnings isn't optimal.
2012-04-03 06:00:51 kd (at) tu-cottbus (dot) de Comment #3 Reply to this comment
The purpose of the hook *is* to be called for any attribute. I don't 
understand what you trying to achieve, besides that it would break 
the intended behavior.
hmm,
i have a personal adressbook 'localsql' (nothing special, mainly the 
sample code) and
a global LDAP adressbook 'localldap'. Now i have no field for e.g. ' 
freebusyUrl' in LDAP.
So i started with:
class Turba_Hooks
{
      public function decode_attribute($attribute, $value, $contact)
      {
            switch ($attribute) {
            case 'freebusyUrl':
                   ....
                   return $url;
            }
           return $value;
      }
}
Now I have 'freebusyUrl' in 'localldap', but magically lost all 
pictures in 'localsql'.
To get  the pictures in 'localsql' back, i have to throw an exception 
instead of the
default passthrough 'return $value'.
Now it works as expected, but i have tons of "No decode handler" 
errors in the Logs.
What I'm doing wrong here?
2012-04-02 21:57:36 Jan Schneider Comment #2
State ⇒ Feedback
Reply to this comment
The purpose of the hook *is* to be called for any attribute. I don't 
understand what you trying to achieve, besides that it would break the 
intended behavior.
2012-04-02 08:54:49 kd (at) tu-cottbus (dot) de Comment #1
Type ⇒ Enhancement
State ⇒ New
Priority ⇒ 1. Low
Summary ⇒ decode_attribute hook an exceptions
Queue ⇒ Turba
Milestone ⇒
Patch ⇒ No
New Attachment: Object.php.patch Download
Reply to this comment
playing with a decode_attribute hook, i noticed, that it only works correctly,
if for each and every attribute in every source that is not handled by 
the hook,
an exception must be thrown
(at least composite fields and images are affected).
In real life only a few attributes from one source need conversion.
One possible solution would be a change in the logic of the getValue method
in Object.php (move the Hook call near the end).

Saved Queries