music-assistant / hass-music-assistant

Turn your Home Assistant instance into a jukebox, hassle free streaming of your favorite media to Home Assistant media players.
Apache License 2.0
1.2k stars 44 forks source link

Recursive Universal player groups should not be allowed #2513

Open madbrain76 opened 1 week ago

madbrain76 commented 1 week ago

What version of Music Assistant has the issue?

2.0.7

What version of the Home Assistant Integration have you got installed?

2024.6.2

Have you tried everything in the Troubleshooting FAQ and reviewed the Open and Closed Issues and Discussions to resolve this yourself?

The problem

It's possible to create universal groups that contain each other, but shouldn't be

How to reproduce

  1. add the UGP provider
  2. add the DLNA provider
  3. in the settings / player list, click "Add group player"
  4. select "universal group player"
  5. name the group G1
  6. select the first DLNA player as member - backyard
  7. save
  8. create group G2
  9. select G1 as member
  10. click save
  11. click G1
  12. add G2 to the member list
  13. click G2
  14. observe that the member list is now empty
  15. add G1 to the member list
  16. click save
  17. click G1
  18. observe that the member list contains "backyard" and "G2"
  19. click G2
  20. observe that the member list contains "G1"
  21. groups G1 and G2 now have each other as members

Music Providers

File system (remote)

Player Providers

DLNA Universal group

Full log output

log.txt

Additional information

I wonder if it is intentional that a group can contain another group as member, or an oversight. Even if intentional, this particular case shouldn't be allowed.

This case probably won't happen accidentally too often - I had to try a bit forcefully - try twice (step 15) - to make it occur. And yes, the test case really needs to be this long. I don't believe I can reduce it.

What version of Home Assistant Core are your running

2024.6.2

What type of installation are you running?

Home Assistant OS

On what type of hardware are you running?

Windows

marcelveldt commented 1 week ago

An Universal Group may contain a sync group but not another UGP group so if that is somehow possible, that would be a bug

madbrain76 commented 1 week ago

@marcelveldt, Yes, it's currently allowed - very easy for you to verify. And if it's disallowed, then the recursive case also becomes impossible.