From d27776ee17a64b157cee6b728399a9e6e217b1cd Mon Sep 17 00:00:00 2001
From: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:04:02 -0800
Subject: [PATCH] Package json fix (#1261)

* Filter incompatible version formats

* Initial testing

* Clean up tests

* Enable test for py38+

* Fix linter error

* Update newrelic/common/package_version_utils.py

Co-authored-by: Timothy Pansino <11214426+TimPansino@users.noreply.github.com>

* Reviewer suggestions

* Make tests more comprehensive

* Add type check to tests

---------

Co-authored-by: Timothy Pansino <11214426+TimPansino@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 newrelic/common/package_version_utils.py      | 12 ++++---
 .../test_package_version_utils.py             | 32 +++++++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py
index 9ab213ddd..278e51368 100644
--- a/newrelic/common/package_version_utils.py
+++ b/newrelic/common/package_version_utils.py
@@ -14,10 +14,8 @@
 
 import sys
 import warnings
-
 from functools import lru_cache
 
-
 # Need to account for 4 possible variations of version declaration specified in (rejected) PEP 396
 VERSION_ATTRS = ("__version__", "version", "__version_tuple__", "version_tuple")  # nosec
 NULL_VERSIONS = frozenset((None, "", "0", "0.0", "0.0.0", "0.0.0.0", (0,), (0, 0), (0, 0, 0), (0, 0, 0, 0)))  # nosec
@@ -81,9 +79,13 @@ def _get_package_version(name):
             try:
                 version = getattr(module, attr, None)
 
-                # In certain cases like importlib_metadata.version, version is a callable
-                # function.
-                if callable(version):
+                # Some frameworks (such as `pypdfium2`) may use a class
+                # property to define the version.  Because class properties
+                # are not callable we need to check if the result is
+                # anything other than a string, tuple, or list.  If so,
+                # we need to skip this method of version retrieval and use
+                # `pkg_resources` or `importlib.metadata`.
+                if version and not isinstance(version, (str, tuple, list)):
                     continue
 
                 # Cast any version specified as a list into a tuple.
diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py
index 8b2abeda8..2641234b2 100644
--- a/tests/agent_unittests/test_package_version_utils.py
+++ b/tests/agent_unittests/test_package_version_utils.py
@@ -150,3 +150,35 @@ def test_version_caching(monkeypatch):
     del sys.modules["mymodule"]
     version = get_package_version("mymodule")
     assert version not in NULL_VERSIONS, version
+
+
+def test_version_as_class_property(monkeypatch):
+    # There is no file/module here, so we monkeypatch
+    # pytest instead for our purposes
+    class FakeModule:
+        @property
+        def version(self):
+            return "1.2.3"
+
+    monkeypatch.setattr(pytest, "version", FakeModule.version, raising=False)
+
+    version = get_package_version("pytest")
+    assert version not in NULL_VERSIONS and isinstance(version, str), version
+
+
+# This test checks to see if the version is a property of the class
+# but also checks to see if the something like version.version_tuple
+# has been exported as a tuple.
+def test_version_as_class_property_and_version_tuple(monkeypatch):
+    # There is no file/module here, so we monkeypatch
+    # pytest instead for our purposes
+    class FakeModule:
+        @property
+        def version(self):
+            return "1.2.3"
+
+    monkeypatch.setattr(pytest, "version", FakeModule.version, raising=False)
+    monkeypatch.setattr(pytest, "version_tuple", (1, 2, 3), raising=False)
+
+    version = get_package_version("pytest")
+    assert version not in NULL_VERSIONS and isinstance(version, str), version