[Dbix-class] CDBICompat NumExplodingSheep

[Dave Howorth]

Peter Rabbitson wrote:
> I contacted the CDBI-compat shim maintainer and here is his reply. I also included
> him in the CC, please reply-all if you discuss it with him to keep me in the loop.
> 
> ==============================================================
> 
> Peter Rabbitson wrote:
>> Haven't seen you on IRC for a while, figured I'll try here. We got a
>> bugreport[1] against the compat layer some time ago. I applied his
>> indeed failing tests[2], and TODOified[3] them. If you can drop by to see
>> if this can/needs to be fixed it'd be great.
> 
> In general, I think you're right to do the full hash copy.
> 
> But I ran the new tests through CDBI and insert() doesn't appear to honor
> accessor_name_for() for the hash keys.  So this sort of thing doesn't work.
> 
> eval {
> 	my $data = { %$data };
> 	$data->{sheep} = 2;
> 	ok my $bt = Film->insert($data), "Modified accessor - with accessor";
> 	isa_ok $bt, "Film";
>         is $bt->sheep, 2, 'sheep bursting violently';
> };
> is $@, '', "No errors";

OK, I've now had time to try this. All I can add to my previous reply is
to confirm that CDBI does indeed perform insert using a modified
accessor for me.

As I said before, I'd be very surprised if it didn't since this test
isn't one I wrote, it's in the existing CDBICompat tests. Indeed, AFAICT
it is copied directly from the Class::DBI test suite, which reads:

  eval {
        my $data = $data;
        $data->{sheep} = 1;
        ok my $bt = Film->insert($data), "Modified accessor - with
accessor";
        isa_ok $bt, "Film";
  };
  is $@, '', "No errors";

> Whether this is a bug or feature of CDBI is arguable, insert() and the
> accessors have always been a little out of sync, but as far as CDBI::Compat is
> concerned just go with what CDBI actually does.
> 
> So those new tests should probably be pulled.

The new tests I suggested adding are tests of find_or_create. I would
like to see them, or a variant of them included, for two reasons:


(1) There are no tests at present - either to demonstrate that the
feature works or it shouldn't. This is a shortcoming shared with the
Class::DBI tests. It may be that given the simplicity of the code it
wasn't felt necessary in CDBI:

  sub find_or_create {
	my $class    = shift;
	my $hash     = ref $_[0] eq "HASH" ? shift: {@_};
	my ($exists) = $class->search($hash);
	return defined($exists) ? $exists : $class->insert($hash);
  }

and of course there are accessor tests of both insert and search.


(2) I use the feature heavily in my existing CDBI applications, so I'm
pretty convinced it works in CDBI and so also, I'm pretty keen that it
work in CDBICompat. I also believe it does not work at present, hence
the need for the tests.

Regards,
Dave Howorth