Acts on feedback from Chris

- Added EXIF tests
- Removed pdb from image processing "ifmain"
- Fixed comment typo in image processing
- Removed unused import in tools.exif

7 files changed, 217 insertions, 11 deletions

diff --git a/mediagoblin/media_types/image/processing.py b/mediagoblin/media_types/image/processing.py
index f669e1a5..78f64be0 100644
--- a/mediagoblin/media_types/image/processing.py
+++ b/mediagoblin/media_types/image/processing.py
@@ -58,10 +58,13 @@ def process_image(entry):
     # Copy the thumb to the conversion subdir, then remotely.
     thumb_filename = 'thumbnail' + extension
     thumb_filepath = create_pub_filepath(entry, thumb_filename)
+
     tmp_thumb_filename = os.path.join(
         conversions_subdir, thumb_filename)
+
     with file(tmp_thumb_filename, 'w') as thumb_file:
         thumb.save(thumb_file)
+
     mgg.public_store.copy_local_to_storage(
         tmp_thumb_filename, thumb_filepath)
 
@@ -69,7 +72,8 @@ def process_image(entry):
     # file, a `medium.jpg` files is created and later associated with the media
     # entry.
     medium = Image.open(queued_filename)
-    # Fox orientation
+
+    # Fix orientation
     medium = exif_fix_image_orientation(medium, exif_tags)
 
     if medium.size[0] > MEDIUM_SIZE[0] or medium.size[1] > MEDIUM_SIZE[1]:
@@ -77,6 +81,7 @@ def process_image(entry):
 
     medium_filename = 'medium' + extension
     medium_filepath = create_pub_filepath(entry, medium_filename)
+
     tmp_medium_filename = os.path.join(
         conversions_subdir, medium_filename)
 
@@ -130,8 +135,5 @@ if __name__ == '__main__':
     clean = clean_exif(result)
     useful = get_useful(clean)
 
-    import pdb
-    pdb.set_trace()
-
     print pp.pprint(
         clean)
