Bonfire icon indicating copy to clipboard operation
Bonfire copied to clipboard

Data Source: Decoupling BF_Model from $this->db...

Open mwhitneysdsu opened this issue 8 years ago • 2 comments

This is somewhat related to an issue I just closed: #953

Basically, I'm looking at not necessarily stripping down the model, but decoupling it from $this->db by creating a DataSourceInterface (and an implementation for $this->db, which would be used by default).

Now, because BF_Model is loaded by the loader as an extension of CI_Model, we can't really do a classic constructor injection of the data source. However, we could at least make the dependency an interface and provide a way for the model to specify a different data source (given that the data source implements the interface).

Ideally, if you don't do anything related to the data source, BF_Model would load up the default data source and you won't notice any difference, even if you use $this->db directly in your model.

The main reason I'm bringing this up at all before working on an implementation is this: I want to limit the number of required methods on the interface, which would hopefully make it easier for others to create data sources of their own. In order to do this, I started looking at the db methods called by BF_Model and the number of choices I had for reducing those. The simplest solution, in my opinion, is simply to use BF_Model's own methods in place of the db methods, wherever possible, and remove some of the uses of what I think of as helper functions in query builder, like get_where(). As an example, BF_Model->find() currently does something like this:

$query = $this->db->get_where($this->table_name, array("{$this->table_name}.{$this->key}" => $id));

This would be replaced with something like this:

$this->where("{$this->table_name}.{$this->key}", $id);
$query = $this->db->get($this->table_name);

Now, the biggest potential issue I see here is in models which override the where() method (or any other method I might come across in this process). We've run into some issues with things like this in the past, and I still have to review quite a bit of Bonfire's code to see if this will be a problem.

I guess I'm just looking for some feedback on whether people think this will be a big problem for them or just another potentially-useful feature. Realistically, other than CI_Model itself (which is smaller than most people realize), $this->db is the largest dependency in BF_Model, and it seems feasible that we could isolate this dependency and open up some possibilities for using different data sources in models without potentially losing access to so many of the facilities the model provides through $this->db.

mwhitneysdsu avatar Sep 02 '15 20:09 mwhitneysdsu

I think it's worth pursuing, personally. But I'd love to see it developed as a standalone, database abstraction layer that would allow layers for other frameworks to be integrated. The one big missing piece to create an application that isn't very reliant on the framework is the database layer. This could be that piece.

lonnieezell avatar Sep 03 '15 03:09 lonnieezell

My personal use case is really for non-database data sources, but I'm sure it could work in that sense as well. I'll definitely keep that in mind as I'm designing it, maybe take a look at a few other frameworks' database layers to see what would be required to interface with them, as well as what would be needed to make it possible to create a lower-level interface with PHP's database extensions and abstraction layers if someone decided to go that route.

I made some notes of what I believe is every external reference in BF_Model, and it basically comes down to:

  • $this->load: called under two conditions to load DB in the constructor, once in validate() to load form_validation
  • $this->config: called once in set_date() to get the 'time_reference' setting
  • $this->form_validation: called once or twice (depending on whether the validation rules are an array or a string) in validate()
  • lang() is used primarily to retrieve error messages
  • All logging goes through a single method in the model, so log_message() only occurs once and Console::log() occurs once conditionally.
  • DB/Query Builder/DB_Result: we have 23 methods to wrap query builder methods (yeah, I'm pretty sure that was my big idea... but I think I can live with it), 2 methods (get_field_info() and prep_data()) which expect to be able to retrieve field metadata in a specific format, one method to retrieve database error messages, and 17 additional methods which interface with query builder and DB_Result objects, usually to a greater extent than the wrapper methods (find(), insert(), etc.).
  • Edit: Forgot $this->auth->user_id(), used 5 times to set created_by/modified_by/deleted_by fields.

mwhitneysdsu avatar Sep 03 '15 13:09 mwhitneysdsu