rfctr: make PackageReader a value object

Use @lazyproperty in place of pre-computed instance variables.

Author: scanny

pptx/compat/__init__.py

@@ -7,9 +7,11 @@
 import collections
 
 try:
+    Container = collections.abc.Container
     Mapping = collections.abc.Mapping
     Sequence = collections.abc.Sequence
 except AttributeError:
+    Container = collections.Container
     Mapping = collections.Mapping
     Sequence = collections.Sequence
 

pptx/opc/package.py

@@ -184,7 +184,7 @@ def _load(self):
     @lazyproperty
     def _package_reader(self):
         """|PackageReader| object providing access to package-items in pkg_file."""
-        return PackageReader.from_file(self._pkg_file)
+        return PackageReader(self._pkg_file)
 
     @lazyproperty
     def _parts(self):

pptx/opc/serialized.py

@@ -5,39 +5,29 @@
 import os
 import zipfile
 
-from pptx.compat import is_string
+from pptx.compat import Container, is_string
 from pptx.exceptions import PackageNotFoundError
 from pptx.opc.constants import CONTENT_TYPE as CT, RELATIONSHIP_TARGET_MODE as RTM
 from pptx.opc.oxml import CT_Types, parse_xml, serialize_part_xml
 from pptx.opc.packuri import CONTENT_TYPES_URI, PACKAGE_URI, PackURI
 from pptx.opc.shared import CaseInsensitiveDict
 from pptx.opc.spec import default_content_types
+from pptx.util import lazyproperty
 
 
-class PackageReader(object):
-    """
-    Provides access to the contents of a zip-format OPC package via its
-    :attr:`serialized_parts` and :attr:`pkg_srels` attributes.
+class PackageReader(Container):
+    """Provides access to package-parts of an OPC package with dict semantics.
+
+    The package may be in zip-format (a .pptx file) or expanded into a directory
+    structure, perhaps by unzipping a .pptx file.
     """
 
-    def __init__(self, content_types, pkg_srels, sparts):
-        super(PackageReader, self).__init__()
-        self._pkg_srels = pkg_srels
-        self._sparts = sparts
+    def __init__(self, pkg_file):
+        self._pkg_file = pkg_file
 
-    @staticmethod
-    def from_file(pkg_file):
-        """
-        Return a |PackageReader| instance loaded with contents of *pkg_file*.
-        """
-        phys_reader = _PhysPkgReader(pkg_file)
-        content_types = _ContentTypeMap.from_xml(phys_reader.content_types_xml)
-        pkg_srels = PackageReader._srels_for(phys_reader, PACKAGE_URI)
-        sparts = PackageReader._load_serialized_parts(
-            phys_reader, pkg_srels, content_types
-        )
-        phys_reader.close()
-        return PackageReader(content_types, pkg_srels, sparts)
+    def __contains__(self, pack_uri):
+        """Return True when part identified by `pack_uri` is present in package."""
+        return pack_uri in self._blob_reader
 
     def iter_sparts(self):
         """
@@ -71,23 +61,40 @@ def rels_xml_for(self, pkg_file, partname):
         phys_reader.close()
         return rels_xml
 
-    @staticmethod
-    def _load_serialized_parts(phys_reader, pkg_srels, content_types):
-        """
+    @lazyproperty
+    def _content_types(self):
+        """Filty temporary hack during refactoring."""
+        phys_reader = _PhysPkgReader(self._pkg_file)
+        return _ContentTypeMap.from_xml(phys_reader.content_types_xml)
+        phys_reader.close()
+
+    @lazyproperty
+    def _pkg_srels(self):
+        """Filty temporary hack during refactoring."""
+        phys_reader = _PhysPkgReader(self._pkg_file)
+        pkg_srels = self._srels_for(phys_reader, PACKAGE_URI)
+        phys_reader.close()
+        return pkg_srels
+
+    @lazyproperty
+    def _sparts(self):
+        """Filty temporary hack during refactoring.
+
         Return a list of |_SerializedPart| instances corresponding to the
         parts in *phys_reader* accessible by walking the relationship graph
