Attachments transform

[stevebest]

Includes support for preserving digest and length of stub: true attachments.

This is like #97, only a bit better. :)

[simonexmachina]

@stevebest I think that the transforms should return [] rather than null so that you can add attachments to an object that was previously saved before the attr('attachment') was added.

[simonexmachina]

I've added code to return [] and {} from the transform when there is no value, but even with those changes, the property returns null when the model hasn't been saved:

console.log(store.createRecord('pca').get('files')) // => null
store.createRecord('pca').save().then(pca => console.log(pca.get('files'))) // => []

It looks like the transform isn't called when the model hasn't been saved, which makes sense, but does provide a challenge. I suppose we can just say that you need to save the model before adding files to it, but it doesn't seem right to me.

I'll submit a PR to your fork now.

[fsmanuel]

@aexmachina You can use defaultValue.

photos: DS.attr('attachment', { defaultValue: [] });

I guess the best is to mention it in the readme.
See the options section in the ember data docs

[stevebest]

Yeah, it probably makes sense to deserialize "no attachments" into an [].

Meanwhile, when creating new instances of models with attachments, you can be explicit about them:

const post = store.createRecord('post', {
  title: 'How to attach files',
  attachments: [],  // It's a DS.attr('attachment')
});

Or you could be smarter than me and use defaultValue as @fsmanuel suggested. :)

[simonexmachina]

@stevebest another thought is that there's no need for the defaultValue solution if you're just setting a single file. What about having two transforms: attachment and attachments?]

Also I think the README should include the defaultValue solution, because otherwise the behaviour is quite unexpected:

// myapp/models/photo-album.js
export default DS.Model.extend({
  photos: DS.attr('attachment', {
    defaultValue: () => []
  });
});

[simonexmachina]

This is working as expected now, however I've noticed that the data that's sent to Couch is empty if the PouchDB instance is set to a remote URL.

When I use new PouchDB('files_development') the attachment is stored as expected, but when the PouchDB instance is created using new PouchDB('http://localhost:5984/files_development') then the request that gets sent contains empty data:

POST http://localhost:5984/files_development/_bulk_docs
{"docs":[{"_id":"file_2_9967B44B-C266-2D54-9FAE-5A108B63891B","data":{"owner":"E2878890-34E9-5B27-B0EE-C60AF5862217","ownerType":"pca","attachment":{"test.png":{"content_type":"image/png","data":{}}}}}],"new_edits":true}

@nolanlawson I can't really say, but it feels to me that this might be a problem with Relational Pouch or PouchDB itself? How could I investigate further?

I've confirmed that this does work if I use PouchDB directly:

new PouchDB('http://localhost:5984/files_development').put({
  _id: 'test',
  _attachments: {
    'test.png': {
      content_type: 'text/png',
      data: document.querySelector('input[type=file]').files[0]
    }
  }
})

[simonexmachina]

I've been looking into this and I can't see how this works at all. Relational Pouch checks for an attachments property in toRawDoc, but I can't see how the values produced by the transform become attachments at all. Some guidance would be appreciated!

[nolanlawson]

@aexmachina Hm my hunch is that the test is failing for CouchDB because it only inserts a stub. CouchDB expects you to insert a stub only if you've already inserted the full attachment at some point. PouchDB may be more lenient on this.

My recommendation would be to fix the test so that it truly inserts a real attachment, then afterwards update that same document but include the stub. Also protip: use non-text data (e.g. a GIF, PNG, etc.) so that the md5 sum is the same between Pouch and Couch; will make the test easier to write.

[simonexmachina]

@nolanlawson I'm not using the tests, I'm just trying to get my feature completed (it's taken longer than expected, so just focused on getting it to work right now). It works when the model uses a local PouchDB, and doesn't work when it's a CouchDB.

If you can point me in the right direction then I can probably work it out, but I can't currently see where the data produced by this transform is handled as an attachment at all. I'd be really appreciative if we could screenshare for a few minutes and try to get it worked out.

[stevebest]

@aexmachina Uhm, guys, I lied to you. You MUST name the field of your model attachments, plural, for it to work. So yeah.

const Foo = DS.Model.extend({
   attachments: DS.attr('attachment'),
});

Also, I should probably rename the transform to be plural, too, because this is ridiculous. Sorry about me being an idiot. :)

The thing is, originally I thought about having several attrs like this:

const Foo = DS.Model.extend({
  image: DS.attr('attachment'),
  song: DS.attr('attachment'),
  maybeSomeOtherFile: DS.attr('attachment'),
});

but it doesn't quite work because Transforms only look at one attr at a time, and we need to somehow gather several values into one object.

[simonexmachina]

@stevebest I'm working on this as well and nearly have a solution for that problem.

[simonexmachina]

I've submitted a PR to @stevebest's fork that contains attachment and attachments transforms, and support for attributes with any name.

[simonexmachina]

@stevebest, closing this PR in favour of #136.