Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve encoding practices during saving #182

Closed
romaninsh opened this issue Oct 5, 2016 · 3 comments · Fixed by #184
Closed

Improve encoding practices during saving #182

romaninsh opened this issue Oct 5, 2016 · 3 comments · Fixed by #184
Assignees
Labels

Comments

@romaninsh
Copy link
Member

romaninsh commented Oct 5, 2016

Currently when you save a field, it executes typecastSaveField that must convert PHP value into database-friendly value. For example for MongoDB it would convert date from DateTime into MongoDate, for SQL it would convert it into "STRING".

In some cases we want to encode / serialize data. For exmaple we want to store "array" into a field or even some arbitrary data (could be array, could be string could be boolean). Also what about storing StdObject? Another case is storing binary data in base64? and what about encrypting data?

If you do not define type, then by definition AgileData will not perform any conversion, but that's not what we want.

I think there should be another mechanism to encode data.

$this->addField('data', ['serialize'=>true]);

the above would cause data to be serialize()d before it actually hits the database. Upon load the data will be un-serialized. Also we could define several serialization mechanisms:

$this->addField('data', ['serialize'=>'base64']);

or

$this->addField('data', ['serialize'=>'json']);

There are however few questions that stop me from implementing this:

Combining type and serialization

What if I want to use DateTime type and serialize it into SQL?

$this->addField('ts', ['type'=>'datetime', 'serialize'=>true]);

Unfortunately when used with SQL this would convert datetime into string first, then serialize string and then store. Not really what user migth have wanted. I think DateTime object should be serialized without typecasting.

On other hand, what abut this:

$this->addField('ts', ['type'=>'datetime', 'serialize'=>'base64']);

base64 can only store strings so attempting to serialize date will cause a problem. Would serialize be different depending on if we store inside Mondo or SQL ? It shouldn't be, right? Creating multiple 'serialize' / 'encode' etc parameters seems like an overkill already.

json_decode - should it use true/false?

json_encode / decode cannot distinguish between "StdClass" objects and arrays. So you would have to specify which one you want. Do we create 2 different serialization types because of that?

conflicting with types

Currently we have type "struct" that stores array as json. It does not really allow to store non-array values. I think it should be renamed into 'array' anyway, but tehn would 'struct' shore StdClass objects? what about storing scalar values?

@DarkSide666
Copy link
Member

DarkSide666 commented Oct 5, 2016

Field types:

  • array - will json_decode($value, true) or []
  • object|stdobject - json_decode($value, false) or stdClass()
  • struct - remove this type completely because it's confusing and now array will replace it

Serialization types:

  • true - use serialize and unserialize
  • serialize - alias for true
  • base64 - use base64_encode($v) and base64_decode($v)
  • json_array - use json_encode($v) and json_decode($value, true)
  • json_object - use json_encode($v) and json_decode($v, false)
  • json - alias for json_array
  • [serialize_callback, unserialize_callback] - for extra nerds who want to develop their own encoding/decoding mechanisms

Execution order:

  • When saving - if serialization is active, then simply serialize (do not typecast at all), otherwise typecast
  • When loading - if serialization is active, then simply unserialize (do not typecast at all), otherwise typecast

Developer should be mindful and take care of type / serialize clashes himself. If he use type=datetime, then it's quite logical, that serialize=base64 can not be used in any way.

On worst case developer can always use:

  • loadCallback and saveCallback properties of Model or
  • serialize=>[serialize_callback, unserialize_callback] property of Field.

@romaninsh
Copy link
Member Author

romaninsh commented Oct 5, 2016

everything sounds great! let's call that thing object (not stdobject/stdclass).

Few notes:

  • Add field->typecasting. Typecasting will be performed if($typecasting===true || ($typecasting===null && !$serialize).
  • for base64 you would use ['type'=>'date', 'typecast'=>true, 'serialize'=>'base64']
  • rename loadCallback/saveCallback into typecast_load / typecast_save
  • use field->serialize=[$encode_callback, $decode_callback]

Future:

  • and in the future we might also add model type, which would store model class + id or conditions in a serialized way.

@romaninsh
Copy link
Member Author

Another note. If you use combination of type=array/object and serialize= then I think we shouldn't double-serialize if we can help it.

I think we only need "json" serialize type.

  • type array, serialize = false, SQL => json_encode($array)/json_decode($array, true);
  • type array, serialize = false, MongoDB => $arary / $array
  • type array, serialize = json, SQL => json_encode($array)/json_decode($array, true);
  • type array, serialize = json, MongoDB => json_encode($array)/json_decode($array, true);
  • type object, serialize = json, Array => json_encode($object)/json_decode($object)

etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants