interchange / interchange6-schema

DBIC schema for Interchange 6
8 stars 7 forks source link

This experimental patch should make the Message's "uri" unique. #168

Closed melmothx closed 9 years ago

melmothx commented 9 years ago

This in theory should make the update_or_create possible when the uri is supplied.

Anyway, the overloading of "type" seems not to work and an exception is raised.

The update_or_create failure happens even without the patch to the Message class.

SysPete commented 9 years ago

The problem here is that type is not a column but a Moo attribute which allows quick creation of a record without having to mess with MessageType class. What actually happens on insert (in a very simplified way) is:

$message->create(
    {
        message_type => { name => $type },
        uri => ...
    }
);

You cannot use type in a find or any method which makes use of find such as update_or_create or find_or_create. If you want to use these methods then you'll need to do something like:

my $type = $schema->resultset('MessageType')->find({ name => 'static' });
$schema->throw_exception("cannot find message type static") unless $type;
my $message = $schema->resultset('Message')->update_or_create(
    {
        type => $type,
        uri => '/blabla',
        ...
    }
);

The other option we have is to scrap MessageType and add type as a column to Message. @racke / @sbatschelet: thoughts?

SysPete commented 9 years ago

Update of example code since I made a mistake above:

my $type = $schema->resultset('MessageType')->find({ name => 'static' });
$schema->throw_exception("cannot find message type static") unless $type;
my $message = $schema->resultset('Message')->update_or_create(
    {
        message_types_id => $type,
        uri => '/blabla',
        ...
    }
);
SysPete commented 9 years ago

On a positive note: I agree completely with adding unique constraint to uri in Message class. We just need some tests that work.

SysPete commented 9 years ago

Added constraint via commit 27e0fd08719 - tests no so important for this. Thanks @melmothx for the PR.