search

Art-of-WiFi / UniFi-API-client

A PHP API client class to interact with Ubiquiti's UniFi Controller API
MIT License
1.09k stars 217 forks source link

Login fails if no site is given since 1.1.84 #215

Closed pbksol closed 4 months ago

pbksol commented 4 months ago

I guess its not intended…

Up to version 1.1.84 it was possible to log into the controller without giving a site id. <1.1.84 accepted NULL as parameter. The site parameter seems mandatory now.

NULL will fail and an empty string will fail, too.

This has one quite important side effect: the login will always fail without already knowing the site id. This disables the feature to login in and check using list_sites which sites are accessible for the used login credentials.

pbksol commented 4 months ago

Fixed in 1.1.86

malle-pietje commented 4 months ago

Thanks for the PSA 😉 I hope people read it!

At some point in time we need to make the client class more strict, and therefore robust, and type hinting is one important instrument IMHO. Perhaps I went overboard with these two parameters in the constructor, especially when we have to maintain backwards with existing code.

pbksol commented 4 months ago

First, thanks for all the work. I normally try to avoid using external functions at all cost when it comes to doing API stuff. This one is one of the very, very rare occasions where I depart from the normal.

Regarding the breaking changes, I suggest bumping the version to – let's say 1.2 or something to make it more obvious that bigger things might have changed. A x.x.01 change doesn't shout "hey, look, my version changed, stuff might break", especially when in the past these little version changes never broke things and people tend to get used to it.

Again, thanks for your work :-)

malle-pietje commented 4 months ago

You’re welcome! Yes, totally makes sense since most devs will be requiring 1.1. Will probably bump to 2.0.0 with the next release when I plan on adding Exceptions and removing the trigger_error() calls.

I also have plans to release a separate API client package that either uses Guzzle directly or the Laravel HTTP client. That package will obviously require the use of composer due to the dependencies while the current package allows devs to simply copy and require the Client.php file.

malle-pietje commented 4 months ago

BTW, usually I try to follow semantic versioning as described here: https://devhints.io/semver