diff --git a/mediagoblin/tests/test_exif.py b/mediagoblin/tests/test_exif.py
new file mode 100644
index 00000000..9f2219c0
--- /dev/null
+++ b/mediagoblin/tests/test_exif.py
@@ -0,0 +1,189 @@
+# GNU MediaGoblin -- federated, autonomous media hosting
+# Copyright (C) 2011 MediaGoblin contributors.  See AUTHORS.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+import pkg_resources
+import Image
+
+from mediagoblin.tools.exif import exif_fix_image_orientation, \
+    extract_exif, clean_exif, get_gps_data, get_useful
+
+GOOD_JPG = pkg_resources.resource_filename(
+    'mediagoblin.tests',
+    os.path.join(
+        'test_exif',
+        'good.jpg'))
+EMPTY_JPG = pkg_resources.resource_filename(
+    'mediagoblin.tests',
+    os.path.join(
+        'test_exif',
+        'empty.jpg'))
+BAD_JPG = pkg_resources.resource_filename(
+    'mediagoblin.tests',
+    os.path.join(
+        'test_exif',
+        'bad.jpg'))
+GPS_JPG = pkg_resources.resource_filename(
+    'mediagoblin.tests',
+    os.path.join(
+        'test_exif',
+        'has-gps.jpg'))
+
+def test_exif_extraction():
+    '''
+    Test EXIF extraction from a good image
+    '''
+    result = extract_exif(GOOD_JPG)
+    clean = clean_exif(result)
+    useful = get_useful(clean)
+    gps = get_gps_data(result)
+
+    # Do we have the result?
+    assert len(result) == 108
+
+    # Do we have clean data?
+    assert len(clean) == 105
+
+    # GPS data?
+    assert gps == {}
+
+    # Do we have the "useful" tags?
+    assert useful == {
+        'EXIF Flash': {
+            'field_type': 3,
+            'printable': 'No',
+            'field_offset': 380,
+            'tag': 37385,
+            'values': [0],
+            'field_length': 2},
+        'EXIF ExposureTime': {
+            'field_type': 5,
+            'printable': '1/125',
+            'field_offset': 700,
+            'tag': 33434,
+            'values': [[1, 125]],
+            'field_length': 8},
+        'EXIF FocalLength': {
+            'field_type': 5,
+            'printable': '18',
+            'field_offset': 780,
+            'tag': 37386,
+            'values': [[18, 1]],
+            'field_length': 8},
+        'Image Model': {
+            'field_type': 2,
+            'printable': 'NIKON D80',
+            'field_offset': 152,
+            'tag': 272,
+            'values': 'NIKON D80',
+            'field_length': 10},
+        'Image Make': {
+            'field_type': 2,
+            'printable': 'NIKON CORPORATION',
+            'field_offset': 134,
+            'tag': 271,
+            'values': 'NIKON CORPORATION',
+            'field_length': 18},
+        'EXIF ExposureMode': {
+            'field_type': 3,
+            'printable': 'Manual Exposure',
+            'field_offset': 584,
+            'tag': 41986,
+            'values': [1],
+            'field_length': 2},
+        'EXIF ISOSpeedRatings': {
+            'field_type': 3,
+            'printable': '100',
+            'field_offset': 260,
+            'tag': 34855,
+            'values': [100],
+            'field_length': 2},
+        'EXIF FNumber': {
+            'field_type': 5,
+            'printable': '10',
+            'field_offset': 708,
+            'tag': 33437,
+            'values': [[10, 1]],
+            'field_length': 8},
+        'EXIF UserComment': {
+            'field_type': 7,
+            'printable': 'Joar Wandborg                       ',
+            'field_offset': 26180,
+            'tag': 37510,
+            'values': [
+                65, 83, 67, 73, 73, 0, 0, 0, 74, 111, 97, 114, 32, 87,
+                97, 110, 100, 98, 111, 114, 103, 32, 32, 32, 32, 32, 32,
+                32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32, 32,
+                32, 32, 32],
+            'field_length': 44}}
+
+def test_exif_image_orientation():
+    '''
+    Test image reorientation based on EXIF data
+    '''
+    result = extract_exif(GOOD_JPG)
+
+    image = exif_fix_image_orientation(
+        Image.open(GOOD_JPG),
+        result)
+
+    # Are the dimensions correct?
+    assert image.size == (428, 640)
+
+    # If this pixel looks right, the rest of the image probably will too.
+    assert image.getdata()[10000] == (41, 28, 11)
+
+def test_exif_no_exif():
+    '''
+    Test an image without exif
+    '''
+    result = extract_exif(EMPTY_JPG)
+    clean = clean_exif(result)
+    useful = get_useful(clean)
+    gps = get_gps_data(result)
+
+    assert result == {}
+    assert clean == {}
+    assert gps == {}
+    assert useful == {}
+
+def test_exif_bad_image():
+    '''
+    Test EXIF extraction from a faithful, but bad image
+    '''
+    result = extract_exif(BAD_JPG)
+    clean = clean_exif(result)
+    useful = get_useful(clean)
+    gps = get_gps_data(result)
+
+    assert result == {}
+    assert clean == {}
+    assert gps == {}
+    assert useful == {}
+
+def test_exif_gps_data():
+    '''
+    Test extractiion of GPS data
+    '''
+    result = extract_exif(GPS_JPG)
+    gps = get_gps_data(result)
+
+    assert gps == {
+        'latitude': 59.336666666666666,
+        'direction': 25.674046740467404,
+        'altitude': 37.64365671641791,
+        'longitude': 18.016166666666667}
+    
diff --git a/mediagoblin/tests/test_exif/bad.jpg b/mediagoblin/tests/test_exif/bad.jpg
new file mode 100644
index 00000000..4cde23cd
--- /dev/null
+++ b/mediagoblin/tests/test_exif/bad.jpg
@@ -0,0 +1,18 @@
+V2UncmUgbm8gc3RyYW5nZXJzIHRvIGxvdmUKWW91IGtub3cgdGhlIHJ1bGVzIGFuZCBzbyBkbyBJ
+CkEgZnVsbCBjb21taXRtZW50J3Mgd2hhdCBJJ20gdGhpbmtpbicgb2YKWW91IHdvdWxkbid0IGdl
+dCB0aGlzIGZyb20gYW55IG90aGVyIGd1eQpJIGp1c3Qgd2FubmEgdGVsbCB5b3UgaG93IEknbSBm
+ZWVsaW4nCkdvdHRhIG1ha2UgeW91IHVuZGVyc3RhbmQKCihDaG9ydXMpCk5ldmVyIGdvbm5hIGdp
+dmUgeW91IHVwCk5ldmVyIGdvbm5hIGxldCB5b3UgZG93bgpOZXZlciBnb25uYSBydW4gYXJvdW5k
+IGFuZCBkZXNlcnQgeW91Ck5ldmVyIGdvbm5hIG1ha2UgeW91IGNyeQpOZXZlciBnb25uYSBzYXkg
+Z29vZGJ5ZQpOZXZlciBnb25uYSB0ZWxsIGEgbGllIGFuZCBodXJ0IHlvdQoKV2UndmUga25vdyBl
+YWNoIG90aGVyIGZvciBzbyBsb25nCllvdXIgaGVhcnQncyBiZWVuIGFjaGluJyBidXQgeW91J3Jl
+IHRvbyBzaHkgdG8gc2F5IGl0Ckluc2lkZSB3ZSBib3RoIGtub3cgd2hhdCdzIGJlZW4gZ29pbmcg
+b24KV2Uga25vdyB0aGUgZ2FtZSBhbmQgd2UncmUgZ29ubmEgcGxheSBpdApBbmQgaWYgeW91IGFz
+ayBtZSBob3cgSSdtIGZlZWxpbicKRG9uJ3QgdGVsbCBtZSB5b3UncmUgdG9vIGJsaW5kIHRvIHNl
+ZQoKKENob3J1cyB4MikKCihHaXZlIHlvdSB1cCwgZ2l2ZSB5b3UgdXApCk5ldmVyIGdvbm5hIGdp
+dmUsIG5ldmVyIGdvbm5hIGdpdmUKKEdpdmUgeW91IHVwKQpOZXZlciBnb25uYSBnaXZlLCBuZXZl
+ciBnb25uYSBnaXZlCihHaXZlIHlvdSB1cCkKCldlJ3ZlIGtub3cgZWFjaCBvdGhlciBmb3Igc28g
+bG9uZwpZb3VyIGhlYXJ0J3MgYmVlbiBhY2hpbicgYnV0IHlvdSdyZSB0b28gc2h5IHRvIHNheSBp
+dApJbnNpZGUgd2UgYm90aCBrbm93IHdoYXQncyBiZWVuIGdvaW5nIG9uCldlIGtub3cgdGhlIGdh
+bWUgYW5kIHdlJ3JlIGdvbm5hIHBsYXkgaXQKSSBqdXN0IHdhbm5hIHRlbGwgeW91IGhvdyBJJ20g
+ZmVlbGluJwpHb3R0YSBtYWtlIHlvdSB1bmRlcnN0YW5kCgooQ2hvcnVzIHgzKQo=
diff --git a/mediagoblin/tests/test_exif/empty.jpg b/mediagoblin/tests/test_exif/empty.jpg
new file mode 100644
index 00000000..37533af5
--- /dev/null
+++ b/mediagoblin/tests/test_exif/empty.jpg
diff --git a/mediagoblin/tests/test_exif/good.jpg b/mediagoblin/tests/test_exif/good.jpg
new file mode 100644
index 00000000..0ee956fe
--- /dev/null
+++ b/mediagoblin/tests/test_exif/good.jpg
diff --git a/mediagoblin/tests/test_exif/has-gps.jpg b/mediagoblin/tests/test_exif/has-gps.jpg
new file mode 100644
index 00000000..c7d2cc93
--- /dev/null
+++ b/mediagoblin/tests/test_exif/has-gps.jpg
diff --git a/mediagoblin/tools/exif.py b/mediagoblin/tools/exif.py
index 445907ba..3c1aebe5 100644
--- a/mediagoblin/tools/exif.py
+++ b/mediagoblin/tools/exif.py
@@ -18,8 +18,6 @@ from mediagoblin.tools.extlib.EXIF import process_file, Ratio
 from mediagoblin.processing import BadMediaFail
 from mediagoblin.tools.translate import pass_to_ugettext as _
 
-from collections import OrderedDict
-
 # A list of tags that should be stored for faster access
 USEFUL_TAGS = [
     'Image Make',
@@ -73,8 +71,7 @@ def extract_exif(filename):
 
 def clean_exif(exif):
     '''
-    Clean the result from anyt
-hing the database cannot handle
+    Clean the result from anything the database cannot handle
     '''
     # Discard any JPEG thumbnail, for database compatibility
     # and that I cannot see a case when we would use it.
@@ -129,11 +126,11 @@ def get_gps_data(tags):
     """
     Processes EXIF data returned by EXIF.py
     """
-    if not 'Image GPSInfo' in tags:
-        return False
-
     gps_data = {}
 
+    if not 'Image GPSInfo' in tags:
+        return gps_data
+
     try:
         dms_data = {
             'latitude': tags['GPS GPSLatitude'],