Thread

I’ve been thinking about some of the errors I’m fighting.  One of the most obscure from above has 2 components.  The _to_array method, used to get the insert and update array, doesn’t check to see if a field is not set.  Because it doesn’t, there isn’t a way to insert a row into the database that has a default value.

For example, I have a NOT NULL field with a default, and when I create a new item, I don’t want to have to set that field.  (I know I can work around this by setting all the defaults in my constructor, but I don’t think I want to do this, yet.)

The second part of this is that the _clear method sets all the fields to NULL.  I think it makes more sense to unset the fields completely.  This truly clears out the data.  Any thoughts?

I think there is a “bug” in your insert/update checking, but it’s obscure: an ‘id’ of 0 (zero) will always be inserted, even if it is supposed to be updated.  Probably best to leave this one, but I thought I’d point it out for the documentation.

Last, on the issues of table names and pluralities:  This has caused me no end to frustration.  Apparently, somewhere in the code, one of the functions is still trying to ‘convert’ the model name, instead of using the provided $table and $model.  This is an issue, because the table is named statuses (yes, it’s correct), and the model is Status.  I keep getting errors where the joining table is labeled status, and the model gets renamed statu!

Well, good news!  First, half of my error was that my table was wrong in the relationship :red:!

Second, I think I found out where the other errors are coming from:

The status -> statu were coming from the constructor.  It was calling singular() on the class - but the class is singular by default!  I think you want to remove this.

I fixed the relationship_table issues (in two locations, near lines 457 and 469):

// old
//$relationship_table = ($this->table < $table) ? $this->table . '_' . $table : $table . '_' . $this->table;
// new
$m = ucfirst($model);
$nm = new $m;
$relationship_table = $this->_get_relationship_table($nm->prefix, $nm->table, $nm->model);

I also found that the $table was being re-pluralized in the _related function.  Again, I think this is a mistake.

// ~ line 1576
// $table = $prefix . plural($table);
$table = $prefix . $table;

Then I added the unset routines listed above.  I’m rethinking setting the defaults in code instead.  There might even be an argument for default values being stored in an array, and a default() method to re-initialize the class?  Interested on others opinions on this matter.

Also, I think there is a bit of copy-and-paste cruft in the _clear method.  The if block on line 2128 appears to simply be clearing the fields a second time.

I originally had the _to_array() method setup to ignore empty fields but then it was pointed out that there are situations where a developer will want to set a field to be an empty string, zero, or NULL, which is why I removed the check, which is the correct behaviour.

MySQL handles this correctly on its inserts/updates but it seems PostgreSQL doesn’t.  I don’t have the time to setup and configure a PostgreSQL system so unfortunately can’t provide much support for it.

Yeah, I don’t consider the insert/update check you mentioned to be a real bug. It’s true that an ID of 0 (zero) will always be inserted but then all Database systems I know of begin their AUTO INCREMENT at 1.  Still, I might add a check for the zero to be safe.

On your status/statuses issue, I’ve made a number of changes to the referencing of model and table names so hopefully the issue you’re having will not occur in the next version.  Just to clarify, have you specified the table in the model as “statuses”?

UPDATE

I’d hold off on reporting any more issues for now as a lot of the recent ones you’ve mentioned are already corrected in the next version.  Easier to wait and try again with it, since it shouldn’t be too long before I release it.

Oh, and not trying to be nit picky or anything :red: but rather than posting multiple times in a row, could you update your last post if it’s looking like you’re going to do multiple posts?  Makes things easier to follow/respond to.

Sure - I wasn’t sure what would be easier…  I’ll make sure to edit my posts from now on.  (I’ve been doing a lot of coding all week, as you might have guessed.  Brain’s a little rattled.)

I wouldn’t check for the 0 id - I just thought I’d point it out as a possible issue for some people.

I know that checking for empty() on the _to_array method won’t work - that’s why I was looking into using isset(), which only returns FALSE if the variable doesn’t exists.  It correctly handles all the empty types (NULL, 0, ‘’, ‘0’, etc.)  However, you then have to unset() the fields that are not in use.

All that being said, like I mentioned, I’m thinking about filling in default values in the constructor.  This will solve the insert problem, updates don’t matter because they get filled beforehand, etc.

Finally, I am not trying to harp on DataMapper - I’m really impressed with it.  I’m really trying to help, so if this barrage of info is too much, please tell me to stop!  I understand that you have other things you do.

- Phil DeJarnett

PS: and my status/statuses table was correct in the Model, incorrect in the relationship, and being affected by those singular/plurals I mentioned.

PPS: I think I’ve gotten the SortableModel worked out, however, it requires a little fudging on the whole subcollection thing.  Until I get something written up, and test it some more, I’ll wait to post the newer version.

UPDATE 1: PostGreSQL just hates inserts.  I mean, really.  I love the DB otherwise, but it will not accept NULL for the autoincrement column.  I still think the best way to handle this is to unset($data[‘id’]).  It shouldn’t break any other databases.

