CRM-13123 - Add serialization metadata to schema

[colemanw]

Overview

This PR adds metadata to the schema for use by Api4. It doesn't change any functionality in core but adds some helper functions.

Technical Details

• Tags each serialized field in the schema with information about how they are being serialized (I've identified 5 different ways a field might be serialized and defined constants for each).

• Adds some functions for serializing and unserializing a given field. These functions aren't called from anywhere in core (yet) but will be used by api4.

• Adds a unit test for the serialization functions.

• CRM-13123: Handle value-separated fields at the dao level

[seamuslee001]

I quite like this idea and i think it would be quite worthwhile thoughts @totten @eileenmcnaughton. @colemanw i think we should put in searlisation for civicrm_payment_processor.accepted_credit_cards its storing as a JSON array at the moment

[totten]

Another +1 on the general concept of adding this to the metadata. It does make sense that this would go into APIv4 -- that pasture is green enough this metadata can bring some rewards.

Looking at the list of serialization mechanisms, SEPARATOR_BOOKEND, PHP, and JSON feel like the most common in the current schema.

I raised an eyebrow at two of the serialization mechanisms:

• COMMA -- Don't recall seeing this in the schema before.
• SEPARATOR_TRIMMED -- I had always assumed these cases were bugs -- eg the SEPERATOR_BOOKEND format is weird but functional (i.e. \0 doesn't clash with content, and you can use SQL to search on %\0myvalue\0%). SEPARATOR_TRIMMED would be harder to search.

Aside: If you like oddballs, civicrm_case_type.definition is serialized as XML. XML often requires a custom serialize/deserialize routine. In this case CRM_Case_BAO_CaseType has convertXmlToDefinition() and convertDefinitionToXML().

I'd be tempted for the metadata to simply name the serialize/deserialize callback.

[JoeMurray]

I've always been concerned that storing serialized data when the DB doesn't provide good tools for understanding it is an antipattern that should be looked at sceptically. MySQL has made great strides with JSON support over the last many years. Adding metadata about these fields is generally a good idea, so longas it doesn't encourage inappropriate proliferation of new serialized fields.

[eileenmcnaughton]

How come we lost html definition off these fields?

[colemanw]

Cause it's not an html field in any sense that I can tell. The html tags were originally added by someone with not much familiarity with the db and they're still not used for much of anything so I think it's safe to remove this.

[eileenmcnaughton]

ok

[eileenmcnaughton]

I think this is a good change - I would just suggest that some comments be added to the functions / constants to make it clear what is the preferred format for new fields.

[colemanw]

@totten I think it would be fine to add another constant for SERIALIZE_CALLBACK and add callback function names to the metadata as well, but IMO that's a project for another day as there's no immediate need for it, at least not until writing the api4 case api. But this PR gives a clear path forward for doing so.

As for your two raised eyebrows (is that both eyes?) I agree the SEPARATOR_TRIMMED format sucks, but it's what we're stuck with for a handful of fields until we correct them. Doing so is outside the scope of this PR but I think identifying what's what is at least a step in the right direction.

Are there any fields in the db that use a comma? I thought there were some but now I can't find them so maybe we don't need that constant.

[totten]

@colemanw

• Re: CaseType.definition / SERIALIZE_CALLBACK / SEPARATOR_TRIMMED - Roger that. Don't need to block the main PR for these elements.
• Re: SEPARATOR_COMMA - I don't think I've seen any before in the DB, and there aren't any mentioned in the patch, so +1 for removing it. (Aside: I have seen comma-separated values in the API -- e.g. Extension.install keys=foo,bar. However, that's not mapped through to a DB field.)

[MegaphoneJon]

This should be documented. I opened a ticket for it: https://github.com/civicrm/civicrm-dev-docs/issues/463