From 797a9c3de5aae5bcdff5b635b7efa4d018fcd8f9 Mon Sep 17 00:00:00 2001
From: Michael Chisholm <chishm@chishm.com>
Date: Thu, 3 Mar 2022 02:54:47 +1100
Subject: [PATCH] Sort DMS results using only criteria supported by the device
 (#67475)

---
 homeassistant/components/dlna_dms/dms.py      | 23 +++++-
 .../dlna_dms/test_dms_device_source.py        | 77 ++++++++++++++++++-
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/homeassistant/components/dlna_dms/dms.py b/homeassistant/components/dlna_dms/dms.py
index 94ac67b0f1a..0fa4d77d005 100644
--- a/homeassistant/components/dlna_dms/dms.py
+++ b/homeassistant/components/dlna_dms/dms.py
@@ -547,7 +547,7 @@ class DmsDeviceSource:
         children = await self._device.async_browse_direct_children(
             object_id,
             metadata_filter=DLNA_BROWSE_FILTER,
-            sort_criteria=DLNA_SORT_CRITERIA,
+            sort_criteria=self._sort_criteria,
         )
 
         return self._didl_to_media_source(base_object, children)
@@ -680,6 +680,27 @@ class DmsDeviceSource:
         """Make an identifier for BrowseMediaSource."""
         return f"{self.source_id}/{action}{object_id}"
 
+    @property  # type: ignore
+    @functools.cache
+    def _sort_criteria(self) -> list[str]:
+        """Return criteria to be used for sorting results.
+
+        The device must be connected before reading this property.
+        """
+        assert self._device
+
+        if self._device.sort_capabilities == ["*"]:
+            return DLNA_SORT_CRITERIA
+
+        # Filter criteria based on what the device supports. Strings in
+        # DLNA_SORT_CRITERIA are prefixed with a sign, while those in
+        # the device's sort_capabilities are not.
+        return [
+            criterion
+            for criterion in DLNA_SORT_CRITERIA
+            if criterion[1:] in self._device.sort_capabilities
+        ]
+
 
 class Action(StrEnum):
     """Actions that can be specified in a DMS media-source identifier."""
diff --git a/tests/components/dlna_dms/test_dms_device_source.py b/tests/components/dlna_dms/test_dms_device_source.py
index 4ee9cce91ba..d6fcdb267d6 100644
--- a/tests/components/dlna_dms/test_dms_device_source.py
+++ b/tests/components/dlna_dms/test_dms_device_source.py
@@ -8,7 +8,7 @@ from async_upnp_client.profiles.dlna import ContentDirectoryErrorCode, DmsDevice
 from didl_lite import didl_lite
 import pytest
 
-from homeassistant.components.dlna_dms.const import DOMAIN
+from homeassistant.components.dlna_dms.const import DLNA_SORT_CRITERIA, DOMAIN
 from homeassistant.components.dlna_dms.dms import (
     ActionError,
     DeviceConnectionError,
@@ -686,6 +686,81 @@ async def test_browse_media_object(
         assert not child.children
 
 
+async def test_browse_object_sort_anything(
+    device_source_mock: DmsDeviceSource, dms_device_mock: Mock
+) -> None:
+    """Test sort criteria for children where device allows anything."""
+    dms_device_mock.sort_capabilities = ["*"]
+
+    object_id = "0"
+    dms_device_mock.async_browse_metadata.return_value = didl_lite.Container(
+        id="0", restricted="false", title="root"
+    )
+    dms_device_mock.async_browse_direct_children.return_value = DmsDevice.BrowseResult(
+        [], 0, 0, 0
+    )
+    await device_source_mock.async_browse_object("0")
+
+    # Sort criteria should be dlna_dms's default
+    dms_device_mock.async_browse_direct_children.assert_awaited_once_with(
+        object_id, metadata_filter=ANY, sort_criteria=DLNA_SORT_CRITERIA
+    )
+
+
+async def test_browse_object_sort_superset(
+    device_source_mock: DmsDeviceSource, dms_device_mock: Mock
+) -> None:
+    """Test sorting where device allows superset of integration's criteria."""
+    dms_device_mock.sort_capabilities = [
+        "dc:title",
+        "upnp:originalTrackNumber",
+        "upnp:class",
+        "upnp:artist",
+        "dc:creator",
+        "upnp:genre",
+    ]
+
+    object_id = "0"
+    dms_device_mock.async_browse_metadata.return_value = didl_lite.Container(
+        id="0", restricted="false", title="root"
+    )
+    dms_device_mock.async_browse_direct_children.return_value = DmsDevice.BrowseResult(
+        [], 0, 0, 0
+    )
+    await device_source_mock.async_browse_object("0")
+
+    # Sort criteria should be dlna_dms's default
+    dms_device_mock.async_browse_direct_children.assert_awaited_once_with(
+        object_id, metadata_filter=ANY, sort_criteria=DLNA_SORT_CRITERIA
+    )
+
+
+async def test_browse_object_sort_subset(
+    device_source_mock: DmsDeviceSource, dms_device_mock: Mock
+) -> None:
+    """Test sorting where device allows subset of integration's criteria."""
+    dms_device_mock.sort_capabilities = [
+        "dc:title",
+        "upnp:class",
+    ]
+
+    object_id = "0"
+    dms_device_mock.async_browse_metadata.return_value = didl_lite.Container(
+        id="0", restricted="false", title="root"
+    )
+    dms_device_mock.async_browse_direct_children.return_value = DmsDevice.BrowseResult(
+        [], 0, 0, 0
+    )
+    await device_source_mock.async_browse_object("0")
+
+    # Sort criteria should be reduced to only those allowed,
+    # and in the order specified by DLNA_SORT_CRITERIA
+    expected_criteria = ["+upnp:class", "+dc:title"]
+    dms_device_mock.async_browse_direct_children.assert_awaited_once_with(
+        object_id, metadata_filter=ANY, sort_criteria=expected_criteria
+    )
+
+
 async def test_browse_media_path(
     device_source_mock: DmsDeviceSource, dms_device_mock: Mock
 ) -> None: