tattersoftware / codeigniter4-settings

Lightweight settings manager for CodeIgniter 4
MIT License
22 stars 5 forks source link

Feature: Support array datatypes. #21

Closed sfadschm closed 3 years ago

sfadschm commented 3 years ago

This PR additionally allows the use of the "array" and "json-array" casts for datatypes as defined in CI4's entity. In the previous state, while it was possible do use these datatype with Settings, the casts would fail in two cases:

  1. If an encoded string (datatype 'array' or 'json-array') is stored in the database and loaded into the entity, it will trigger the set-cast, thus encoding/serializing the already encoded string again.
  2. When trying to write an encoded string into the database via the entity, this would also trigger the additional encoding. (This is a rather rare case, as databse writing is usually done through the CLI.)

This PR skips the additional encoding if

For the simple 'json' datatype, I think additional checks would be needed to see if the string value is already valid JSON or not.

MGatner commented 3 years ago

I'm confused why the framework Entity isn't handling this properly. What are we doing wrong that we need to intervene with this cast scenario but not any other? Can you provide code or example scenarios so I can recreate what is happening?

sfadschm commented 3 years ago

I'm confused why the framework Entity isn't handling this properly

~I have absolutely no clue how this is working (or if it is at all). If we have stored a json object as an encoded string in the database and load this into an entity (let's say model->find() with $returnType = EntityClass) the data from the database will always be cast by the _set method.~ See #23.

MGatner commented 3 years ago

Okay thank you for the test cases, that was helpful. I tracked down how the framework is handling this (it is rather buried) and found that Result handles it with setAttributes():

        if (is_subclass_of($className, Entity::class)) {
            return empty($data = $this->fetchAssoc()) ? false : (new $className())->setAttributes($data);
        }

I think it is fair to assume that for any instances where we create the Setting directly from database results we should be using setAttributes().

sfadschm commented 3 years ago

Oh yeah, good find. I did not see that snippet when I looked for it. As far as I see, the only place where Settings pushes database values into an entity is the getTemplates() method of the model. I added a new helper method for Setting to account for this in #23, which at least solves the test cases (and my use case 😄 ).

MGatner commented 3 years ago

This was resolved by #23, correct?

sfadschm commented 3 years ago

Yes, thought I referenced it ...