- 🇵🇱Poland sayco Toruń
First of all, I think the ticket needs to be updated with the information that the fromInternalUri is now part of the Url class, not the UrlHelper.
The other thing about this (before we apply any changes in the code) is that the implementation of the function is wrongly designed. It breaks the SRP, as it’s not only responsible for creating the internal URI, but also for validation against the external URI. I think this is the wrong approach.
I know it requires a lot of changes, but the fromInternalUri should be responsible for building the internal URI from whatever is given, or it should return a null value or raise an exception. Whatever is using the function should be responsible for handling the exception or null value, checking what the given value is, and taking appropriate actions depending on the outcome. It shouldn’t be the responsibility of the fromInternalUri method to know about other dependencies and all the use cases.
We also should avoid adding any silent caches in the code. At the very least, logging with debug level should be added. Otherwise, it’s hard to guess that anything unexpected happened in the code.