I’ve recently added this function to a patch I’ve submitted to Drupal 8. It’s meant to be used with the assert function
public static function validCaller($scope = NULL) {
$trace = debug_backtrace();
// This function must always be called from assertion.
if (!$trace[1]['function'] === 'assert') {
throw new FaultException('mustAssert');
}
// First level is this function, second the assert. Drop.
array_shift($trace);
array_shift($trace);
// Get this level.
$origin = array_shift($trace);
// To find the true origin of the call we have to traverse any
// override functions by the children. We'll know when we've hit the caller
// because the function name will change.
do {
$frame = array_shift($trace);
} while ($frame['function'] === $origin['function']);
// Resolve the namespace of the origin.
$namespace = substr($origin['class'], 0, strrpos($origin['class'], '\\'));
// Be a little forgiving of devs who start the scope with a \
if ($scope && strpos($scope, '\\') === 0) {
$scope = substr($scope, 1);
}
// Now do the check.
return strpos($frame['class'], $scope === NULL ? $namespace : $scope) === 0 ||
strpos($frame['class'] . '\\' . $frame['function'], $scope === NULL ? $namespace : $scope) === 0;
}
What this does in a nutshell is allows the programmer to assert that a function he has written is being called only from the specified scope, or the same namespace if no scope is specified. You can be as specific as requiring the method only be called from one other method (or an override).
I’m seeking opinions on this check. I think it’s a good idea because it prevents misuse of an API. For example, a common pattern is to have element classes of some sort be used by a collection class, such as a database and statments in the case of PDO (which does this sort of limiting at the code level incidentally). This code removes the need to test what the element classes will do if used by someone other than the class intended to use them.
Since the function traverses the call stack to take overrides into account it doesn’t prohibit extension of either element or collection. (I want to say child/parent but that term is used for inheritance within a single class, and isn’t correct here).
This basically mimics Java’s protected keyword which limits visibility to the current package. (Imho a much better use of protected than PHPs nerfed implementation) so yes I would say it’s a good idea. Java also allows Private/Protected classes for similar reasons.
I can see that such a function would be handy under some circumstances, but to be honest it feels like kind of a coupling smell; if you need to make sure that a certain method is only called from a certain namespace, that method is too tightly coupled to something else in that namespace and/or the method is in a class that’s exposing too much of its internals to the outside world (or else why would you prevent said outside world from calling it?).
If you only create classes that do one thing and one thing well, with a well-defined public interface in theory any other class should be able to use it. Whether it’s useful for other classes to use it is a different story altogether.
Also, I would be vary wary if an beginner level developer got his hands on this function and started littering all their methods with it …
Both protected (java style package level) methods and private classes are incredibly useful when writing libraries you want others to use. As the library author it means you can expose the exact API that you want. The advantage of this is it makes meaning restructuring the internals of the library can be guaranteed to not break BC. Changing the constructor arguments of a private class? or even removing the class from the package entirely? It doesn’t matter. I can guarantee that nobodies code will break when they upgrade to the new version because there was no way they could have instantiated it… if it was public… well it’s a bit of a gamble.
The same is true of protected methods (java style, not php). As the library author you are free to change the API of the method and guarantee not to break anyone else’s code when they upgrade the library because you can be sure they don’t have any code calling the method.
Basically it helps prevent BC issues by only exposing a minimal API.
Yes, I agree that that is useful, and it would have been nice to have something like that in PHP. However, since we don’t, we need to do something else, and I’m not convinced Michael’s function is the way to go.
I’ve seen more that one package where the constuctor takes a $config array parameter. Which I suppose is their way of preventing BC; if you ever add more options just make sure they have sensible defaults and all should be fine. To me this solves the “protected within the namespace” in an acceptable manner.
That, combined with as little mutator methods as possible, should give you a good start to narrowing down the public API of a class.
Fair point. I think my only issue with it is that it won’t actually make the guarantee it tries to. Presumably in production the assertions are turned off and this check never happens, loosening the API and forfeiting the benefit. Other than that and the obvious performance consideration of traversing the stack trace I can’t see any major downsides to this approach.
That said, Implementation wise I’d like to see this made into a trait rather than a static method as it makes the class’ dependency on the assertion method more explicit and you’ll more easily be able to work out what namespace the method making the assertion is in because you’ll have a $this reference.
I’d certainly agree with @TomB that this would certainly be better as a trait than as a static method call.
I’d have to say though that the whole idea of an object having to check the backtrace to see who’s calling it (and whether they’re allowed to) makes me feel… uncomfortable to say the least.
I’m not familiar with Drupal’s code layout but couldn’t an errant developer put a class in the permitted namespace and invoke the supposedly protected method anyway?
ie.
namespace Rascal\Cheeseit;
class MyThing
{
public function trickster()
{
$naughty = new \Protected\Package\Cheeky();
$naughty->fakeIt();
}
}
// same file [ugh!] or via require() ...[also ugh!]
namespace \Protected\Package;
class Cheeky
{
public function fakeIt()
{
$protected = new ProtectedThing();
return $protected->protectedMethod();
}
}
To be honest, I can’t imagine how you’d ‘protect’ a method to achieve what Java does other than provide an overall God Object (anti-pattern!) for the package, which acts as a composite for the package’s objects and as the factory – package classes require God Object instance on their constructor. So called “protected/secured” methods then require the God Object instance as an additional parameter, which is subsequently checked against the God Object instance received on the constructor in the first place. Finally, God Object presents the ‘authorised’ package API to the rest of the application.
disclaimer: I’m tired and this last bit seems proper nasty Still doesn’t provide any real protection either!
Yeah, a programmer could do such a thing - and the core code team would be under no obligation whatsoever to insure their code works when upgrades happen. At the end of the day there is nothing no code base or programming language can do in the face of brazen stupidity.
I personally don’t waste my time idiot proofing code to that degree.
The trait would be bound at run time at a performance cost, so no.
As to the assertion being bound to a singleton - Drupal already has about 50 or so such singletons. While they are moving away from this pattern, it isn’t there quite yet.
Also, over obsession with “I can’t define this here, do it elsewhere - no not here elsewhere - abstract, abstract, abstract…” leads to piece of crap codebases like Magento where the devs have effectively created a poor programming language in XML out of the config files as a result of their abstraction obsession.
At some point a binding decision will be made in code. That’s a reality. This Assertion singleton does create a dependency on the drupal code, but if you’re writing a drupal module how is that a problem? Complaining about it is as asinine as complaining about the file having a dependency on PHP itself.
Still doesn’t provide any real protection either!