fix: Add compatibility with protobuf==5.x (#433)

* ci: add support for pre-release sessions * add compatibility with protobuf 5.x * fix RecursionError: maximum recursion depth exceeded while calling a Python object in tests * update required checks * update comments * update required checks * See protocolbuffers/protobuf#16596 breaking change * lint * Address review feedback * lint * add tests * Update tests/test_fields_mitigate_collision.py * add deprecation warning * Cater for protobuf 5.x+ * style * Filter deprecation warning * remove redundant code * revert * add comment * add comment * refactor code * style * update warning filter * address review comments * map_composite_types_str->map_composite_type_names * update comment * lint * add more test cases in test_json_default_values * add more test cases to tests/test_message.py * address review feedback * add comment * formatting * formatting * typo * Address review feedback * Update proto/marshal/marshal.py Co-authored-by: Victor Chudnovsky <[email protected]> * typo * address review feedback * address review feedback * fix test case * stye * add more test cases * add test case * Update tests/test_message.py Co-authored-by: Victor Chudnovsky <[email protected]> --------- Co-authored-by: Victor Chudnovsky <[email protected]>
googleapis · Jun 4, 2024 · 0f89372 · 0f89372
1 parent 0833e4c
commit 0f89372
Show file tree

Hide file tree

Showing 12 changed files with 447 additions and 58 deletions.
diff --git a/.github/sync-repo-settings.yaml b/.github/sync-repo-settings.yaml
@@ -7,24 +7,27 @@ branchProtectionRules:
   requiredStatusCheckContexts:
     - 'style-check'
     - 'docs'
-    - 'unit (3.6)'
     - 'unit (3.6, cpp)'
-    - 'unit (3.7)'
+    - 'unit (3.6, python)'
+    - 'unit (3.6, upb)'
     - 'unit (3.7, cpp)'
+    - 'unit (3.7, python)'
     - 'unit (3.7, upb)'
-    - 'unit (3.8)'
     - 'unit (3.8, cpp)'
+    - 'unit (3.8, python)'
     - 'unit (3.8, upb)'
-    - 'unit (3.9)'
     - 'unit (3.9, cpp)'
+    - 'unit (3.9, python)'
     - 'unit (3.9, upb)'
-    - 'unit (3.10)'
     - 'unit (3.10, cpp)'
+    - 'unit (3.10, python)'
     - 'unit (3.10, upb)'
-    - 'unit (3.11)'
+    - 'unit (3.11, python)'
     - 'unit (3.11, upb)'
-    - 'unit (3.12)'
+    - 'unit (3.12, python)'
     - 'unit (3.12, upb)'
+    - 'prerelease (3.12, python)'
+    - 'prerelease (3.12, upb)'
     - cover
     - OwlBot Post Processor
     - 'cla/google'

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -42,19 +42,27 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        python: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12']
-        variant: ['', 'cpp', 'upb']
+        python: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12', '3.13']
+        variant: ['cpp', 'python', 'upb']
+        # TODO(https://github.com/googleapis/proto-plus-python/issues/389):
+        # Remove the 'cpp' implementation once support for Protobuf 3.x is dropped.
+        # The 'cpp' implementation requires Protobuf == 3.x however version 3.x
+        # does not support Python 3.11 and newer. The 'cpp' implementation
+        # must be excluded from the test matrix for these runtimes.
         exclude:
           - variant: "cpp"
             python: 3.11
           - variant: "cpp"
             python: 3.12
+          - variant: "cpp"
+            python: 3.13
     steps:
       - uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.python }}
         uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python }}
+          allow-prereleases: true
       - name: Install nox
         run: |
           pip install nox
@@ -68,12 +76,38 @@ jobs:
         env:
           COVERAGE_FILE: .coverage-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }}
         run: |
-          nox -s unit${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }}
+          nox -s "unit-${{ env.PYTHON_VERSION_TRIMMED }}(implementation='${{ matrix.variant }}')"
       - name: Upload coverage results
         uses: actions/upload-artifact@v4
         with:
           name: coverage-artifact-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }}
           path: .coverage-${{ matrix.variant }}-${{ env.PYTHON_VERSION_TRIMMED }}