Also, I tweaked the code to handle default values, like this:

// in DataMapper(), before _clear()
$this->fields = $this->db->list_fields($this->table);
foreach($this->fields as $field) {
    $this->default_values[$field] = isset($this->{$field}) ? $this->{$field} : NULL;
}

/* ... */

// in _clear()
$this->{$field} = $this->default_values[$field];

This prevents the default values from being an empty string.  To set default values, simply initialize the fields in the class definition.

UPDATE 2: Regarding my get_by_id suggestion, if you include it, you should wrap the $id value in an intval().  This makes the function even more useful, because you can safely pass in anything, including user provided data, and try to get an object out of it.

function get_by_id($id) {
    $this->where('id', intval($id));
    return $this->get();
}

Hi stensi. Great work on this class! It’s well commented and worked right away (saving me lots of time).

I added a slight modification to the save function of your DataMapper class. It allows you to do some final processing on the values before they are inserted or updated. It also allows you to have computed fields.

I inserted the following code on line 357 of your latest code (Right after the ‘if ($this->valid)’ line) in save function.

// Check whether there are any insert or update hooks
foreach ($this->fields as $field)
{
    //If there is an update hook, call it.
    if (!empty($data['id']) && method_exists($this, $field . '_update'))
    {
        call_user_func(array($this, $field . '_update'), &$this->{$field});
    }
    
    //If there is a insert hook, call it.
    if (empty($data['id']) && method_exists($this, $field . '_insert'))
    {
        call_user_func(array($this, $field . '_insert'), &$this->{$field});
    }
}

Here is an example of this feature in use:

class User extends DataMapper
{
    
    public $table = 'users';
    public $created_field = 'created_on';
    
    function __construct()
    {
        parent::__construct();
    }
    
    protected function username_insert($username)
    {
        $username = trim($username);
        
        if (empty($this->display_name))
        {
            $this->display_name = $username;
        }
    }
    
    protected function password_insert($password)
    {
        $password = md5($password);
    }
    
}

What do you think?

@vendiddy: That’s a great idea, if you ask me!  I was thinking something along the same lines.  I would also like to have hooks for getting the fields.  Those could be $this->{$field . '_load'}, or something similar.

Those two functions alone would save me a lot of pre- and post- processing for the PostGreSQL problems.  I could then handle booleans and dates in a more intuitive manner.

Also, Stensi, I’m not sure if I made this clear earlier, but I’m still concerned about the technique used to retrieve newly inserted information from the database.  If you have a very simple table [ id, name ], and I insert two values with the same name (say they have relationships that would make them unique otherwise), the get() will most likely pick up the wrong id.

Instead, it probably makes more sense to wrap the insert() in a transaction, and use last_insert_id() to get the inserted id column.

Another option would be to break out the update() and insert() methods into their own method, so they could easily be overridden in a subclass:

// in save()
if (!empty($data['id']))
{
    return $this->_update($data);
}
else
{
    return $this->_insert($data);
}

/* rest of save() function ... */

function _update($data) {
    // Update existing record
    $this->db->where('id', $data['id']);
    $this->db->update($this->table, $data);

    // Reset validated
    $this->validated = FALSE;
}

function _insert($data) {
    // Create new record
    $this->db->insert($this->table, $data);

    // Repopulate this object to retrieve new ID (validated is reset during get)
    $this->get();
            
    return TRUE;
}

This would allow other DB types to modify the inserts and updates easily.

@vendiddy: That’s a pretty cool idea but the validation can do this for you already.

You can add a custom function to your class and apply it to the property.  Validation rules are run whenever you call save().  For example, here’s how I would reproduce your code with the same result:

class User extends DataMapper
{
    var $validation = array(
        array(
            'field' => 'username',
            'label' => 'Username',
            'rules' => array('trim', 'populate_field' => 'display_name')
        ),
        array(
            'field' => 'password',
            'label' => 'Password',
            'rules' => array('md5')
        )
    );

    var $table = 'users';
    var $created_field = 'created_on';
    
    function User()
    {
        parent::DataMapper();
    }
    
    function _populate_field($field, $param)
    {
        if (empty($this->{$param}))
        {
            $this->{$param} = $this->{$field};
        }
    }
}

So, when you save a user, the validation rules are applied automatically, so the password field becomes encrypted, and the display_name will be set with the username.

You’ll notice that the _populate_field validation rule is re-usable for any other fields you want to do this for, so it’s not just restricted to the username/display_name.

@OverZealous.com: I can see how separating the insert and updates out to allow them to be easily overridden would be handy.

As for the insert_id thing, I see what you mean.  I’m working on adding transactions in at the moment, but this will only benefit those with InnoDB or BDB Databases.  I’ll test to see whether insert_id works for MyISAM, in which case, rather than doing another get, I’ll just set the id to the insert_id(), removing the need of doing an extra query.