-        starting with *pkg_srels*.
+        starting with `pkg_srels`.
         """
+        phys_reader = _PhysPkgReader(self._pkg_file)
         sparts = []
-        part_walker = PackageReader._walk_phys_parts(phys_reader, pkg_srels)
+        part_walker = self._walk_phys_parts(phys_reader, self._pkg_srels)
         for partname, blob, srels in part_walker:
-            content_type = content_types[partname]
+            content_type = self._content_types[partname]
             spart = _SerializedPart(partname, content_type, blob, srels)
             sparts.append(spart)
+        phys_reader.close()
         return tuple(sparts)
 
-    @staticmethod
-    def _srels_for(phys_reader, source_uri):
+    def _srels_for(self, phys_reader, source_uri):
         """
         Return |_SerializedRelationshipCollection| instance populated with
         relationships for source identified by *source_uri*.
@@ -97,8 +104,7 @@ def _srels_for(phys_reader, source_uri):
             source_uri.baseURI, rels_xml
         )
 
-    @staticmethod
-    def _walk_phys_parts(phys_reader, srels, visited_partnames=None):
+    def _walk_phys_parts(self, phys_reader, srels, visited_partnames=None):
         """
         Generate a 3-tuple `(partname, blob, srels)` for each of the parts in
         *phys_reader* by walking the relationship graph rooted at srels.
@@ -112,10 +118,10 @@ def _walk_phys_parts(phys_reader, srels, visited_partnames=None):
             if partname in visited_partnames:
                 continue
             visited_partnames.append(partname)
-            part_srels = PackageReader._srels_for(phys_reader, partname)
+            part_srels = self._srels_for(phys_reader, partname)
             blob = phys_reader.blob_for(partname)
             yield (partname, blob, part_srels)
-            for partname, blob, srels in PackageReader._walk_phys_parts(
+            for partname, blob, srels in self._walk_phys_parts(
                 phys_reader, part_srels, visited_partnames
             ):
                 yield (partname, blob, srels)

tests/opc/test_serialized.py

@@ -40,7 +40,6 @@
     call,
     class_mock,
     function_mock,
-    initializer_mock,
     instance_mock,
     loose_mock,
     method_mock,
@@ -56,92 +55,6 @@
 class DescribePackageReader(object):
     """Unit-test suite for `pptx.opc.serialized.PackageReader` objects."""
 
-    def it_can_construct_from_pkg_file(
-        self, init, _PhysPkgReader_, from_xml, _srels_for, _load_serialized_parts
-    ):
-
-        phys_reader = _PhysPkgReader_.return_value
-        content_types = from_xml.return_value
-        pkg_srels = _srels_for.return_value
-        sparts = _load_serialized_parts.return_value
-        pkg_file = Mock(name="pkg_file")
-
-        pkg_reader = PackageReader.from_file(pkg_file)
-
-        _PhysPkgReader_.assert_called_once_with(pkg_file)
-        from_xml.assert_called_once_with(phys_reader.content_types_xml)
-        _srels_for.assert_called_once_with(phys_reader, "/")
-        _load_serialized_parts.assert_called_once_with(
-            phys_reader, pkg_srels, content_types
-        )
-        phys_reader.close.assert_called_once_with()
-        init.assert_called_once_with(pkg_reader, content_types, pkg_srels, sparts)
-        assert isinstance(pkg_reader, PackageReader)
-
-    def it_can_iterate_over_the_serialized_parts(self):
-        # mockery ----------------------
-        partname, content_type, blob = ("part/name.xml", "app/vnd.type", "<Part_1/>")
-        spart = Mock(
-            name="spart", partname=partname, content_type=content_type, blob=blob
-        )
-        pkg_reader = PackageReader(None, None, [spart])
-        iter_count = 0
-        # exercise ---------------------
-        for retval in pkg_reader.iter_sparts():
-            iter_count += 1
-        # verify -----------------------
-        assert retval == (partname, content_type, blob)
-        assert iter_count == 1
-
-    def it_can_iterate_over_all_the_srels(self):
-        # mockery ----------------------
-        pkg_srels = ["srel1", "srel2"]
-        sparts = [
-            Mock(name="spart1", partname="pn1", srels=["srel3", "srel4"]),
-            Mock(name="spart2", partname="pn2", srels=["srel5", "srel6"]),
-        ]
-        pkg_reader = PackageReader(None, pkg_srels, sparts)
-        # exercise ---------------------
-        generated_tuples = [t for t in pkg_reader.iter_srels()]
-        # verify -----------------------
-        expected_tuples = [
-            ("/", "srel1"),
-            ("/", "srel2"),
-            ("pn1", "srel3"),
-            ("pn1", "srel4"),
-            ("pn2", "srel5"),
-            ("pn2", "srel6"),
-        ]
-        assert generated_tuples == expected_tuples
-
-    def it_can_load_serialized_parts(self, _SerializedPart_, _walk_phys_parts):
-        # test data --------------------
-        test_data = (
-            ("/part/name1.xml", "app/vnd.type_1", "<Part_1/>", "srels_1"),
-            ("/part/name2.xml", "app/vnd.type_2", "<Part_2/>", "srels_2"),
-        )
-        iter_vals = [(t[0], t[2], t[3]) for t in test_data]
-        content_types = dict((t[0], t[1]) for t in test_data)
-        # mockery ----------------------
-        phys_reader = Mock(name="phys_reader")
-        pkg_srels = Mock(name="pkg_srels")
-        _walk_phys_parts.return_value = iter_vals
-        _SerializedPart_.side_effect = expected_sparts = (
-            Mock(name="spart_1"),
-            Mock(name="spart_2"),
-        )
-        # exercise ---------------------
-        retval = PackageReader._load_serialized_parts(
-            phys_reader, pkg_srels, content_types
-        )
-        # verify -----------------------
-        expected_calls = [
-            call("/part/name1.xml", "app/vnd.type_1", "<Part_1/>", "srels_1"),
-            call("/part/name2.xml", "app/vnd.type_2", "<Part_2/>", "srels_2"),
-        ]
-        assert _SerializedPart_.call_args_list == expected_calls
-        assert retval == expected_sparts
-
     def it_can_walk_phys_pkg_parts(self, _srels_for):
         # test data --------------------
         # +----------+       +--------+
@@ -175,7 +88,7 @@ def it_can_walk_phys_pkg_parts(self, _srels_for):
         phys_reader.blob_for.side_effect = [part_1_blob, part_2_blob, part_3_blob]
         # exercise ---------------------
         generated_tuples = [
-            t for t in PackageReader._walk_phys_parts(phys_reader, pkg_srels)
+            t for t in PackageReader(None)._walk_phys_parts(phys_reader, pkg_srels)
         ]
         # verify -----------------------
         expected_tuples = [
@@ -195,36 +108,14 @@ def it_can_retrieve_srels_for_a_source_uri(
         load_from_xml = _SerializedRelationshipCollection_.load_from_xml
         srels = load_from_xml.return_value
         # exercise ---------------------
-        retval = PackageReader._srels_for(phys_reader, source_uri)
+        retval = PackageReader(None)._srels_for(phys_reader, source_uri)
         # verify -----------------------
         phys_reader.rels_xml_for.assert_called_once_with(source_uri)
         load_from_xml.assert_called_once_with(source_uri.baseURI, rels_xml)
         assert retval == srels
 
     # fixture components -----------------------------------
 
-    @pytest.fixture
-    def from_xml(self, request):
-        return method_mock(request, _ContentTypeMap, "from_xml")
-
-    @pytest.fixture
-    def init(self, request):
-        return initializer_mock(request, PackageReader)
-
-    @pytest.fixture
-    def _load_serialized_parts(self, request):
-        return method_mock(request, PackageReader, "_load_serialized_parts")
-
-    @pytest.fixture
-    def _PhysPkgReader_(self, request):
-        _patch = patch("pptx.opc.serialized._PhysPkgReader", spec_set=_ZipPkgReader)
-        request.addfinalizer(_patch.stop)
-        return _patch.start()
-
-    @pytest.fixture
-    def _SerializedPart_(self, request):
-        return class_mock(request, "pptx.opc.serialized._SerializedPart")
-
     @pytest.fixture
     def _SerializedRelationshipCollection_(self, request):
         return class_mock(
@@ -235,10 +126,6 @@ def _SerializedRelationshipCollection_(self, request):
     def _srels_for(self, request):
         return method_mock(request, PackageReader, "_srels_for")
 
-    @pytest.fixture
-    def _walk_phys_parts(self, request):
-        return method_mock(request, PackageReader, "_walk_phys_parts")
-
 
 class Describe_ContentTypeMap(object):
     def it_can_construct_from_ct_item_xml(self, from_xml_fixture):