+  prerelease:
+    runs-on: ubuntu-20.04
+    strategy:
+      matrix:
+        python: ['3.12']
+        variant: ['python', 'upb']
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python ${{ matrix.python }}
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python }}
+          allow-prereleases: true
+      - name: Install nox
+        run: |
+          pip install nox
+      - name: Run unit tests
+        env:
+          COVERAGE_FILE: .coverage-prerelease-${{ matrix.variant }}
+        run: |
+          nox -s "prerelease_deps(implementation='${{ matrix.variant }}')"
+      - name: Upload coverage results
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-artifact-prerelease-${{ matrix.variant }}
+          path: .coverage-prerelease-${{ matrix.variant }}
   cover:
     runs-on: ubuntu-latest
     needs:

diff --git a/noxfile.py b/noxfile.py
@@ -13,10 +13,9 @@
 # limitations under the License.
 
 from __future__ import absolute_import
-import os
-import pathlib
 
 import nox
+import pathlib
 
 
 CURRENT_DIRECTORY = pathlib.Path(__file__).parent.absolute()
@@ -30,33 +29,38 @@
     "3.10",
     "3.11",
     "3.12",
+    "3.13",
 ]
 
 # Error if a python version is missing
 nox.options.error_on_missing_interpreters = True
 
 
 @nox.session(python=PYTHON_VERSIONS)
-def unit(session, proto="python"):
+@nox.parametrize("implementation", ["cpp", "upb", "python"])
+def unit(session, implementation):
     """Run the unit test suite."""
 
     constraints_path = str(
         CURRENT_DIRECTORY / "testing" / f"constraints-{session.python}.txt"
     )
 
-    session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = proto
+    session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = implementation
     session.install("coverage", "pytest", "pytest-cov", "pytz")
     session.install("-e", ".[testing]", "-c", constraints_path)
-    if proto == "cpp":  # 4.20 does not have cpp.
-        session.install("protobuf==3.19.0")
+    # TODO(https://github.com/googleapis/proto-plus-python/issues/389):
+    # Remove the 'cpp' implementation once support for Protobuf 3.x is dropped.
+    # The 'cpp' implementation requires Protobuf<4.
+    if implementation == "cpp":
+        session.install("protobuf<4")
 
     # TODO(https://github.com/googleapis/proto-plus-python/issues/403): re-enable `-W=error`
     # The warnings-as-errors flag `-W=error` was removed in
     # https://github.com/googleapis/proto-plus-python/pull/400.
     # It should be re-added once issue
