#66 (comment)
I'm pretty sure the map/dictionary is enough and probably prefereable. If a blob field has the same content_type and digest as the one already stored in database but no actual data, it's a no-op for the blob. The blob should be preserved. If you also send data then it's replaced.
Fixing this issue has implications in the way documents are read and saved to/from the database, and would significantly impact the platform code, hence this new discussion.
One of the issues I see currently is that the platform code is doing a lot of work parsing data both when reading and saving documents. Below is a working proof of concept that delegates most of the serialization/deserialization to the library itself, therefore greatly simplifying the plugin's code.
A short breakdown of the important bits:
-
The code below is entirely self-contained and sufficient to read and save documents, and it produces the exact same output as the old implementations. The Dart code requires no changes (except if we want to also handle the revision property, which is strangely missing now).
-
The SDK has methods which serialize/deserialize maps and they support all the types that the method channel's codec supports - except the Blobs. Therefore, except for transforming the Blob instances into dictionaries, there's no need to do any kind of custom parsing in the platform code.
-
When reading a document. The SDK has a 'document.toMap()' method, which, if it weren't for the blobs could be just passed to the method channel without any changes. the "_documentToMapNew(Document doc)" method replaces all Blob instances with their dictionary counterpart. The Blob class even has a method for it in 'getProperties()'.
-
When saving a document the situation is very similar. The map that the method channel passes to the Java code is the same that the MutableDocument's constructors expect - minus the blobs. Here the situation is a bit more complex but not by much.
If you set a @blob field with byte data in it, the MutableDocument needs a Blob instance instead of a simple dictionary.
Ex: this is a document that is being saved with a new blob
{age=10, languages=[en, es], active=true, id=person1, avatar={digest=null, @type=blob, length=null, data=[B@e3e7d9b, content_type=image/png}, settings={list=[1, 2, 3, 4, 5], a=b, x=y, map={1=one, 2=two, 3=three}}, doctype=person, name=Person1, height=5.6, birthday=2000-02-02T00:00:00.000}
If there are @blob fields in the dictionary, but their data field is empty, then the MutableDocument can deserialize it by himself and the blobs are preserved as expected. In other words, if there is no data sent, there is no need to convert the metadata into a Blob instance. With the new code that is accessing the blob files directly, this allows an important optimization on the dart side to only send data when the data is actually changed (like when uploading a new avatar). In every other case only the metada is ever sent. More importantly, the part that is relevant to the PR66 is that there is no need to ever keep track or cache the blobs in the platform side. It is the resposability of the dart code to ensure that the blobs in a document are preserved, updated or deleted according to the business logic.
Ex: this is a document that was read in dart using "database.document()' and saved again but without reading or writing the 'avatar' field.
{age=10, active=true, languages=[en, es], id=person1, avatar={digest=sha1-K5R8dtFHQYSI4daLqfJwXQgZb8k=, @type=blob, length=120, content_type=image/png}, settings={a=b, list=[1, 2, 3, 4, 5], x=y, map={1=one, 2=two, 3=three}}, doctype=person, height=5.6, name=Person1, birthday=2000-02-02T00:00:00.000}
Hint: The blob was preserved.
- Currently this code assumes that the blobs are only at the first level of the document, and doesn't handle blobs in nested values. I'm not aware of any way to set blobs deeper in the document, but I'll try to investigate this further to confirm it.
Map<String, Object> getDocumentWithIdNew(Database database, String _id) {
HashMap<String, Object> resultMap = new HashMap<>();
Document document = database.getDocument(_id);
if (document != null) {
resultMap.put("doc", _documentToMapNew(document));
resultMap.put("id", document.getId());
resultMap.put("rev", document.getRevisionID());
resultMap.put("sequence", document.getSequence());
} else {
resultMap.put("doc", null);
resultMap.put("id", _id);
}
return resultMap;
}
Map<String, Object> saveDocumentNew(Database database, Object id, Map<String, Object> data, ConcurrencyControl concurrencyControl) throws CouchbaseLiteException {
// Blobs need special attention
// When sending data we replace the value in the dictionary with a Blob instance.
// When there is no data sent, we leave the blob's metadata dictionary as is
// and let Couchbase Core to deserialize it. This will ensure the blobs are preserved.
for (String key : data.keySet()) {
Object value = data.get(key);
if (value instanceof Map<?, ?>) {
Map<?,?> v = (Map<?,?>) value;
if (Objects.equals(v.get("@type"),"blob")) {
if (v.get("data") != null) {
String contentType = (String) v.get("content_type");
byte[] blobData = (byte[]) v.get("data");
data.put(key, new Blob(contentType, blobData));
}
}
}
}
MutableDocument mutableDoc;
if (id != null && data != null) {
mutableDoc = new MutableDocument(id.toString(), data);
}
else if (id == null && data == null) {
mutableDoc = new MutableDocument();
}
else if (data == null) {
mutableDoc = new MutableDocument(id.toString());
}
else {
mutableDoc = new MutableDocument(data);
}
boolean success = database.save(mutableDoc, concurrencyControl);
HashMap<String, Object> resultMap = new HashMap<>();
resultMap.put("success", success);
if (success) {
resultMap.put("id", mutableDoc.getId());
resultMap.put("sequence", mutableDoc.getSequence());
resultMap.put("rev", mutableDoc.getRevisionID());
resultMap.put("doc", _documentToMapNew(mutableDoc));
}
return resultMap;
}
private Map<String, Object> _documentToMapNew(Document doc) {
Map<String, Object> map = doc.toMap();
// Replace all Blob instances with their "json" metadata
for (String key : map.keySet()) {
if (map.get(key) instanceof Blob) {
Map<String,Object> json = ((Blob) map.get(key)).getProperties();
json.put("@type", "blob");
map.put(key,json);
}
}
return map;
}
I have an example app with test code, that I'm cleaning up and I'll try to share in the next few days. I just wanted to put this out there for feedback.
Similar concepts can be used in handling query results too, but that's for another day. Queries return every document in the result set twice, which is not only very inefficient, but also is diverges from the API.