refactored template code to fix #213

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file. Dates are d
 
 Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 
+#### [v0.35.3](https://github.com/RhetTbull/osxphotos/compare/v0.35.2...v0.35.3)
+
+> 15 October 2020
+
+- Fix for issue #235, #236 [`41b2399`](https://github.com/RhetTbull/osxphotos/commit/41b23991df3d1d553b70889ede237f83b6874519)
+
+#### [v0.35.2](https://github.com/RhetTbull/osxphotos/compare/v0.35.1...v0.35.2)
+
+> 12 October 2020
+
+- Fix for issue #234 [`da100f9`](https://github.com/RhetTbull/osxphotos/commit/da100f93a9b849ca4750336d7f90e9023e39dd07)
+
 #### [v0.35.1](https://github.com/RhetTbull/osxphotos/compare/v0.35.0...v0.35.1)
 
 > 12 October 2020

osxphotos/__main__.py

@@ -1,9 +1,7 @@
 """ command line interface for osxphotos """
 import csv
 import datetime
-import functools
 import json
-import logging
 import os
 import os.path
 import pathlib
@@ -14,12 +12,6 @@ import unicodedata
 
 import click
 import yaml
-from pathvalidate import (
-    is_valid_filename,
-    is_valid_filepath,
-    sanitize_filename,
-    sanitize_filepath,
-)
 
 import osxphotos
 
@@ -29,11 +21,12 @@ from ._constants import (
     _UNKNOWN_PLACE,
     UNICODE_FORMAT,
 )
-from .export_db import ExportDB, ExportDBInMemory
 from ._version import __version__
 from .datetime_formatter import DateTimeFormatter
 from .exiftool import get_exiftool_path
+from .export_db import ExportDB, ExportDBInMemory
 from .fileutil import FileUtil, FileUtilNoOp
+from .path_utils import is_valid_filepath, sanitize_filename, sanitize_filepath
 from .photoinfo import ExportResults
 from .phototemplate import TEMPLATE_SUBSTITUTIONS, TEMPLATE_SUBSTITUTIONS_MULTI_VALUED
 