-    # https://github.com/protocolbuffers/protobuf/issues/12186 is fixed.
+    # https://github.com/protocolbuffers/protobuf/issues/15077 is fixed.
     session.run(
-        "py.test",
+        "pytest",
         "--quiet",
         *(
             session.posargs  # Coverage info when running individual tests is annoying.
@@ -71,17 +75,59 @@ def unit(session, proto="python"):
     )
 
 
-# Check if protobuf has released wheels for new python versions
-# https://pypi.org/project/protobuf/#files
-# This list will generally be shorter than 'unit'
-@nox.session(python=["3.6", "3.7", "3.8", "3.9", "3.10"])
-def unitcpp(session):
-    return unit(session, proto="cpp")
+# Only test upb and python implementation backends.
+# As of protobuf 4.x, the "ccp" implementation is not available in the PyPI package as per
+# https://github.com/protocolbuffers/protobuf/tree/main/python#implementation-backends
+@nox.session(python=PYTHON_VERSIONS[-2])
+@nox.parametrize("implementation", ["python", "upb"])
+def prerelease_deps(session, implementation):
+    """Run the unit test suite against pre-release versions of dependencies."""
 
+    session.env["PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION"] = implementation
 
-@nox.session(python=PYTHON_VERSIONS)
-def unitupb(session):
-    return unit(session, proto="upb")
+    # Install test environment dependencies
+    session.install("coverage", "pytest", "pytest-cov", "pytz")
+
+    # Install the package without dependencies
+    session.install("-e", ".", "--no-deps")
+
+    prerel_deps = [
+        "google-api-core",
+        # dependency of google-api-core
+        "googleapis-common-protos",
+    ]
+
+    for dep in prerel_deps:
+        session.install("--pre", "--no-deps", "--upgrade", dep)
+
+    session.install("--pre", "--upgrade", "protobuf")
+    # Print out prerelease package versions
+    session.run(
+        "python", "-c", "import google.protobuf; print(google.protobuf.__version__)"
+    )
+    session.run(
+        "python", "-c", "import google.api_core; print(google.api_core.__version__)"
+    )
+
+    # TODO(https://github.com/googleapis/proto-plus-python/issues/403): re-enable `-W=error`
+    # The warnings-as-errors flag `-W=error` was removed in
+    # https://github.com/googleapis/proto-plus-python/pull/400.
+    # It should be re-added once issue
+    # https://github.com/protocolbuffers/protobuf/issues/15077 is fixed.
+    session.run(
+        "pytest",
+        "--quiet",
+        *(
+            session.posargs  # Coverage info when running individual tests is annoying.
+            or [
+                "--cov=proto",
+                "--cov-config=.coveragerc",
+                "--cov-report=term",
+                "--cov-report=html",
+                "tests",
+            ]
+        ),
+    )
 
 
 @nox.session(python="3.9")

diff --git a/proto/marshal/compat.py b/proto/marshal/compat.py
@@ -19,6 +19,9 @@
 # not be included.
 
 from google.protobuf.internal import containers
+import google.protobuf
+
+PROTOBUF_VERSION = google.protobuf.__version__
 
 # Import protobuf 4.xx first and fallback to earlier version
 # if not present.
@@ -37,14 +40,28 @@
 repeated_scalar_types = (containers.RepeatedScalarFieldContainer,)
 map_composite_types = (containers.MessageMap,)
 
+# In `proto/marshal.py`, for compatibility with protobuf 5.x,
+# we'll use `map_composite_type_names` to check whether
+# the name of the class of a protobuf type is
+# `MessageMapContainer`, and, if `True`, return a MapComposite.
+# See https://github.com/protocolbuffers/protobuf/issues/16596
+map_composite_type_names = ("MessageMapContainer",)
+
 if _message:
     repeated_composite_types += (_message.RepeatedCompositeContainer,)
     repeated_scalar_types += (_message.RepeatedScalarContainer,)
-    map_composite_types += (_message.MessageMapContainer,)
 
+    # In `proto/marshal.py`, for compatibility with protobuf 5.x,
+    # we'll use `map_composite_type_names` to check whether
+    # the name of the class of a protobuf type is
+    # `MessageMapContainer`, and, if `True`, return a MapComposite.
+    # See https://github.com/protocolbuffers/protobuf/issues/16596
+    if PROTOBUF_VERSION[0:2] in ["3.", "4."]:
+        map_composite_types += (_message.MessageMapContainer,)
 
 __all__ = (
     "repeated_composite_types",
     "repeated_scalar_types",
     "map_composite_types",
+    "map_composite_type_names",
 )
diff --git a/proto/marshal/marshal.py b/proto/marshal/marshal.py
@@ -188,7 +188,14 @@ def to_python(self, proto_type, value, *, absent: bool = None):
                 return Repeated(value, marshal=self)
 
         # Same thing for maps of messages.
-        if value_type in compat.map_composite_types:
+        # See https://github.com/protocolbuffers/protobuf/issues/16596
+        # We need to look up the name of the type in compat.map_composite_type_names
+        # as class `MessageMapContainer` is no longer exposed
+        # This is done to avoid taking a breaking change in proto-plus.
+        if (
+            value_type in compat.map_composite_types
+            or value_type.__name__ in compat.map_composite_type_names
+        ):
             return MapComposite(value, marshal=self)
         return self.get_rule(proto_type=proto_type).to_python(value, absent=absent)