From 1e19969f0c8e1c8886276af42a8370fbd6afc581 Mon Sep 17 00:00:00 2001 From: Alastair Douglas Date: Fri, 19 Oct 2018 16:45:53 +0100 Subject: [PATCH 1/5] Fix most egregious problem with generate script --- scripts/class_generate.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/class_generate.pl b/scripts/class_generate.pl index a480a8c..5081b40 100644 --- a/scripts/class_generate.pl +++ b/scripts/class_generate.pl @@ -29,9 +29,9 @@ } my $connection_details = { dbname => $db }; -$connection_details->username => $user if $user; -$connection_details->password => $pass if $pass; -$connection_details->host => $host if $host; +$connection_details->{username} = $user if $user; +$connection_details->{password} = $pass if $pass; +$connection_details->{host} = $host if $host; my $client = OpenERP::XMLRPC::Client->new( $connection_details ); my $class_info = $client->model_fields($model); From 6b21c45823a640fa10e22c3792be9f91a0a735b4 Mon Sep 17 00:00:00 2001 From: Alastair Douglas Date: Fri, 19 Oct 2018 17:28:19 +0100 Subject: [PATCH 2/5] Setting context currently breaks requests We really need to figure out why having a context causes the request to break in the first place --- lib/OpenERP/OOM/Class/Base.pm | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/OpenERP/OOM/Class/Base.pm b/lib/OpenERP/OOM/Class/Base.pm index 59ac45e..1713e21 100644 --- a/lib/OpenERP/OOM/Class/Base.pm +++ b/lib/OpenERP/OOM/Class/Base.pm @@ -356,13 +356,6 @@ sub _get_context my $self = shift; my $context = shift; - my %translation = ( lang => $self->schema->lang ); - if($context) - { - # merge the context with our language for translation. - @translation{keys %$context} = values %$context; - } - $context = \%translation; return $context; } From c8d73cfe068dc3a992cfad634a9bc2d861c6257b Mon Sep 17 00:00:00 2001 From: Alastair Douglas Date: Tue, 21 May 2019 15:49:51 +0100 Subject: [PATCH 3/5] Create and consume roles to tighten screws The whole module was creating and using objects in manners that did not match the classes. It meant that creating an alternative link provider was unreliable, because there was no documentation on any of it, nor any code-based constraints. This creates roles for the classes to consume, and that we can consume elsewhere to improve compatibility. --- lib/OpenERP/OOM/Class/Base.pm | 2 - lib/OpenERP/OOM/Link/DBIC.pm | 82 +++++++++++----------------- lib/OpenERP/OOM/Object.pm | 14 +++-- lib/OpenERP/OOM/Object/Base.pm | 26 ++------- lib/OpenERP/OOM/Roles/DefaultLink.pm | 36 ++++++++++++ lib/OpenERP/OOM/Roles/Link.pm | 69 +++++++++++++++++++++++ 6 files changed, 151 insertions(+), 78 deletions(-) create mode 100644 lib/OpenERP/OOM/Roles/DefaultLink.pm create mode 100644 lib/OpenERP/OOM/Roles/Link.pm diff --git a/lib/OpenERP/OOM/Class/Base.pm b/lib/OpenERP/OOM/Class/Base.pm index 1713e21..128c2ad 100644 --- a/lib/OpenERP/OOM/Class/Base.pm +++ b/lib/OpenERP/OOM/Class/Base.pm @@ -56,8 +56,6 @@ sub _build_object_class { return $self->object->new; } -#------------------------------------------------------------------------------- - =head2 search Searches OpenERP and returns a list of objects matching a given query. diff --git a/lib/OpenERP/OOM/Link/DBIC.pm b/lib/OpenERP/OOM/Link/DBIC.pm index 68f6240..53f3173 100644 --- a/lib/OpenERP/OOM/Link/DBIC.pm +++ b/lib/OpenERP/OOM/Link/DBIC.pm @@ -1,15 +1,24 @@ package OpenERP::OOM::Link::DBIC; -=head1 NAME - -OpenERP::OOM::Link::DBIC +# ABSTRACT: Provides a basic link into a DBIC schema =head1 DESCRIPTION -Class used to link OpenERP data with data in DBIC. +If you do not provide your own C when you create your +L, this class will be used by default whenever you create +a link whose C is C. + +It provides a very simple interface into a DBIC schema by assuming every class +in your schema has a single-column primary key. This PK value is stored against +the configured column in the OpenERP object, typically C. + +This is where the C property ends up, from +L. =head1 PROPERTIES +See also L for inherited properties. + =head2 dbic_schema This is the DBIC Schema object. If you need a generic DBIC schema object @@ -17,40 +26,18 @@ this is normally the simplest way to access it. =head1 METHODS -These methods are not normally called directly. - -=head2 create - -Returns the new ID of a row it creates in a table using DBIC. - - my $id = $link->create({ class => 'RSName' }, $object_data); - -=head2 retrieve - -This is equivalent to doing a find on a ResultSet. - - my $object = $link->retrieve({ class => 'RSName' }, $id); - -=head2 search - -This is equivalent to doing a search on a ResultSet and then returning a list -of all the id fields. - - my @ids = $link->search({ class => 'RSName' }, $search, $options); +See L for methods. -=head1 COPYRIGHT & LICENSE - -Copyright (C) 2011 OpusVL - -This library is free software; you can redistribute it and/or modify it under the same terms as Perl itself. +All return values are DBIC row objects (or arrayrefs thereof). =cut use 5.010; use Moose; use Try::Tiny; -extends 'OpenERP::OOM::Link'; -with 'OpenERP::OOM::DynamicUtils'; +with 'OpenERP::OOM::Roles::Link', + 'OpenERP::OOM::Roles::DefaultLink', + 'OpenERP::OOM::DynamicUtils'; has 'dbic_schema' => ( is => 'ro', @@ -60,18 +47,15 @@ has 'dbic_schema' => ( sub _build_dbic_schema { my $self = shift; - + $self->ensure_class_loaded($self->config->{schema_class}); - + return $self->config->{schema_class}->connect(@{$self->config->{connect_info}}); } - -#------------------------------------------------------------------------------- - sub create { my ($self, $args, $data) = @_; - + try { my $object = $self->dbic_schema->resultset($args->{class})->create($data); ### Created linked object with ID $object->id @@ -81,28 +65,26 @@ sub create { }; } - -#------------------------------------------------------------------------------- - sub retrieve { my ($self, $args, $id) = @_; - + if (my $object = $self->dbic_schema->resultset($args->{class})->find($id)) { return $object; } + return; } +sub retrieve_list { + my ($self, $args, $ids) = @_; -#------------------------------------------------------------------------------- + # Note we do not support a compound PK. That behaviour would require a + # custom class that serialises the PK into OpenERP and deserialises it for + # search. + my $RS = $self->dbic_schema->resultset($args->{class}); + my ($pk) = $rs->result_source->primary_columns; -sub search { - my ($self, $args, $search, $options) = @_; - - # FIXME - Foreign primary key column is hard-coded to "id" - return map {$_->id} $self->dbic_schema->resultset($args->{class})->search($search, $options)->all; + my $rs = $RS->search({ $pk => { -in => $ids } }); + return [$rs->all]; } - -#------------------------------------------------------------------------------- - 1; diff --git a/lib/OpenERP/OOM/Object.pm b/lib/OpenERP/OOM/Object.pm index bbba300..387df8b 100644 --- a/lib/OpenERP/OOM/Object.pm +++ b/lib/OpenERP/OOM/Object.pm @@ -57,13 +57,19 @@ are specified in OpenERP in those terms. =head2 has_link -Used to indicate links with other systems. Typically this is to another table -in DBIC at the moment. +Used to indicate links with other systems. The key field is in OpenERP and is used for the ids of the objects to link to. -The class is used to ask the link provider to provide a link by this name. It -is not necessarily treated as a class, but the default link provider does do that. +The C is used merely to select a link by the name you have defined. When +you construct your schema, you pass it a link provider; the link provider +answers the request for a link of this class. + +The C is an arbitrary property, typically a hashref. This is passed to all +calls to the link object when the link provider returns it. A common use of this +is to provide the C key, referencing the ResultSet to link to on the +other side of a DBIC link. The actual value here is entirely dictated by the +C of link you are using, and what your link provider uses to access it. Possible options for type are C and C. diff --git a/lib/OpenERP/OOM/Object/Base.pm b/lib/OpenERP/OOM/Object/Base.pm index e262049..7e5bc00 100644 --- a/lib/OpenERP/OOM/Object/Base.pm +++ b/lib/OpenERP/OOM/Object/Base.pm @@ -21,7 +21,7 @@ OpenERP::OOM::Class::Base my $obj = $schema->class('Name')->create(\%args); - :say $obj->id; + say $obj->id; $obj->name('New name'); $obj->update; @@ -62,6 +62,9 @@ sub BUILD { $self->meta->add_method( $name, sub { + # XXX !!! + # This cannot rely on 'retrieve' because links are not + # required to provide 'retrieve' my $obj = shift; $obj->{"_$name"} //= $obj->class->schema->link($link->{class})->retrieve($link->{args}, $obj->{$link->{key}}); @@ -98,9 +101,6 @@ sub BUILD { } } - -#------------------------------------------------------------------------------- - =head1 METHODS =head2 update @@ -167,8 +167,6 @@ sub update { return $self; } -#------------------------------------------------------------------------------- - =head2 update_single Updates OpenERP with a single property of an object. @@ -206,8 +204,6 @@ sub update_single { return $self; } -#------------------------------------------------------------------------------- - =head2 refresh Reloads an object's properties from OpenERP. @@ -230,9 +226,6 @@ sub refresh { return $self; } - -#------------------------------------------------------------------------------- - =head2 delete Deletes an object from OpenERP. @@ -275,8 +268,6 @@ sub copy return $clone; } -#------------------------------------------------------------------------------- - =head2 print This is a debug method. @@ -289,9 +280,6 @@ sub print { say "Print called"; } - -#------------------------------------------------------------------------------- - =head2 real_create_related This actually does the create related via OpenERP. @@ -542,9 +530,6 @@ sub search_related { croak 'Unable to search_related'; # beat up the lame programmer who did this. } - -#------------------------------------------------------------------------------- - =head2 add_related Adds a related or linked object to a one2many, many2many, or multiple relationship. @@ -579,9 +564,6 @@ sub add_related { } } - -#------------------------------------------------------------------------------- - =head2 set_related Like the DBIx::Class set_related. Sets up a link to a related object. diff --git a/lib/OpenERP/OOM/Roles/DefaultLink.pm b/lib/OpenERP/OOM/Roles/DefaultLink.pm new file mode 100644 index 0000000..74a1a49 --- /dev/null +++ b/lib/OpenERP/OOM/Roles/DefaultLink.pm @@ -0,0 +1,36 @@ +package OpenERP::OOM::Roles::DefaultLink; + +# ABSTRACT: Provides properties required by the default link provider +our $VERSION = '0'; + +=head1 DESCRIPTION + +This role should be used by anything in the C namespace, +except C, which is a special snowflake. + +This is the namespace used to discover link types for the default provider. + +=head1 PROPERTIES + +=head2 config + +The default link provider will hold config for each link type. This will be +passed in as appropriate when the link is constructed. + +Its contents are entirely down to the specific link class being constructed, and +will be ultimately sourced from user-provided configuration. An example of this +is to hold to connection info for a DBIC schema - this obviously must come from +the end user somehow. + +That ends up here, and the link class should document it and make use of it. + +=cut + +use Moose::Role; + +has 'config' => ( + isa => 'HashRef', + is => 'ro', +); + +1; diff --git a/lib/OpenERP/OOM/Roles/Link.pm b/lib/OpenERP/OOM/Roles/Link.pm new file mode 100644 index 0000000..c5fa568 --- /dev/null +++ b/lib/OpenERP/OOM/Roles/Link.pm @@ -0,0 +1,69 @@ +package OpenERP::OOM::Roles::Link; + +# ABSTRACT: Defines required interface for link objects +our $VERSION = '0'; + +use Moose::Role; + +=head1 DESCRIPTION + +See L for how links work. The link provider that +your schema uses must return objects that consume this role when asked to +provide a link. + +Note there is no "search". This is because we search using OpenERP's weird +nested-list syntax and we would only be able to pass this exact same data to the +user's search method to search across links. Since this is likely a lot of work +to implement, we don't require you to do so. + +=head1 PROPERTIES + +=head2 schema + +This holds the L object that the link is linking to. + +This is not done automatically; your link provider will have to set this. + +=head1 REQUIRED METHODS + +The first parameter to every method will be C<$args>. + +C<$args> is arbitrary data that is provided by L, +and can be used to specify information such as which type of link-side object we +are dealing with. For example, when using the C link type, the C<$args> +parameter will be a hashref containing C, and that will contain the +resultset to use for the operation. + +=head2 create + +B: C<$args>, C<$object_data> + +Use C<$object_data> to create an object. See above for C<$args>. Feel free to +die if you can't do it. + +=head2 retrieve + +=head2 retrieve_list + +B: C<$args>, C<$object_data> + +Use C<$object_data> to locate an object. See above for C<$args>. + +In the case of C, C<$object_data> will be an arrayref, but you +can treat the items the same as the single parameter to C. + +The method may return arbitrary data, or nothing at all. The method may die if +the item doesn't exist, if that's what you want. In the list case, return an +arrayref in all situations, even if it's empty. + +The consumer will define the types that will be returned. + +=cut + +has 'schema' => ( + is => 'ro', +); + +requires qw/create retrieve retrieve_list/; + +1; From 9e370e7415a699416047eab2bec45a8bccdd6782 Mon Sep 17 00:00:00 2001 From: Alastair Douglas Date: Wed, 22 May 2019 10:44:08 +0100 Subject: [PATCH 4/5] Always ignore context --- lib/OpenERP/OOM/Class/Base.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OpenERP/OOM/Class/Base.pm b/lib/OpenERP/OOM/Class/Base.pm index 128c2ad..0a37ca4 100644 --- a/lib/OpenERP/OOM/Class/Base.pm +++ b/lib/OpenERP/OOM/Class/Base.pm @@ -354,7 +354,7 @@ sub _get_context my $self = shift; my $context = shift; - return $context; + return; } sub _inflate_object From adf3843503c75381c107a856c5fad2efdfcd7172 Mon Sep 17 00:00:00 2001 From: Alastair Douglas Date: Thu, 23 May 2019 10:36:55 +0100 Subject: [PATCH 5/5] Do not support holding a schema on generic links --- lib/OpenERP/OOM/Link/DBIC.pm | 3 +-- lib/OpenERP/OOM/Roles/DefaultLink.pm | 13 +++++++++++++ lib/OpenERP/OOM/Roles/Link.pm | 12 ------------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/OpenERP/OOM/Link/DBIC.pm b/lib/OpenERP/OOM/Link/DBIC.pm index 53f3173..9a95308 100644 --- a/lib/OpenERP/OOM/Link/DBIC.pm +++ b/lib/OpenERP/OOM/Link/DBIC.pm @@ -35,8 +35,7 @@ All return values are DBIC row objects (or arrayrefs thereof). use 5.010; use Moose; use Try::Tiny; -with 'OpenERP::OOM::Roles::Link', - 'OpenERP::OOM::Roles::DefaultLink', +with 'OpenERP::OOM::Roles::DefaultLink', 'OpenERP::OOM::DynamicUtils'; has 'dbic_schema' => ( diff --git a/lib/OpenERP/OOM/Roles/DefaultLink.pm b/lib/OpenERP/OOM/Roles/DefaultLink.pm index 74a1a49..9ade03e 100644 --- a/lib/OpenERP/OOM/Roles/DefaultLink.pm +++ b/lib/OpenERP/OOM/Roles/DefaultLink.pm @@ -12,6 +12,14 @@ This is the namespace used to discover link types for the default provider. =head1 PROPERTIES +=head2 schema + +This holds the L object that the link is linking to. + +This is set by the schema itself, but should probably not be relied on. The +generic link does not define a schema because there is no general path to +providing a schema to the link provider. + =head2 config The default link provider will hold config for each link type. This will be @@ -27,6 +35,11 @@ That ends up here, and the link class should document it and make use of it. =cut use Moose::Role; +with 'OpenERP::OOM::Roles::Link'; + +has 'schema' => ( + is => 'ro', +); has 'config' => ( isa => 'HashRef', diff --git a/lib/OpenERP/OOM/Roles/Link.pm b/lib/OpenERP/OOM/Roles/Link.pm index c5fa568..841d4d8 100644 --- a/lib/OpenERP/OOM/Roles/Link.pm +++ b/lib/OpenERP/OOM/Roles/Link.pm @@ -16,14 +16,6 @@ nested-list syntax and we would only be able to pass this exact same data to the user's search method to search across links. Since this is likely a lot of work to implement, we don't require you to do so. -=head1 PROPERTIES - -=head2 schema - -This holds the L object that the link is linking to. - -This is not done automatically; your link provider will have to set this. - =head1 REQUIRED METHODS The first parameter to every method will be C<$args>. @@ -60,10 +52,6 @@ The consumer will define the types that will be returned. =cut -has 'schema' => ( - is => 'ro', -); - requires qw/create retrieve retrieve_list/; 1;