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

reference form attachments in case attachments instead of copying #13200

Merged
merged 6 commits into from
Sep 13, 2016
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions corehq/ex-submodules/casexml/apps/case/tests/test_multimedia.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
from casexml.apps.case.tests.util import TEST_DOMAIN_NAME
from casexml.apps.case.xml import V2
from corehq.apps.receiverwrapper.util import submit_form_locally
from corehq.blobs import get_blob_db
from corehq.blobs.tests.util import TemporaryS3BlobDB
from corehq.form_processor.interfaces.dbaccessors import CaseAccessors, FormAccessors
from couchforms.models import XFormInstance
from dimagi.utils.parsing import json_format_datetime
from corehq.form_processor.interfaces.processor import FormProcessorInterface
from corehq.form_processor.tests.utils import FormProcessorTestUtils, run_with_all_backends
from corehq.util.test_utils import TestFileMixin

from corehq.util.test_utils import TestFileMixin, trap_extra_setup

TEST_CASE_ID = "EOL9FIAKIQWOFXFOH0QAMWU64"
CREATE_XFORM_ID = "6RGAZTETE3Z2QC0PE2DKM88MO"
Expand Down Expand Up @@ -300,3 +301,33 @@ def test_sync_log_invalidation_bug(self):
# this used to fail before we fixed http://manage.dimagi.com/default.asp?158373
self._doSubmitUpdateWithMultimedia(new_attachments=['commcare_logo_file'], removes=[],
sync_token=sync_log._id)


class CaseMultimediaS3DBTest(BaseCaseMultimediaTest):
"""
Tests new attachments for cases and case properties
Spec: https://github.com/dimagi/commcare/wiki/CaseAttachmentAPI
"""

def setUp(self):
super(CaseMultimediaS3DBTest, self).setUp()
with trap_extra_setup(AttributeError, msg="S3_BLOB_DB_SETTINGS not configured"):
config = settings.S3_BLOB_DB_SETTINGS
self.s3db = TemporaryS3BlobDB(config)
assert get_blob_db() is self.s3db, (get_blob_db(), self.s3db)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the above two lines don't need to (and probably shouldn't) be in the with trap_extra_setup block.


def tearDown(self):
self.s3db.close()
super(CaseMultimediaS3DBTest, self).tearDown()

@run_with_all_backends
def test_case_attachment(self):
single_attach = 'fruity_file'
xform, case = self._doCreateCaseWithMultimedia(attachments=[single_attach])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sends attachments to submit_form_locally, which I would expect to work. I thought case attachments were working, but still stored in couch. Is that not true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case attachments come from form attachments hence the call to
submit_form_locally

response, form, cases = submit_form_locally(
xml_data,
TEST_DOMAIN_NAME,
attachments=dict_attachments,
last_sync_token=sync_token,
received_on=date
)
.

The SQL backend uses the blobdb for all attachments (form and case).
Currently we duplicate form attachments for cases so one thing we could
consider is just referencing the same blob in both the form and case.

I also see that boto S3 has a copy API method which we could also use
rather than doing a read and then write.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying out the copy API right now. Will have a PR soon, hopefully.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


self.assertEqual(1, len(case.case_attachments))
self.assertTrue(single_attach in case.case_attachments)
self.assertEqual(
self._calc_file_hash(single_attach),
hashlib.md5(case.get_attachment(single_attach)).hexdigest()
)
7 changes: 2 additions & 5 deletions corehq/form_processor/backends/sql/update_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,12 @@ def _apply_attachments_action(self, attachment_action, xform):
new_attachment = CaseAttachmentSQL.from_case_update(att)
if new_attachment.is_present:
form_attachment = xform.get_attachment_meta(att.attachment_src)
new_attachment.update_from_attachment(form_attachment)

if identifier in current_attachments:
existing_attachment = current_attachments[identifier]
existing_attachment.update_from_attachment(new_attachment)
existing_attachment.copy_content(form_attachment)
existing_attachment.from_form_attachment(form_attachment)
self.case.track_update(existing_attachment)
else:
new_attachment.copy_content(form_attachment)
new_attachment.from_form_attachment(form_attachment)
new_attachment.case = self.case
self.case.track_create(new_attachment)
elif identifier in current_attachments:
Expand Down
29 changes: 16 additions & 13 deletions corehq/form_processor/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,19 @@ class XFormAttachmentSQL(AbstractAttachment, IsImageMixin):
related_name=AttachmentMixin.ATTACHMENTS_RELATED_NAME, related_query_name="attachment"
)

def __unicode__(self):
return unicode(
"XFormAttachmentSQL("
"attachment_id='{a.attachment_id}', "
"form_id='{a.form_id}', "
"name='{a.name}', "
"content_type='{a.content_type}', "
"content_length='{a.content_length}', "
"md5='{a.md5}', "
"blob_id='{a.blob_id}', "
"properties='{a.properties}', "
).format(a=self)

class Meta:
db_table = XFormAttachmentSQL_DB_TABLE
app_label = "form_processor"
Expand Down Expand Up @@ -856,14 +869,15 @@ class CaseAttachmentSQL(AbstractAttachment, CaseAttachmentMixin):
attachment_src = models.TextField(null=True)
attachment_from = models.TextField(null=True)

def update_from_attachment(self, attachment):
def from_form_attachment(self, attachment):
"""
Update fields in this attachment with fields from anaother attachment
Update fields in this attachment with fields from another attachment

:param attachment: XFormAttachmentSQL or CaseAttachmentSQL object
"""
self.content_length = attachment.content_length
self.blob_id = attachment.blob_id
self.blob_bucket = attachment._blobdb_bucket()
self.md5 = attachment.md5
self.content_type = attachment.content_type
self.properties = attachment.properties
Expand All @@ -878,17 +892,6 @@ def update_from_attachment(self, attachment):
self.attachment_src = attachment.attachment_src
self.attachment_from = attachment.attachment_from

def copy_content(self, attachment):
if self.is_saved():
deleted = self.delete_content()
if not deleted:
logging.warn(
"Case attachment content not deleted. bucket=%s, blob_id=%s",
self._blobdb_bucket(), self.blob_id
)
content = attachment.read_content(stream=True)
self.write_content(content)

@classmethod
def from_case_update(cls, attachment):
if attachment.attachment_src:
Expand Down