@@ -1240,10 +1233,10 @@ def query(
     "--jpeg-quality",
     type=click.FloatRange(0.0, 1.0),
     default=1.0,
-    help="Value in range 0.0 to 1.0 to use with --convert-to-jpeg. " 
+    help="Value in range 0.0 to 1.0 to use with --convert-to-jpeg. "
     "A value of 1.0 specifies best quality, "
     "a value of 0.0 specifies maximum compression. "
-    "Defaults to 1.0." 
+    "Defaults to 1.0.",
 )
 @click.option(
     "--sidecar",
@@ -2409,7 +2402,9 @@ def get_filenames_from_template(photo, filename_template, original_name):
     """
     if filename_template:
         photo_ext = pathlib.Path(photo.original_filename).suffix
-        filenames, unmatched = photo.render_template(filename_template, path_sep="_")
+        filenames, unmatched = photo.render_template(
+            filename_template, path_sep="_", filename=True
+        )
         if not filenames or unmatched:
             raise click.BadOptionUsage(
                 "filename_template",
@@ -2418,6 +2413,8 @@ def get_filenames_from_template(photo, filename_template, original_name):
         filenames = [f"{file_}{photo_ext}" for file_ in filenames]
     else:
         filenames = [photo.original_filename] if original_name else [photo.filename]
+
+    filenames = [sanitize_filename(filename) for filename in filenames]
     return filenames
 
 
@@ -2448,22 +2445,18 @@ def get_dirnames_from_template(photo, directory, export_by_date, dest, dry_run):
         dest_paths = [dest_path]
     elif directory:
         # got a directory template, render it and check results are valid
-        dirnames, unmatched = photo.render_template(directory)
+        dirnames, unmatched = photo.render_template(directory, dirname=True)
-        if not dirnames:
+        if not dirnames or unmatched:
-            raise click.BadOptionUsage(
-                "directory",
-                f"Invalid template '{directory}': results={dirnames} unmatched={unmatched}",
-            )
-        elif unmatched:
             raise click.BadOptionUsage(
                 "directory",
                 f"Invalid template '{directory}': results={dirnames} unmatched={unmatched}",
             )
+
         dest_paths = []
         for dirname in dirnames:
-            dirname = sanitize_filepath(dirname, platform="auto")
+            dirname = sanitize_filepath(dirname)
             dest_path = os.path.join(dest, dirname)
-            if not is_valid_filepath(dest_path, platform="auto"):
+            if not is_valid_filepath(dest_path):
                 raise ValueError(f"Invalid file path: '{dest_path}'")
             if not dry_run and not os.path.isdir(dest_path):
                 os.makedirs(dest_path)
@@ -2491,7 +2484,7 @@ def find_files_in_branch(pathname, filename):
     files = []
 
     # walk down the tree
-    for root, directories, filenames in os.walk(pathname):
+    for root, _, filenames in os.walk(pathname):
         # for directory in directories:
         # print(os.path.join(root, directory))
         for fname in filenames:

osxphotos/_constants.py

@@ -102,3 +102,10 @@ _OSXPHOTOS_NONE_SENTINEL = "OSXPhotosXYZZY42_Sentinel$"
 
 # SearchInfo categories for Photos 5, corresponds to categories in database/search/psi.sqlite
 SEARCH_CATEGORY_LABEL = 2024
+
+# Max filename length on MacOS
+MAX_FILENAME_LEN = 255
+
+# Max directory name length on MacOS
+MAX_DIRNAME_LEN = 255
+

osxphotos/_version.py

@@ -1,4 +1,4 @@
 """ version info """
 
-__version__ = "0.35.2"
+__version__ = "0.35.4"
 

osxphotos/path_utils.py

@@ -0,0 +1,78 @@
+""" utility functions for validating/sanitizing path components """
+
+from ._constants import MAX_DIRNAME_LEN, MAX_FILENAME_LEN
+import pathvalidate
+
+
+def sanitize_filepath(filepath):
+    """ sanitize a filepath """
+    return pathvalidate.sanitize_filepath(filepath, platform="macos")
+
+
+def is_valid_filepath(filepath):
+    """ returns True if a filepath is valid otherwise False """
+    return pathvalidate.is_valid_filepath(filepath, platform="macos")
+
+
+def sanitize_filename(filename, replacement=":"):
+    """ replace any illegal characters in a filename and truncate filename if needed
+
+    Args:
+        filename: str, filename to sanitze
+        replacement: str, value to replace any illegal characters with; default = ":"
+    
+    Returns:
+        filename with any illegal characters replaced by replacement and truncated if necessary
+    """
+
+    if filename:
+        filename = filename.replace("/", replacement)
+        if len(filename) > MAX_FILENAME_LEN:
+            parts = filename.split(".")
+            drop = len(filename) - MAX_FILENAME_LEN
+            if len(parts) > 1:
+                # has an extension
+                ext = parts.pop(-1)
+                stem = ".".join(parts)
+                if drop > len(stem):
+                    ext = ext[:-drop]
+                else:
+                    stem = stem[:-drop]
+                filename = f"{stem}.{ext}"
+            else:
+                filename = filename[:-drop]
+    return filename
+
+
+def sanitize_dirname(dirname, replacement=":"):
+    """ replace any illegal characters in a directory name and truncate directory name if needed
+
+    Args:
+        dirname: str, directory name to sanitze
+        replacement: str, value to replace any illegal characters with; default = ":"
+    
+    Returns:
+        dirname with any illegal characters replaced by replacement and truncated if necessary
+    """
+    if dirname:
+        dirname = sanitize_pathpart(dirname, replacement=replacement)
+    return dirname
+
+
+def sanitize_pathpart(pathpart, replacement=":"):
+    """ replace any illegal characters in a path part (either directory or filename without extension) and truncate name if needed
+
+    Args:
+        pathpart: str, path part to sanitze
+        replacement: str, value to replace any illegal characters with; default = ":"
+    
+    Returns:
+        pathpart with any illegal characters replaced by replacement and truncated if necessary
+    """
+    if pathpart:
+        pathpart = pathpart.replace("/", replacement)
+        if len(pathpart) > MAX_DIRNAME_LEN:
+            drop = len(pathpart) - MAX_DIRNAME_LEN
+            pathpart = pathpart[:-drop]
+    return pathpart
+

osxphotos/photoinfo/_photoinfo_export.py

@@ -592,9 +592,8 @@ def export2(
                     export_as_hardlink,
                     exiftool,
                     touch_file,
-                    convert_to_jpeg,
+                    False,
                     fileutil=fileutil,
-                    jpeg_quality=jpeg_quality,
                 )
                 exported_files.extend(results.exported)
                 update_new_files.extend(results.new)

osxphotos/photoinfo/photoinfo.py

@@ -545,6 +545,9 @@ class PhotoInfo:
         """
         if self._db._db_version <= _PHOTOS_4_VERSION and self._info["has_raw"]:
             return self._info["raw_pair_info"]["UTI"]
+        elif self.shared:
+            # TODO: need reliable way to get original UTI for shared
+            return self.uti
         else:
             return self._info["UTI_original"]
 
@@ -805,6 +808,9 @@ class PhotoInfo:
         path_sep=None,
         expand_inplace=False,
         inplace_sep=None,
+        filename=False,
+        dirname=False,
+        replacement=":",
     ):
         """Renders a template string for PhotoInfo instance using PhotoTemplate
 
@@ -817,6 +823,9 @@ class PhotoInfo:
                 instead of returning individual strings
             inplace_sep: optional string to use as separator between multi-valued keywords
                 with expand_inplace; default is ','
+            filename: if True, template output will be sanitized to produce valid file name
+            dirname: if True, template output will be sanitized to produce valid directory name 
+            replacement: str, value to replace any illegal file path characters with; default = ":"
         
         Returns:
             ([rendered_strings], [unmatched]): tuple of list of rendered strings and list of unmatched template values
@@ -828,6 +837,9 @@ class PhotoInfo:
             path_sep=path_sep,
             expand_inplace=expand_inplace,
             inplace_sep=inplace_sep,
+            filename=filename,
+            dirname=dirname,
+            replacement=replacement
         )
 
     @property

osxphotos/photosdb/photosdb.py

@@ -1908,6 +1908,7 @@ class PhotosDB:
                 info["type"] = None
 
             info["UTI"] = row[18]
+            info["UTI_original"] = None # filled in later
 
             # handle burst photos
             # if burst photo, determine whether or not it's a selected burst photo

osxphotos/phototemplate.py

@@ -12,11 +12,13 @@
 import datetime
 import locale
 import os
-import re
 import pathlib
+import re
+from functools import partial
 
 from ._constants import _UNKNOWN_PERSON
 from .datetime_formatter import DateTimeFormatter
+from .path_utils import sanitize_dirname, sanitize_filename, sanitize_pathpart
 
 # ensure locale set to user's locale
 locale.setlocale(locale.LC_ALL, "")
@@ -131,6 +133,9 @@ class PhotoTemplate:
         path_sep=None,
         expand_inplace=False,
         inplace_sep=None,
+        filename=False,
+        dirname=False,
+        replacement=":",
     ):
         """ Render a filename or directory template 
 