I keep getting close to finishing the next version but then people keep adding good suggestions, lol.  The conversion to a library is finished and I’ve reworked a lot of things, especially around the internal workings of relationship naming (using remains the same) so pretty much all the issues around that part are solved.  Also done a lot of tidying of the code, to make it meet EllisLab’s Development Guidelines.

Anyway, it’s coming!

UPDATE

Yep, looks like insert_id() works for them all, so I’ll remove the extra get() call after a save().

Cool, didn’t know validation was that simple! Guess I should have read the documentation more thoroughly…

One (small) advantage of my method is that it follows the principle of convention over configuration.

Version 1.4 has been released!

View the Change Log to see what’s changed.

In short, DataMapper has been converted into a Library (along with a config file for easy global config settings). Transaction handling methods have been added, along with the choice for automated transaction handling. The $has_one and $has_many relationship arrays are no longer associative.  Automated timestamps can now be local time or GMT/UTC and can be of the DateTime or Unix Timestamp format.

There’s also lots of other changes and improvements, such as cleaner code that better follows the EllisLab Development Guidelines and a better way of internally determining relationship table names.

UPGRADING

As this is now a library, be sure to remove DataMapper from the autoload models array in the application/config/autoload.php file.

$autoload['models'] = array(); // no longer has 'datamapper'

I’d also remove the previous version of DataMapper from your application/models folder.

After that, just follow the Installation Instructions.

Enjoy 😊

I was trying to set up the library version, and something is wrong with the config file.  It’s apparently only loading it on the base class (DataMapper).  What this means is that the prefix and join prefix are not being loaded in.

I think that CI is only loading in configs for matching classes.  Since my model is called User, it doesn’t find a ‘user.php’ config file.

Possible solution: load in the config file explicitly?

- Forgot to mention that this (obviously) also breaks timestamps

Not sure how this breaks timestamps (except if you just mean timestamp settings from the config aren’t being applied).

I’ll change it to specifically load the config file.  Have you come across any other issues yet?

AH-HA: I almost reported this, but I forgot you said to change the relationship arrays.  I must update that to continue…

Version 1.4.1 has been released!

View the Change Log to see what’s changed.

In short, I changed it so the autoload of the configuration file is explicit.

Thanks for bringing that to my attention Phil 😊

I’ve been thinking about a way to fix the PostGreSQL insert bugs that will also allow default values from the database to show through.  It isn’t too horrible, I promise!

The idea would be to capture a collection of “changed” fields from within the __set() method.  Then, when building the $data array, only include those items that were changed.

// at top
var $_changed_fields = array();

function __set($name, $value)
{
    $this->{$name} = $value;
    // Added these three lines
    if(in_array($name, $this->fields))
    {
        array_push($this->_changed_fields, $name);
    }
}

function _to_array($validate = FALSE)
{
    $data = array();

    foreach($this->fields as $field)
    {
        // Added this line, and updated the if()
        $changed = in_array($field, $this->_changed_fields);
        if ($changed && empty($this->{$field}) && $validate)
        {
            continue;
        }
            
        $data[$field] = $this->{$field};
    }
        
    return $data;
}

// Added new function, needs to be called in get() and _clear()
function _clear_changed_fields($all = FALSE)
{
    $this->_changed_fields = array();
}

/* Also the save() method will need to be changed to user $this->id
 * for the check and to pass into the update's where.
 * There is no reason to ever put the [id] into the query
 * since it is auto-generated, and shouldn't change for updates.
 */

Now, updates and inserts will be limited to those fields that get set between get() and save().  This would reduce some DB traffic, and also provide cleaner inserts in general.

So, does that make sense?  Would it break anything on the MySQL end?  It would help me eliminate my only remaining hacks - default values and removing the id.  Then PostGreSQL (using smallints instead of booleans) should work fine, at least, so far as I’ve noticed.

Thanks stensi, works brilliant as a library.

To make the autoload working for Modular Extensions, I’ve applied a small patch to this class. Since it wouldn’t hurt non-ME users, perhaps it can be in a next version.

public static function autoload($class)
    {
        /* don't try to autoload CI_ or MY_ prefixed classes */
        if (strstr($class, 'CI_') OR strstr($class, 'MY_')) return;
        
       // Prepare class
        $class = strtolower($class);

        // Prepare path
        $path = APPPATH . 'models';

        // Prepare file
        $file = $path . '/' . $class . EXT;

        // Check if file exists, require_once if it does
        if (file_exists($file))
        {
            require_once($file);
        }
        else
        {
            // Look in ME Model directories
            if (defined('MODBASE')) {
                $paths = glob(MODBASE . '*/models/' . strtolower($class) . EXT, GLOB_NOSORT);
        
                if (file_exists($paths[0]))
                {
                    require_once($paths[0]);
                    return;
                }
            }
            // Still not found: Do a recursive search of the path for the class
            DataMapper::recursive_require_once($class, $path);
        }
    }