6.0.0-git
2019-10-23

[#10880] Rdo: Make *- to-one-relations aware of "no relation" state
Summary Rdo: Make *- to-one-relations aware of "no relation" state
Queue Horde Framework Packages
Queue Version Git master
Type Enhancement
State Assigned
Priority 1. Low
Owners lang (at) b1-systems (dot) de
Requester lang (at) b1-systems (dot) de
Created 2011-12-22 (2862 days ago)
Due
Updated 2017-01-02 (1024 days ago)
Assigned 2016-01-28 (1364 days ago)
Resolved
Milestone
Patch No

History
2017-01-02 17:16:35 Ralf Lang (B1 Systems GmbH) Comment #7 Reply to this comment
Unfortunately, the join code for eager relations is not trivial to 
patch, at least I did not find an elegant way. to say "join result AND 
everything where the join key is null.

The alternative is also not great:

Always use the lazy code path (but on load instead of on request)

For broken relation keys, my current approach is to overload certain 
mappers to null the key on failure or on ::delete of the partner. It's 
sort of messy and should best be handled in the relation configuration:

          'hosts' => array('type' => Horde_Rdo::ONE_TO_MANY,
                           'foreignKey' => 'device_id',
                           'onDelete' => ['delete' | 'removeRelation']
                           'mapper' => 'APP_HostMapper'),


2016-01-28 16:46:47 Jan Schneider State ⇒ Assigned
 
2012-03-15 19:22:30 Ralf Lang (B1 Systems GmbH) Assigned to Ralf Lang (B1 Systems GmbH)
 
2012-01-09 20:31:52 Ralf Lang (B1 Systems GmbH) Comment #6 Reply to this comment
It seems like lazy [one|many]_to_one relations are resolved by a 
different part of code than eager relations.

lazy relations:
Base.php __get triggers a findOne on the target mapper with either a 
supplied query object (undocumented?) or the defined foreignKey 
field's value. A findOne without restrictions catches the first 
object, if any exist or null otherwise. I think I have fixed THIS

eager relations:
REFERENCING Objects with broken eager relations of either type (null 
value or wrong key value) simply aren't created. No exception is 
thrown until code wants to access the object's properties.
This seems to be handled by Horde_Rdo_Query generating an INNER JOIN 
which results in no object
No idea how to tackle this.
2012-01-09 20:31:49 Git Commit Comment #5 Reply to this comment
Changes have been made in Git for this ticket:

Fix lazy to-one relations as in ticket #10880 Add tests for base 
objects not loaded when eager to-one relations don't work Change tests 
to reflect lazy relations now work as expected for failing cases

  11 files changed, 97 insertions(+), 44 deletions(-)
http://git.horde.org/horde-git/-/commit/b0cfa915b40a5f61321b446bd18bebbe07f6fd94
2012-01-09 16:20:15 Git Commit Comment #4 Reply to this comment
Changes have been made in Git for this ticket:

Add some unit tests to Horde_Rdo 
testToOneRelationReturnsNullWhenKeyIsNotFound show to-one-relation 
returns null if referenced object id doesn't exist (shouldn't this 
throw exception?) testToOneRelationReturnsNullWhenKeyIsNull shows 
actually Rdo fetches the first entry of the relation type when a to 
one relation's key field is '' or null (should return null or throw 
exception)
see ticket #10880 [10880]

  11 files changed, 263 insertions(+), 0 deletions(-)
http://git.horde.org/horde-git/-/commit/dd36e1abd2961826262ff3c3ee551347db057d5c
2012-01-03 09:04:45 Jan Schneider Comment #3
State ⇒ Feedback
Reply to this comment
I'm not sure whether returning an empty result is the correct behavior 
by design, but returning arbitrary records on a null reference key 
definitly is not.
Throwing an exception could be considered a bug fix instead of a BC 
break, but returning an empty result sounds feasible too. If we are 
going to throw an exception, than rather in the case where the linked 
record is missing, than when the link is NULL.
It would speed up implementing a solution if you could create unit 
tests for those two cases.
2011-12-22 18:14:45 Ralf Lang (B1 Systems GmbH) Comment #2 Reply to this comment
Rdo currently doesn't understand the concept of a missing relation.
I'd suggest to either return no object or an exception.
Consistence with to-many-relations is hard.
These relations always return a list object, possibly an empty one.
As to-one-relations are often used in chains (at least in my code) 
I'd prefer a clean exception over an empty result but the current 
situation is worst for me.
When there is a value in the ID field (0, 33, "somekey" as opposed to 
'' or NULL), but the referenced object is no longer there, I get an 
empty result rather than a wrong object or an exception. Trying to 
blindly execute code on this not-an-object results in a crash.

Changing this too would be clearly BC breaking.
2011-12-22 18:05:11 Ralf Lang (B1 Systems GmbH) Comment #1
Type ⇒ Enhancement
State ⇒ New
Priority ⇒ 1. Low
Summary ⇒ Rdo: Make *- to-one-relations aware of "no relation" state
Queue ⇒ Horde Framework Packages
Milestone ⇒
Patch ⇒ No
Reply to this comment
I feel the current behaviour of Horde_Rdo resolving to-one-relations 
for the corner case of NULL or "empty string" in the relation key 
field is misleading.

Example:

Two data entities exist, a network host table and a certificate table.
A host may or may not have a certificate.

Since certificates are managed for other objects too it is not
feasible to reverse the relation and say the certificate must 
reference the host.

CREATE TABLE `appname_hosts` (
   `host_id` int(10) NOT NULL auto_increment,
   `certificate_id` int(10) default NULL,
#[... more fields that don't matter here]
   PRIMARY KEY  (`host_id`)
) ENGINE=MyISAM AUTO_INCREMENT=125 DEFAULT CHARSET=latin1

CREATE TABLE `appname_certificates` (
   `certificate_id` int(10) unsigned NOT NULL auto_increment,
   `certificate_crt` text,
   `certificate_key` text,
   `ca_id` int(10) default NULL,
   `certificate_cn` varchar(100) default NULL,
   `certificate_csr` text,
   `certificate_pkcs12` text,
   PRIMARY KEY  (`certificate_id`)
) ENGINE=MyISAM AUTO_INCREMENT=151 DEFAULT CHARSET=latin;

Now we have a Appname_HostMapper Class with relation

     protected $_lazyRelationships = array(
         'certificate' => array('type' => Horde_Rdo::ONE_TO_ONE,
                           'foreignKey' => 'certificate_id',
                           'mapper' => 'Appname_CertificateMapper')
     );

Now when I have a Host table entry with value of '' or NULL, I do get 
an instance of Appname_Certificate as a response, usually the first 
entry in the table (with default order).

Rdo currently doesn't understand the concept of a missing relation.
I'd suggest to either return no object or an exception.
Consistence with to-many-relations is hard.
These relations always return a list object, possibly an empty one.
As to-one-relations are often used in chains (at least in my code) I'd 
prefer a clean exception over an empty result but the current 
situation is worst for me.

My current workaround is to check length($this->certificate_id) before 
accessing $this->certificate but it's obviously not a clean solution.

I've classified this as an enhancement rather than a bug because it 
breaks current behaviour and Rdo possibly wasn't designed with this 
case in mind.

Saved Queries