@@ -142,6 +147,9 @@ class PhotoTemplate:
                 instead of returning individual strings
             inplace_sep: optional string to use as separator between multi-valued keywords
             with expand_inplace; default is ','
+            filename: if True, template output will be sanitized to produce valid file name
+            dirname: if True, template output will be sanitized to produce valid directory name 
+            replacement: str, value to replace any illegal file path characters with; default = ":"
 
         Returns:
             ([rendered_strings], [unmatched]): tuple of list of rendered strings and list of unmatched template values
@@ -170,13 +178,21 @@ class PhotoTemplate:
         if type(template) is not str:
             raise TypeError(f"template must be type str, not {type(template)}")
 
-        def make_subst_function(self, none_str, get_func=self.get_template_value):
+        # used by make_subst_function to get the value for a template substitution
+        get_func = partial(
+            self.get_template_value,
+            filename=filename,
+            dirname=dirname,
+            replacement=replacement,
+        )
+
+        def make_subst_function(self, none_str, get_func=get_func):
             """ returns: substitution function for use in re.sub 
                 none_str: value to use if substitution lookup is None and no default provided
                 get_func: function that gets the substitution value for a given template field
                         default is get_template_value which handles the single-value fields """
 
-            # closure to capture photo, none_str in subst
+            # closure to capture photo, none_str, filename, dirname in subst
             def subst(matchobj):
                 groups = len(matchobj.groups())
                 if groups == 4:
@@ -186,13 +202,13 @@ class PhotoTemplate:
                         return matchobj.group(0)
 
                     if val is None:
-                        return (
+                        val = (
                             matchobj.group(3)
                             if matchobj.group(3) is not None
                             else none_str
                         )
-                    else:
+
-                        return val
+                    return val
                 else:
                     raise ValueError(
                         f"Unexpected number of groups: expected 4, got {groups}"
@@ -239,7 +255,13 @@ class PhotoTemplate:
 
             for str_template in rendered_strings:
                 if regex_multi.search(str_template):
-                    values = self.get_template_value_multi(field, path_sep)
+                    values = self.get_template_value_multi(
+                        field,
+                        path_sep,
+                        filename=filename,
+                        dirname=dirname,
+                        replacement=replacement,
+                    )
                     if expand_inplace:
                         # instead of returning multiple strings, join values into a single string
                         val = (
@@ -248,11 +270,11 @@ class PhotoTemplate:
                             else None
                         )
 
-                        def lookup_template_value_multi(lookup_value, default):
+                        def lookup_template_value_multi(lookup_value, _):
                             """ Closure passed to make_subst_function get_func 
                                     Capture val and field in the closure 
                                     Allows make_subst_function to be re-used w/o modification
-                                    default is not used but required so signature matches get_template_value """
+                                    _ is not used but required so signature matches get_template_value """
                             if lookup_value == field:
                                 return val
                             else:
@@ -269,11 +291,11 @@ class PhotoTemplate:
                         # create a new template string for each value
                         for val in values:
 
-                            def lookup_template_value_multi(lookup_value, default):
+                            def lookup_template_value_multi(lookup_value, _):
                                 """ Closure passed to make_subst_function get_func 
                                     Capture val and field in the closure 
                                     Allows make_subst_function to be re-used w/o modification
-                                    default is not used but required so signature matches get_template_value """
+                                    _ is not used but required so signature matches get_template_value """
                                 if lookup_value == field:
                                     return val
                                 else:
@@ -307,14 +329,24 @@ class PhotoTemplate:
             for rendered_str in rendered_strings
         ]
 
+        if filename:
+            rendered_strings = [
+                sanitize_filename(rendered_str) for rendered_str in rendered_strings
+            ]
+
         return rendered_strings, unmatched
 
-    def get_template_value(self, field, default):
+    def get_template_value(
+        self, field, default, filename=False, dirname=False, replacement=":"
+    ):
         """lookup value for template field (single-value template substitutions)
 
         Args:
             field: template field to find value for.
             default: the default value provided by the user
+            filename: if True, template output will be sanitized to produce valid file name
+            dirname: if True, template output will be sanitized to produce valid directory name 
+            replacement: str, value to replace any illegal file path characters with; default = ":"
         
         Returns:
             The matching template value (which may be None).
@@ -327,289 +359,236 @@ class PhotoTemplate:
         if self.today is None:
             self.today = datetime.datetime.now()
 
-        # must be a valid keyword
+        value = None
+
+        # wouldn't a switch/case statement be nice...
         if field == "name":
-            return pathlib.Path(self.photo.filename).stem
+            value = pathlib.Path(self.photo.filename).stem
-
+        elif field == "original_name":
-        if field == "original_name":
+            value = pathlib.Path(self.photo.original_filename).stem
-            return pathlib.Path(self.photo.original_filename).stem
+        elif field == "title":
-
+            value = self.photo.title
-        if field == "title":
+        elif field == "descr":
-            return self.photo.title
+            value = self.photo.description
-
+        elif field == "created.date":
-        if field == "descr":
+            value = DateTimeFormatter(self.photo.date).date
-            return self.photo.description
+        elif field == "created.year":
-
+            value = DateTimeFormatter(self.photo.date).year
-        if field == "created.date":
+        elif field == "created.yy":
-            return DateTimeFormatter(self.photo.date).date
+            value = DateTimeFormatter(self.photo.date).yy
-
+        elif field == "created.mm":
-        if field == "created.year":
+            value = DateTimeFormatter(self.photo.date).mm
-            return DateTimeFormatter(self.photo.date).year
+        elif field == "created.month":
-
+            value = DateTimeFormatter(self.photo.date).month
-        if field == "created.yy":
+        elif field == "created.mon":
-            return DateTimeFormatter(self.photo.date).yy
+            value = DateTimeFormatter(self.photo.date).mon
-
+        elif field == "created.dd":
-        if field == "created.mm":
+            value = DateTimeFormatter(self.photo.date).dd
-            return DateTimeFormatter(self.photo.date).mm
+        elif field == "created.dow":
-
+            value = DateTimeFormatter(self.photo.date).dow
-        if field == "created.month":
+        elif field == "created.doy":
-            return DateTimeFormatter(self.photo.date).month
+            value = DateTimeFormatter(self.photo.date).doy
-
+        elif field == "created.hour":
-        if field == "created.mon":
+            value = DateTimeFormatter(self.photo.date).hour
-            return DateTimeFormatter(self.photo.date).mon
+        elif field == "created.min":
-
+            value = DateTimeFormatter(self.photo.date).min
-        if field == "created.dd":
+        elif field == "created.sec":
-            return DateTimeFormatter(self.photo.date).dd
+            value = DateTimeFormatter(self.photo.date).sec
-
+        elif field == "created.strftime":
-        if field == "created.dow":
-            return DateTimeFormatter(self.photo.date).dow
-
-        if field == "created.doy":
-            return DateTimeFormatter(self.photo.date).doy
-
-        if field == "created.hour":
-            return DateTimeFormatter(self.photo.date).hour
-
-        if field == "created.min":
-            return DateTimeFormatter(self.photo.date).min
-
-        if field == "created.sec":
-            return DateTimeFormatter(self.photo.date).sec
-
-        if field == "created.strftime":
             if default:
                 try:
-                    return self.photo.date.strftime(default)
+                    value = self.photo.date.strftime(default)
                 except:
                     raise ValueError(f"Invalid strftime template: '{default}'")
             else:
-                return None
+                value = None
-
+        elif field == "modified.date":
-        if field == "modified.date":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).date
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.year":
-        if field == "modified.year":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).year
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.yy":
-        if field == "modified.yy":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).yy
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.mm":
-        if field == "modified.mm":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).mm
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.month":
-        if field == "modified.month":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).month
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.mon":
-        if field == "modified.mon":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).mon
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.dd":
-        if field == "modified.dd":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).dd
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.doy":
-        if field == "modified.doy":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).doy
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.hour":
-        if field == "modified.hour":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).hour
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.min":
-        if field == "modified.min":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).min
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "modified.sec":
-        if field == "modified.sec":
+            value = (
-            return (
                 DateTimeFormatter(self.photo.date_modified).sec
                 if self.photo.date_modified
                 else None
             )
-
+        elif field == "today.date":
-        # TODO: disabling modified.strftime for now because now clean way to pass
+            value = DateTimeFormatter(self.today).date
-        # a default value if modified time is None
+        elif field == "today.year":
-        # if field == "modified.strftime":
+            value = DateTimeFormatter(self.today).year
-        #     if default and self.photo.date_modified:
+        elif field == "today.yy":
-        #         try:
+            value = DateTimeFormatter(self.today).yy
-        #             return self.photo.date_modified.strftime(default)
+        elif field == "today.mm":
-        #         except:
+            value = DateTimeFormatter(self.today).mm
-        #             raise ValueError(f"Invalid strftime template: '{default}'")
+        elif field == "today.month":
-        #     else:
+            value = DateTimeFormatter(self.today).month
-        #         return None
+        elif field == "today.mon":
-
+            value = DateTimeFormatter(self.today).mon
-        if field == "today.date":
+        elif field == "today.dd":
-            return DateTimeFormatter(self.today).date
+            value = DateTimeFormatter(self.today).dd
-
+        elif field == "today.dow":
-        if field == "today.year":
+            value = DateTimeFormatter(self.today).dow
-            return DateTimeFormatter(self.today).year
+        elif field == "today.doy":
-
+            value = DateTimeFormatter(self.today).doy
-        if field == "today.yy":
+        elif field == "today.hour":
-            return DateTimeFormatter(self.today).yy
+            value = DateTimeFormatter(self.today).hour
-
+        elif field == "today.min":
-        if field == "today.mm":
+            value = DateTimeFormatter(self.today).min
-            return DateTimeFormatter(self.today).mm
+        elif field == "today.sec":
-
+            value = DateTimeFormatter(self.today).sec
-        if field == "today.month":
+        elif field == "today.strftime":
-            return DateTimeFormatter(self.today).month
-
-        if field == "today.mon":
-            return DateTimeFormatter(self.today).mon
-
-        if field == "today.dd":
-            return DateTimeFormatter(self.today).dd
-
-        if field == "today.dow":
-            return DateTimeFormatter(self.today).dow
-
-        if field == "today.doy":
-            return DateTimeFormatter(self.today).doy
-
-        if field == "today.hour":
-            return DateTimeFormatter(self.today).hour
-
-        if field == "today.min":
-            return DateTimeFormatter(self.today).min
-
-        if field == "today.sec":
-            return DateTimeFormatter(self.today).sec
-
-        if field == "today.strftime":
             if default:
                 try:
-                    return self.today.strftime(default)
+                    value = self.today.strftime(default)
                 except:
                     raise ValueError(f"Invalid strftime template: '{default}'")
             else:
-                return None
+                value = None
-
+        elif field == "place.name":
-        if field == "place.name":
+            value = self.photo.place.name if self.photo.place else None
-            return self.photo.place.name if self.photo.place else None
+        elif field == "place.country_code":
-
+            value = self.photo.place.country_code if self.photo.place else None
-        if field == "place.country_code":
+        elif field == "place.name.country":
-            return self.photo.place.country_code if self.photo.place else None
+            value = (
-
-        if field == "place.name.country":
-            return (
                 self.photo.place.names.country[0]
                 if self.photo.place and self.photo.place.names.country
                 else None
             )
-
+        elif field == "place.name.state_province":
-        if field == "place.name.state_province":
+            value = (
-            return (
                 self.photo.place.names.state_province[0]
                 if self.photo.place and self.photo.place.names.state_province
                 else None
             )
-
+        elif field == "place.name.city":
-        if field == "place.name.city":
+            value = (
-            return (
                 self.photo.place.names.city[0]
                 if self.photo.place and self.photo.place.names.city
                 else None
             )
-
+        elif field == "place.name.area_of_interest":
-        if field == "place.name.area_of_interest":
+            value = (
-            return (
                 self.photo.place.names.area_of_interest[0]
                 if self.photo.place and self.photo.place.names.area_of_interest
                 else None
             )
-
+        elif field == "place.address":
-        if field == "place.address":
+            value = (
-            return (
                 self.photo.place.address_str
                 if self.photo.place and self.photo.place.address_str
                 else None
             )
-
+        elif field == "place.address.street":
-        if field == "place.address.street":
+            value = (
-            return (
                 self.photo.place.address.street
                 if self.photo.place and self.photo.place.address.street
                 else None
             )
-
+        elif field == "place.address.city":
-        if field == "place.address.city":
+            value = (
-            return (
                 self.photo.place.address.city
                 if self.photo.place and self.photo.place.address.city
                 else None
             )
-
+        elif field == "place.address.state_province":
-        if field == "place.address.state_province":
+            value = (
-            return (
                 self.photo.place.address.state_province
                 if self.photo.place and self.photo.place.address.state_province
                 else None
             )
-
+        elif field == "place.address.postal_code":
-        if field == "place.address.postal_code":
+            value = (
-            return (
                 self.photo.place.address.postal_code
                 if self.photo.place and self.photo.place.address.postal_code
                 else None
             )
-
+        elif field == "place.address.country":
-        if field == "place.address.country":
+            value = (
-            return (
                 self.photo.place.address.country
                 if self.photo.place and self.photo.place.address.country
                 else None
             )
-
+        elif field == "place.address.country_code":
-        if field == "place.address.country_code":
+            value = (
-            return (
                 self.photo.place.address.iso_country_code
                 if self.photo.place and self.photo.place.address.iso_country_code
                 else None
             )
+        else:
+            # if here, didn't get a match
+            raise ValueError(f"Unhandled template value: {field}")
 
-        # if here, didn't get a match
+        if filename:
-        raise ValueError(f"Unhandled template value: {field}")
+            value = sanitize_pathpart(value, replacement=replacement)
+        elif dirname:
+            value = sanitize_dirname(value, replacement=replacement)
+        return value
 
-    def get_template_value_multi(self, field, path_sep):
+    def get_template_value_multi(
+        self, field, path_sep, filename=False, dirname=False, replacement=":"
+    ):
         """lookup value for template field (multi-value template substitutions)
 
         Args:
             field: template field to find value for.
             path_sep: path separator to use for folder_album field
+            dirname: if True, values will be sanitized to be valid directory names; default = False
         
         Returns:
             List of the matching template values or [None].
@@ -621,9 +600,6 @@ class PhotoTemplate:
         """ return list of values for a multi-valued template field """
         if field == "album":
             values = self.photo.albums
-            values = [
-                value.replace("/", ":") for value in values
-            ]  # TODO: temp fix for issue #213
         elif field == "keyword":
             values = self.photo.keywords
         elif field == "person":
@@ -640,17 +616,42 @@ class PhotoTemplate:
             for album in self.photo.album_info:
                 if album.folder_names:
                     # album in folder
-                    folder = path_sep.join(album.folder_names)
+                    if dirname:
-                    folder += path_sep + album.title.replace(
+                        # being used as a filepath so sanitize each part
-                        "/", ":"
+                        folder = path_sep.join(
-                    )  # TODO: temp fix for issue #213
+                            sanitize_dirname(f, replacement=replacement)
+                            for f in album.folder_names
+                        )
+                        folder += path_sep + sanitize_dirname(
+                            album.title, replacement=replacement
+                        )
+                    else:
+                        folder = path_sep.join(album.folder_names)
+                        folder += path_sep + album.title
                     values.append(folder)
                 else:
                     # album not in folder
-                    values.append(album.title.replace("/", ":"))
+                    if dirname:
+                        values.append(
+                            sanitize_dirname(album.title, replacement=replacement)
+                        )
+                    else:
+                        values.append(album.title)
         else:
-            raise ValueError(f"Unhandleded template value: {field}")
+            raise ValueError(f"Unhandled template value: {field}")
+
+        # sanitize directory names if needed, folder_album handled differently above
+        if filename:
+            values = [
+                sanitize_pathpart(value, replacement=replacement) for value in values
+            ]
+        elif dirname and field != "folder_album":
+            # skip folder_album because it would have been handled above
+            values = [
+                sanitize_dirname(value, replacement=replacement) for value in values
+            ]
 
         # If no values, insert None so code below will substite none_str for None
         values = values or [None]
         return values
+

tests/Test-10.15.6.photoslibrary/database/Photos.sqlite.lock

@@ -7,7 +7,7 @@
 	<key>hostuuid</key>
 	<string>9575E48B-8D5F-5654-ABAC-4431B1167324</string>
 	<key>pid</key>
-	<integer>2964</integer>
+	<integer>40469</integer>
 	<key>processname</key>
 	<string>photolibraryd</string>
 	<key>uid</key>

tests/Test-10.15.6.photoslibrary/private/com.apple.photoanalysisd/caches/graph/PhotoAnalysisServicePreferences.plist

@@ -3,24 +3,24 @@
 <plist version="1.0">
 <dict>
 	<key>BackgroundHighlightCollection</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>BackgroundHighlightEnrichment</key>
-	<date>2020-06-24T04:02:12Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>BackgroundJobAssetRevGeocode</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>BackgroundJobSearch</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>BackgroundPeopleSuggestion</key>
-	<date>2020-06-24T04:02:12Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>BackgroundUserBehaviorProcessor</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundGraphConsistencyUpdateJobDateKey</key>
-	<date>2020-05-30T02:16:06Z</date>
+	<date>2020-10-17T23:45:33Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundGraphRebuildJobDate</key>
-	<date>2020-05-29T04:31:37Z</date>
+	<date>2020-10-17T23:45:24Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundMemoryGenerationJobDate</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:26Z</date>
 	<key>SiriPortraitDonation</key>
-	<date>2020-06-24T04:02:13Z</date>
+	<date>2020-10-17T23:45:25Z</date>
 </dict>
 </plist>

tests/Test-10.15.6.photoslibrary/private/com.apple.photoanalysisd/caches/vision/PersonPromoter

@@ -3,7 +3,7 @@
 <plist version="1.0">
 <dict>
 	<key>NumberOfFacesProcessedOnLastRun</key>
-	<integer>7</integer>
+	<integer>11</integer>
 	<key>ProcessedInQuiescentState</key>
 	<true/>
 	<key>SuggestedMeIdentifier</key>

tests/Test-10.15.6.photoslibrary/private/com.apple.photoanalysisd/caches/vision/PhotoAnalysisServicePreferences.plist

@@ -3,8 +3,8 @@
 <plist version="1.0">
 <dict>
 	<key>FaceIDModelLastGenerationKey</key>
-	<date>2020-05-29T03:44:04Z</date>
+	<date>2020-10-17T23:45:32Z</date>
 	<key>LastContactClassificationKey</key>
-	<date>2020-05-29T04:31:40Z</date>
+	<date>2020-10-17T23:45:54Z</date>
 </dict>
 </plist>

tests/test_catalina_10_15_6.py

@@ -24,6 +24,7 @@ KEYWORDS = [
     "St. James's Park",
     "UK",
     "United Kingdom",
+    "foo/bar",
 ]
 # Photos 5 includes blank person for detected face
 PERSONS = ["Katie", "Suzy", "Maria", _UNKNOWN_PERSON]
@@ -47,6 +48,7 @@ KEYWORDS_DICT = {
     "St. James's Park": 1,
     "UK": 1,
     "United Kingdom": 1,
+    "foo/bar": 1,
 }
 PERSONS_DICT = {"Katie": 3, "Suzy": 2, "Maria": 2, _UNKNOWN_PERSON: 1}
 ALBUM_DICT = {

tests/test_cli.py

@@ -229,8 +229,23 @@ CLI_EXPORTED_FILENAME_TEMPLATE_FILENAMES_PATHSEP = [
     "2019-10:11 Paris Clermont/IMG_4547.jpg",
 ]
 
+
+CLI_EXPORTED_FILENAME_TEMPLATE_FILENAMES_KEYWORD_PATHSEP = [
+    "foo:bar/foo:bar_IMG_3092.heic"
+]
+
+CLI_EXPORTED_FILENAME_TEMPLATE_LONG_DESCRIPTION = [
+    "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. "
+    "Aenean commodo ligula eget dolor. Aenean massa. "
+    "Cum sociis natoque penatibus et magnis dis parturient montes, "
+    "nascetur ridiculus mus. Donec quam felis, ultricies nec, "
+    "pellentesque eu, pretium q.tif"
+]
+
 CLI_EXPORT_UUID = "D79B8D77-BFFC-460B-9312-034F2877D35B"
 CLI_EXPORT_UUID_STATUE = "3DD2C897-F19E-4CA6-8C22-B027D5A71907"
+CLI_EXPORT_UUID_KEYWORD_PATHSEP = "7783E8E6-9CAC-40F3-BE22-81FB7051C266"
+CLI_EXPORT_UUID_LONG_DESCRIPTION = "8846E3E6-8AC8-4857-8448-E3D025784410"
 
 CLI_EXPORT_UUID_FILENAME = "Pumkins2.jpg"
 
@@ -568,10 +583,7 @@ def test_query_uuid_from_file_1():
 
     # build list of uuids we got from the output JSON
     json_got = json.loads(result.output)
-    uuid_got = []
+    uuid_got = [photo["uuid"] for photo in json_got]
-    for photo in json_got:
-        uuid_got.append(photo["uuid"])
-
     assert sorted(UUID_EXPECTED_FROM_FILE) == sorted(uuid_got)
 
 
@@ -601,10 +613,7 @@ def test_query_uuid_from_file_2():
 
     # build list of uuids we got from the output JSON
     json_got = json.loads(result.output)
-    uuid_got = []
+    uuid_got = [photo["uuid"] for photo in json_got]
-    for photo in json_got:
-        uuid_got.append(photo["uuid"])
-
     uuid_expected = UUID_EXPECTED_FROM_FILE.copy()
     uuid_expected.append(UUID_NOT_FROM_FILE)
     assert sorted(uuid_expected) == sorted(uuid_got)
@@ -1965,13 +1974,12 @@ def test_export_filename_template_2():
         assert sorted(files) == sorted(CLI_EXPORTED_FILENAME_TEMPLATE_FILENAMES2)
 
 
-def test_export_filename_template_pathsep_in_name():
+def test_export_filename_template_pathsep_in_name_1():
     """ export photos using filename template with folder_album and "/" in album name """
     import locale
     import os
     import os.path
     import pathlib
-    import osxphotos
     from osxphotos.__main__ import export
 
     locale.setlocale(locale.LC_ALL, "en_US")
@@ -1998,6 +2006,71 @@ def test_export_filename_template_pathsep_in_name():
             assert pathlib.Path(fname).is_file()
 
 
+def test_export_filename_template_pathsep_in_name_2():
+    """ export photos using filename template with keyword and "/" in keyword """
+    import locale
+    import os
+    import os.path
+    import pathlib
+    from osxphotos.__main__ import export
+
+    locale.setlocale(locale.LC_ALL, "en_US")
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_6),
+                ".",
+                "-V",
+                "--directory",
+                "{keyword}",
+                "--filename",
+                "{keyword}_{original_name}",
+                "--uuid",
+                CLI_EXPORT_UUID_KEYWORD_PATHSEP,
+            ],
+        )
+        assert result.exit_code == 0
+        for fname in CLI_EXPORTED_FILENAME_TEMPLATE_FILENAMES_KEYWORD_PATHSEP:
+            assert pathlib.Path(fname).is_file()
+
+
+def test_export_filename_template_long_description():
+    """ export photos using filename template with description that exceeds max length """
+    import locale
+    import os
+    import os.path
+    import pathlib
+    import osxphotos
+    from osxphotos.__main__ import export
+
+    locale.setlocale(locale.LC_ALL, "en_US")
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_6),
+                ".",
+                "-V",
+                "--filename",
+                "{descr}",
+                "--uuid",
+                CLI_EXPORT_UUID_LONG_DESCRIPTION,
+            ],
+        )
+        assert result.exit_code == 0
+        for fname in CLI_EXPORTED_FILENAME_TEMPLATE_LONG_DESCRIPTION:
+            assert pathlib.Path(fname).is_file()
+
+
 def test_export_filename_template_3():
     """ test --filename with invalid template """
     import glob
