Re: Proposed Architecture Changes in 2.0 (Request for comments)

[Chris Travers <..hidden..>]

On Mon, Mar 8, 2010 at 4:21 PM, Adam Thompson <..hidden..> wrote:

>> Regardless of where we automate such a transformation, I think it
>> would be helpful to automate it.  Is there a specific objection to
>> automating the movement from a flat representation to a native
>> representation?  Or do you think there is a better place to put such
>> automation?
>
> At the moment, I'm agreeing with Aurynn.  But, I also don't see the
> point of such an automated transformation.  What benefit would this provide?
>
> My gut reaction (without a use case, anyway) is that this sounds neat on
> paper but potentially opens up bad-data injection problems.  If someone
> really wants, say, a RESTful interface to GL transactions that supplies
> text/plain format CSV data, that should be easily added - as an addon.
> The converse should hold true for custom import functionality, no?

Ok.  Here is basically the way I was envisioning this:

We create an invoice object and serialize it to a template which gets
processed and submitted back possibly with new information on it.  The
controller runs its own checks on the object first making sure all
sanity checks it has are passed.  The controller then passes off the
flat representation to the invoice object from_flat() constructor
which takes this, puts it in the form of the object, and runs the
constructor, which runs validation checks on the data it is given.

The idea here is also applicable if trying to upload some sort of data
from some other flat representation.  The data would be checked by the
controller for it's own security and sanity checks but it wouldn't
have sufficient knowledge to enforce the internal structure of the
object.

Basically the question whether code like:

my $ref = {};
for my $key  (qw{reference description date_posted, approved}){
   $ref->{$key} = $flat->{$key};
}

for my $count (1 .. $flat->{rowcount}){
    my $line_item= {};
    for my $key (qw(accno, chart_id, description amount)){
         $line_item->{$key} = $flat->{"${key}_$count"}
    }
    push (@{$ref->{line_items}}, $line_item);
}

my $invoice = LedgerSMB::DBobject::GL::Invoice->new($ref);  #
Constructor would also run checks on the data handed it

It seems to me the constructor would need to check it's own data
structures anyway.  I am wondering what we gain by asking the workflow
script to do this conversion if it is in a long, serialized form with
variable numbers of rows.

I suppose the alternative is to do something like:
my $invoice = LedgerSMB::DBobject::GL::Invoice->new(
           {id => undef,
        reference => $request->{reference},
        description => $request->{description},
        date_posted => $request->{date_posted},
        approved => $request->{approved}
});

for my $count (1 .. $request->{rowcount}){
    $invoice->add_line({
          accno => $request->{"accno_$count"},
         description => $request->{"description_$count"}.
         amount => $request->{"amount_$count"},
         chart_id => $request->{"chart_id_$count"},
    });
}

Personally I find the first easier to read.  It also provides greater
opportunities for data checking in the object because the object is
created "at once" with all necessary data.  For example, if we want to
say "every journal entry must have at least one line item" we can do
that.  In the second example, we can't do it without waiting for the
GL entry to actually be used for something (posted, displayed, etc).
Then we have to check.  (We should check then too, but it would be
nice to have all validation logic centralized so that all the same
checks are run all the time.)

Best Wishes,
Chris Travers