Sqldelight: Allow use of other persistable Parcelable types (such as Serializable) in ContentValues

[kevinmost]

From looking at the implementation of ContentValues, it looks like this is just using Parcelable to move data around. We can already store primitives into ContentValues. However, is it possible (or a good idea at all) to store objects that are Serializable into it?

If possible, this seems like it might be a good solution to marshaling/unmarshaling more complex data types via ColumnAdapters.

[JakeWharton]

Serializable is best if you close your eyes tightly and pretend it never existed.

Do you have a use case that's motivating this? We rarely make changes based on speculative need, but when natural usage necessitates something at least three times.

[kevinmost]

The motivation was to try to store a more complex data-type into a column (in this case, I wanted to store android.location.Address). I figured serializing it might be the easiest quick solution, though potentially not the best.

I've considered alternatively storing just the most important fields from Address as separate String fields, but it might be nice to get them back as the Address class, and it looks like ColumnAdapter expects a 1:1 mapping from the cursor's column to the unmarshaled data type (makes sense).

So maybe this isn't the best route to go down and it's more appropriate to just decompose the address into multiple String columns in the SQL layer, and then just forego using the Address class entirely when I read out of the DB?

[tadfisher]

You could also have a separate table for important Address fields, and a 1:1 mapping via a foreign key on your source table.

But it seems that a more general feature request here is the ability to map multiple columns to one adapted type. That could be difficult to express in a CREATE TABLE statement, but it wouldn't be difficult to implement on the ColumnAdapter end.

[tadfisher]

The other-other option is to wrap Address in something that serializes easily to one field.

[JakeWharton]

You could also write a generic ColumnAdapter which used Serializable to convert types to and from byte[] for storage in a BLOB column. Internally we have a column adapter that does this for protocol buffers. I'll include it below for inspiration:

package com.squareup.cash.data.db;

import android.content.ContentValues;
import android.database.Cursor;
import com.squareup.sqldelight.ColumnAdapter;
import com.squareup.wire.Message;
import com.squareup.wire.ProtoAdapter;
import java.io.IOException;

public final class WireAdapter<T extends Message> implements ColumnAdapter<T> {
  private final ProtoAdapter<T> adapter;

  public WireAdapter(ProtoAdapter<T> adapter) {
    this.adapter = adapter;
  }

  @Override
  public T map(Cursor cursor, int columnIndex) {
    try {
      return adapter.decode(cursor.getBlob(columnIndex));
    } catch (IOException e) {
      throw new IllegalStateException(e);
    }
  }

  @Override
  public void marshal(ContentValues contentValues, String columnName, T value) {
    if (value == null) return;
    contentValues.put(columnName, adapter.encode(value));
  }
}

[kevinmost]

Thoughts? (And is this something worth including internally in SQLDelight, for example? Or might that be bloat that most users won't need at the moment?)

import android.content.ContentValues;
import android.database.Cursor;

import com.squareup.sqldelight.ColumnAdapter;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectInputStream;
import java.io.ObjectOutput;
import java.io.ObjectOutputStream;
import java.io.Serializable;

import timber.log.Timber;

public class SerializableColumnAdapter<T extends Serializable> implements ColumnAdapter<T> {

    @Override
    public T map(Cursor cursor, int columnIndex) {
        try {
            return deserialize(cursor.getBlob(columnIndex));
        } catch (IOException | ClassNotFoundException e) {
            Timber.e(e, "Exception while deserializing for ColumnAdapter");
            return null;
        }
    }

    @Override
    public void marshal(ContentValues values, String key, T value) {
        try {
            values.put(key, serialize(value));
        } catch (IOException e) {
            Timber.e(e, "Exception while serializing for ColumnAdapter");
        }
    }

    private byte[] serialize(T object) throws IOException {
        ByteArrayOutputStream bos = null;
        ObjectOutput out = null;
        try {
            bos = new ByteArrayOutputStream();
            out = new ObjectOutputStream(bos);
            out.writeObject(object);
            return bos.toByteArray();
        } finally {
            if (bos != null) {
                bos.close();
            }
            if (out != null) {
                out.close();
            }
        }
    }

    private T deserialize(byte[] bytes) throws IOException, ClassNotFoundException {
        ByteArrayInputStream bis = null;
        ObjectInput in = null;
        try {
            bis = new ByteArrayInputStream(bytes);
            in = new ObjectInputStream(bis);
            //noinspection unchecked
            return ((T) in.readObject());
        } finally {
            if (bis != null) {
                bis.close();
            }
            if (in != null) {
                in.close();
            }
        }
    }
}

[JakeWharton]

android.location.Address is Parcelable, not Serializable. You're much better off with Parcelable as it's faster and safer for both your column adapter and the implementing class.

Otherwise your code is fine, I would only have minor nits if I were code reviewing it.

[kevinmost]

Ah, my mistake! You're right. On that note, regarding Parcelable, I always hear that Parcelable classes shouldn't be persisted because they're optimized in ways that aren't resilient against changes. But perhaps that's fine in this instance, assuming that we just drop those fields in the database when we migrate? And it seems like a simple adapter, too:

import android.content.ContentValues;
import android.database.Cursor;
import android.os.Parcel;
import android.os.Parcelable;

import com.squareup.sqldelight.ColumnAdapter;

public class ParcelableColumnAdapter<T extends Parcelable> implements ColumnAdapter<T> {

    private final Parcelable.Creator<T> creator;

    public ParcelableColumnAdapter(Parcelable.Creator<T> creator) {
        this.creator = creator;
    }

    @Override
    public T map(Cursor cursor, int columnIndex) {
        return deserialize(cursor.getBlob(columnIndex));
    }

    @Override
    public void marshal(ContentValues values, String key, T value) {
        values.put(key, serialize(value));
    }

    private byte[] serialize(T parcelable) {
        final Parcel parcel = Parcel.obtain();
        parcelable.writeToParcel(parcel, 0);
        final byte[] bytes = parcel.marshall();
        parcel.recycle();
        return bytes;
    }

    private T deserialize(byte[] bytes) {
        final Parcel parcel = Parcel.obtain();
        parcel.unmarshall(bytes, 0, bytes.length);
        parcel.setDataPosition(0);
        final T deserialized = creator.createFromParcel(parcel);
        parcel.recycle();
        return deserialized;
    }
}

[JakeWharton]

I always hear that Parcelable classes shouldn't be persisted because they're optimized in ways that aren't resilient against changes.

Nope, this is totally true. You shouldn't in the DB since Android might upgrade beneath you and change the underlying data format.

I think you're stuck just either implementing a custom ColumnAdapter or modeling the address as its own table with its properties as columns.

[kevinmost]

Right, that sounds like the only way then. ColumnAdapter only provides a way to do a 1:1 mapping between Cursor columns and the adapted type, though, which makes sense because the CLASS('classname') syntax goes on each column in the CREATE TABLE statement.

I guess you could potentially provide multiple-column-to-one-type functionality via some sort of special syntax (address CLASS('android.location.Address') COMPONENTS(STRING, DOUBLE, ...)) or something, but that sounds heavyweight and confusing, and even if it could be done nicely, it doesn't sound like it would be high-priority at such an early stage in the library's lifetime.

I'll probably stick with either making a new table for my addresses and linking to it via foreign keys, or I'll adapt the Address classes I get from Android into a more useful data model that implements Serializable and then use the SerializableColumnAdapter from earlier.

Thanks @JakeWharton and @tadfisher for weighing in on this.

[JakeWharton]

:+1: closing the issue then