@@ -2489,9 +2562,8 @@ def test_export_sidecar_keyword_template():
         "EXIF:ModifyDate": "2020:04:11 12:34:16"}]"""
         )[0]
 
-        json_file = open("Pumkins2.jpg.json", "r")
+        with open("Pumkins2.jpg.json", "r") as json_file:
-        json_got = json.load(json_file)[0]
+            json_got = json.load(json_file)[0]
-        json_file.close()
 
         # some gymnastics to account for different sort order in different pythons
         for k, v in json_got.items():

tests/test_path_utils.py

@@ -0,0 +1,117 @@
+""" Test path_utils.py """
+
+def test_sanitize_filename():
+    from osxphotos.path_utils import sanitize_filename
+    from osxphotos._constants import MAX_FILENAME_LEN
+
+    # basic sanitize
+    filenames = {
+        "Foobar.txt": "Foobar.txt",
+        "Foo:bar.txt": "Foo:bar.txt",
+        "Foo/bar.txt": "Foo:bar.txt",
+        "Foo//.txt": "Foo::.txt",
+    }
+    for filename, sanitized in filenames.items():
+        filename = sanitize_filename(filename)
+        assert filename == sanitized
+
+    # sanitize with replacement
+    filenames = {
+        "Foobar.txt": "Foobar.txt",
+        "Foo:bar.txt": "Foo:bar.txt",
+        "Foo/bar.txt": "Foo_bar.txt",
+        "Foo//.txt": "Foo__.txt",
+    }
+    for filename, sanitized in filenames.items():
+        filename = sanitize_filename(filename, replacement="_")
+        assert filename == sanitized
+
+    # filename too long
+    filename = "foo" + "x" * 512
+    new_filename = sanitize_filename(filename)
+    assert len(new_filename) == MAX_FILENAME_LEN
+    assert new_filename == "foo" + "x" * 252
+
+    # filename too long with extension
+    filename = "x" * 512 + ".jpeg"
+    new_filename = sanitize_filename(filename)
+    assert len(new_filename) == MAX_FILENAME_LEN
+    assert new_filename == "x" * 250 + ".jpeg"
+
+    # more than one extension
+    filename = "foo.bar" + "x" * 255 + ".foo.bar.jpeg"
+    new_filename = sanitize_filename(filename)
+    assert len(new_filename) == MAX_FILENAME_LEN
+    assert new_filename == "foo.bar" + "x" * 243 + ".jpeg"
+
+    # shorter than drop count
+    filename = "foo." + "x" * 256
+    new_filename = sanitize_filename(filename)
+    assert len(new_filename) == MAX_FILENAME_LEN
+    assert new_filename == "foo." + "x" * 251
+
+
+def test_sanitize_dirname():
+    from osxphotos.path_utils import sanitize_dirname
+    from osxphotos._constants import MAX_DIRNAME_LEN
+
+    # basic sanitize
+    dirnames = {
+        "Foobar": "Foobar",
+        "Foo:bar": "Foo:bar",
+        "Foo/bar": "Foo:bar",
+        "Foo//": "Foo::",
+    }
+    for dirname, sanitized in dirnames.items():
+        dirname = sanitize_dirname(dirname)
+        assert dirname == sanitized
+
+    # sanitize with replacement
+    dirnames = {
+        "Foobar": "Foobar",
+        "Foo:bar": "Foo:bar",
+        "Foo/bar": "Foo_bar",
+        "Foo//": "Foo__",
+    }
+    for dirname, sanitized in dirnames.items():
+        dirname = sanitize_dirname(dirname, replacement="_")
+        assert dirname == sanitized
+
+    # dirname too long
+    dirname = "foo" + "x" * 512 + "bar"
+    new_dirname = sanitize_dirname(dirname)
+    assert len(new_dirname) == MAX_DIRNAME_LEN
+    assert new_dirname == "foo" + "x" * 252
+
+def test_sanitize_pathpart():
+    from osxphotos.path_utils import sanitize_pathpart
+    from osxphotos._constants import MAX_DIRNAME_LEN
+
+    # basic sanitize
+    dirnames = {
+        "Foobar": "Foobar",
+        "Foo:bar": "Foo:bar",
+        "Foo/bar": "Foo:bar",
+        "Foo//": "Foo::",
+    }
+    for dirname, sanitized in dirnames.items():
+        dirname = sanitize_pathpart(dirname)
+        assert dirname == sanitized
+
+    # sanitize with replacement
+    dirnames = {
+        "Foobar": "Foobar",
+        "Foo:bar": "Foo:bar",
+        "Foo/bar": "Foo_bar",
+        "Foo//": "Foo__",
+    }
+    for dirname, sanitized in dirnames.items():
+        dirname = sanitize_pathpart(dirname, replacement="_")
+        assert dirname == sanitized
+
+    # dirname too long
+    dirname = "foo" + "x" * 512 + "bar"
+    new_dirname = sanitize_pathpart(dirname)
+    assert len(new_dirname) == MAX_DIRNAME_LEN
+    assert new_dirname == "foo" + "x" * 252
+