diff --git a/WebFiori/Framework/Access.php b/WebFiori/Framework/Access.php index 8570b4c81..10f97d725 100644 --- a/WebFiori/Framework/Access.php +++ b/WebFiori/Framework/Access.php @@ -741,7 +741,7 @@ private function validateId($id): bool { $valid = true; for ($x = 0 ; $x < $len ; $x++) { - $valid = $valid && $id[$x] != ';' && $id[$x] != ' ' && $id[$x] != '-' && $id[$x] != ','; + $valid = $valid && $id[$x] != ';' && $id[$x] != ' ' && $id[$x] != ','; } return $valid; diff --git a/WebFiori/Framework/Middleware/MiddlewareRegistry.php b/WebFiori/Framework/Middleware/MiddlewareRegistry.php index 67e59882e..29cedea39 100644 --- a/WebFiori/Framework/Middleware/MiddlewareRegistry.php +++ b/WebFiori/Framework/Middleware/MiddlewareRegistry.php @@ -104,6 +104,22 @@ public function remove(string $name): void { public function reset(): void { $this->middlewareList = []; } + /** + * Finds a registered middleware by its class name. + * + * @param string $className The fully qualified class name. + * + * @return AbstractMiddleware|null The first registered middleware that is an instance of the given class, or null. + */ + public function findByClass(string $className): ?AbstractMiddleware { + foreach ($this->middlewareList as $mw) { + if ($mw instanceof $className) { + return $mw; + } + } + + return null; + } /** * Returns all registered middleware. * diff --git a/WebFiori/Framework/Privilege.php b/WebFiori/Framework/Privilege.php index 696bb159b..1db89bf29 100644 --- a/WebFiori/Framework/Privilege.php +++ b/WebFiori/Framework/Privilege.php @@ -96,7 +96,7 @@ public function setID(string $code): bool { for ($x = 0 ; $x < $len ; $x++) { $ch = $xid[$x]; - if (!($ch == '_' || ($ch >= 'a' && $ch <= 'z') || ($ch >= 'A' && $ch <= 'Z') || ($ch >= '0' && $ch <= '9'))) { + if (!($ch == '_' || $ch == '.' || $ch == '-' || ($ch >= 'a' && $ch <= 'z') || ($ch >= 'A' && $ch <= 'Z') || ($ch >= '0' && $ch <= '9'))) { return false; } } diff --git a/WebFiori/Framework/PrivilegesGroup.php b/WebFiori/Framework/PrivilegesGroup.php index 38b92b851..b8c416b8c 100644 --- a/WebFiori/Framework/PrivilegesGroup.php +++ b/WebFiori/Framework/PrivilegesGroup.php @@ -216,7 +216,7 @@ public static function isValidID(string $id) : bool { for ($x = 0 ; $x < $len ; $x++) { $ch = $xid[$x]; - if (!($ch == '_' || ($ch >= 'a' && $ch <= 'z') || ($ch >= 'A' && $ch <= 'Z') || ($ch >= '0' && $ch <= '9'))) { + if (!($ch == '_' || $ch == '.' || $ch == '-' || ($ch >= 'a' && $ch <= 'z') || ($ch >= 'A' && $ch <= 'Z') || ($ch >= '0' && $ch <= '9'))) { return false; } } diff --git a/WebFiori/Framework/Router/RouterUri.php b/WebFiori/Framework/Router/RouterUri.php index 9eb5ac02d..a3bc47ead 100644 --- a/WebFiori/Framework/Router/RouterUri.php +++ b/WebFiori/Framework/Router/RouterUri.php @@ -645,14 +645,16 @@ private function resolveDependencies(array $middlewareList): array { $current = array_shift($queue); foreach ($current->getDependencies() as $depName) { - if (isset($byName[$depName])) { + $resolvedName = $this->resolveDepToName($depName, $byName); + + if ($resolvedName !== null && isset($byName[$resolvedName])) { continue; } - $dep = MiddlewareManager::getMiddleware($depName); + $dep = $this->lookupMiddleware($depName); - if ($dep !== null) { - $byName[$depName] = $dep; + if ($dep !== null && !isset($byName[$dep->getName()])) { + $byName[$dep->getName()] = $dep; $queue[] = $dep; } } @@ -701,11 +703,13 @@ private function sortByDependencies(array $middlewareList): array { } foreach ($mw->getDependencies() as $dep) { - if (isset($byName[$dep])) { - $graph[$dep][] = $name; + $resolvedDep = $this->resolveDepToName($dep, $byName); + + if ($resolvedDep !== null && isset($byName[$resolvedDep])) { + $graph[$resolvedDep][] = $name; - if (!isset($inDegree[$dep])) { - $inDegree[$dep] = 0; + if (!isset($inDegree[$resolvedDep])) { + $inDegree[$resolvedDep] = 0; } $inDegree[$name]++; } @@ -757,4 +761,57 @@ private function sortByDependencies(array $middlewareList): array { return $sorted; } + /** + * Resolves a dependency identifier to a middleware name. + * + * If the identifier is already a known name in the map, returns it directly. + * If it looks like a class name (class_exists), finds the registered middleware + * that is an instance of that class and returns its name. + * + * @param string $dep The dependency identifier (name or class). + * @param array $byName Map of name => AbstractMiddleware. + * + * @return string|null The resolved middleware name, or null if unresolvable. + */ + private function resolveDepToName(string $dep, array $byName): ?string { + if (isset($byName[$dep])) { + return $dep; + } + + if (class_exists($dep)) { + foreach ($byName as $mw) { + if ($mw instanceof $dep) { + return $mw->getName(); + } + } + + $mw = MiddlewareManager::getInstance()->findByClass($dep); + + if ($mw !== null) { + return $mw->getName(); + } + } + + return null; + } + /** + * Looks up a middleware by name or class from the registry. + * + * @param string $dep The dependency identifier (name or class). + * + * @return AbstractMiddleware|null + */ + private function lookupMiddleware(string $dep): ?AbstractMiddleware { + $mw = MiddlewareManager::getMiddleware($dep); + + if ($mw !== null) { + return $mw; + } + + if (class_exists($dep)) { + return MiddlewareManager::getInstance()->findByClass($dep); + } + + return null; + } } diff --git a/WebFiori/Framework/Router/ServiceRouter.php b/WebFiori/Framework/Router/ServiceRouter.php index 0c5caac19..7c03ac23d 100644 --- a/WebFiori/Framework/Router/ServiceRouter.php +++ b/WebFiori/Framework/Router/ServiceRouter.php @@ -222,7 +222,9 @@ private static function scanNamespace(string $namespace, string $dir, string $pa if (!empty($attrs)) { $attr = $attrs[0]->newInstance(); - if (!empty($attr->name)) { + if (!empty($attr->path)) { + $name = $attr->path; + } else if (!empty($attr->name)) { if (str_contains($attr->name, '/')) { continue; // Invalid: name must not contain slashes } diff --git a/tests/ServiceRouterFixtures/AuthLoginService.php b/tests/ServiceRouterFixtures/AuthLoginService.php new file mode 100644 index 000000000..f158105ab --- /dev/null +++ b/tests/ServiceRouterFixtures/AuthLoginService.php @@ -0,0 +1,15 @@ +assertTrue(Access::hasGroup('USERS_Group')); $this->assertEquals(0, count(Access::privileges())); $this->assertFalse(Access::newPrivilege('not_exist', 'new_pr')); - $this->assertFalse(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new-pr')); + $this->assertTrue(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new-pr')); $this->assertFalse(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new,pr')); $this->assertFalse(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new;pr')); $this->assertFalse(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new pr')); $this->assertTrue(Access::newPrivilege('USERS_MANAGEMENT_GROUP', 'new_pr')); $this->assertTrue(Access::newPrivilege('USERS_Group', 'new_pr_2')); - $this->assertEquals(2, count(Access::privileges())); + $this->assertEquals(3, count(Access::privileges())); $this->assertEquals(1, count(Access::privileges('USERS_Group'))); - $this->assertEquals(1, count(Access::privileges('USERS_MANAGEMENT_GROUP'))); + $this->assertEquals(2, count(Access::privileges('USERS_MANAGEMENT_GROUP'))); $this->assertEquals(0, count(Access::privileges('ADMINS_Group'))); $this->assertTrue(Access::hasPrivilege('new_pr')); $this->assertFalse(Access::hasPrivilege('new_pr','USERS_Group')); diff --git a/tests/WebFiori/Framework/Tests/Middleware/MiddlewareClassSyntaxDepsTest.php b/tests/WebFiori/Framework/Tests/Middleware/MiddlewareClassSyntaxDepsTest.php new file mode 100644 index 000000000..ed589d01c --- /dev/null +++ b/tests/WebFiori/Framework/Tests/Middleware/MiddlewareClassSyntaxDepsTest.php @@ -0,0 +1,228 @@ +addMiddleware('audit-log'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + $this->assertContains('start-session', $names); + $this->assertContains('audit-log', $names); + $this->assertCount(2, $middleware); + } + + /** + * @test + * When using ::class syntax, the dependency must be executed before + * the dependent middleware. + */ + public function testClassSyntaxDependencyOrder() { + MiddlewareManager::register(new SessionMw()); + MiddlewareManager::register(new AuditLogMw()); + + $uri = new RouterUri('https://example.com/class-order', ''); + $uri->addMiddleware('audit-log'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + // SessionMw must come before AuditLogMw + $this->assertLessThan( + array_search('audit-log', $names), + array_search('start-session', $names) + ); + } + + /** + * @test + * A middleware can mix ::class and string name dependencies. + * Both should be resolved correctly. + */ + public function testMixedClassAndStringDependencies() { + MiddlewareManager::register(new SessionMw()); + MiddlewareManager::register(new AuditLogMw()); + MiddlewareManager::register(new MixedDepsMw()); + + $uri = new RouterUri('https://example.com/mixed', ''); + $uri->addMiddleware('mixed-deps'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + $this->assertContains('start-session', $names); + $this->assertContains('audit-log', $names); + $this->assertContains('mixed-deps', $names); + $this->assertCount(3, $middleware); + } + + /** + * @test + * Execution order with mixed deps: session before audit-log before mixed-deps. + */ + public function testMixedDependenciesOrder() { + MiddlewareManager::register(new SessionMw()); + MiddlewareManager::register(new AuditLogMw()); + MiddlewareManager::register(new MixedDepsMw()); + + $uri = new RouterUri('https://example.com/mixed-order', ''); + $uri->addMiddleware('mixed-deps'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + // session before audit-log + $this->assertLessThan( + array_search('audit-log', $names), + array_search('start-session', $names) + ); + // audit-log before mixed-deps + $this->assertLessThan( + array_search('mixed-deps', $names), + array_search('audit-log', $names) + ); + } + + /** + * @test + * If a ::class dependency references a class not registered in the manager, + * it should be skipped silently (same as unresolvable string names). + */ + public function testUnregisteredClassDependencySkipped() { + MiddlewareManager::register(new UnregisteredClassDepMw()); + + $uri = new RouterUri('https://example.com/unregistered', ''); + $uri->addMiddleware('unregistered-class-dep'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + $this->assertContains('unregistered-class-dep', $names); + $this->assertCount(1, $middleware); + } + + /** + * @test + * No duplicates when the dependency resolved via ::class is already + * explicitly assigned to the route. + */ + public function testNoDuplicateWhenClassDepAlreadyAssigned() { + MiddlewareManager::register(new SessionMw()); + MiddlewareManager::register(new AuditLogMw()); + + $uri = new RouterUri('https://example.com/no-dup', ''); + $uri->addMiddleware('start-session'); + $uri->addMiddleware('audit-log'); + + $middleware = $uri->getMiddleware(); + $names = array_map(fn ($mw) => $mw->getName(), $middleware); + + $this->assertCount(2, $middleware); + $this->assertEquals(1, count(array_keys($names, 'start-session'))); + } +} diff --git a/tests/WebFiori/Framework/Tests/PrivilegeTest.php b/tests/WebFiori/Framework/Tests/PrivilegeTest.php index 957fcf060..8ca8335e0 100644 --- a/tests/WebFiori/Framework/Tests/PrivilegeTest.php +++ b/tests/WebFiori/Framework/Tests/PrivilegeTest.php @@ -52,4 +52,74 @@ public function testToJson00() { $j->setPropsStyle('camel'); $this->assertEquals('{"privilegeId":"Valid_ID_55","name":"Valid Name"}',$j.''); } + /** + * @test + * Tests that a permission ID with dots is accepted and stored correctly. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testSetIDWithDots() { + $pr = new Privilege('orders.create', 'Create Orders'); + $this->assertEquals('orders.create', $pr->getID()); + $this->assertEquals('Create Orders', $pr->getName()); + } + /** + * @test + * Tests that a permission ID with dashes is accepted and stored correctly. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testSetIDWithDashes() { + $pr = new Privilege('orders-create', 'Create Orders'); + $this->assertEquals('orders-create', $pr->getID()); + $this->assertEquals('Create Orders', $pr->getName()); + } + /** + * @test + * Tests that a permission ID with dots and dashes combined is accepted. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testSetIDWithDotsAndDashes() { + $pr = new Privilege('app.orders-create', 'Create Orders'); + $this->assertEquals('app.orders-create', $pr->getID()); + $this->assertEquals('Create Orders', $pr->getName()); + } + /** + * @test + * Tests that multiple permissions with dotted IDs are distinguishable + * and don't collapse to the same default ID. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testMultipleDottedIDsAreUnique() { + $pr1 = new Privilege('orders.create', 'Create Orders'); + $pr2 = new Privilege('orders.view', 'View Orders'); + $pr3 = new Privilege('orders.cancel', 'Cancel Orders'); + + $this->assertEquals('orders.create', $pr1->getID()); + $this->assertEquals('orders.view', $pr2->getID()); + $this->assertEquals('orders.cancel', $pr3->getID()); + + // Ensure they are all different + $this->assertNotEquals($pr1->getID(), $pr2->getID()); + $this->assertNotEquals($pr2->getID(), $pr3->getID()); + $this->assertNotEquals($pr1->getID(), $pr3->getID()); + } + /** + * @test + * Tests that setID() with dots returns true indicating success. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testSetIDWithDotsReturnsTrue() { + $pr = new Privilege(); + $this->assertTrue($pr->setID('orders.create')); + $this->assertEquals('orders.create', $pr->getID()); + } + /** + * @test + * Tests that setID() with dashes returns true indicating success. + * @see https://github.com/webfiori/framework/issues/404 + */ + public function testSetIDWithDashesReturnsTrue() { + $pr = new Privilege(); + $this->assertTrue($pr->setID('orders-create')); + $this->assertEquals('orders-create', $pr->getID()); + } } diff --git a/tests/WebFiori/Framework/Tests/Router/ServiceRouterTest.php b/tests/WebFiori/Framework/Tests/Router/ServiceRouterTest.php index 173ab293d..cb860faa0 100644 --- a/tests/WebFiori/Framework/Tests/Router/ServiceRouterTest.php +++ b/tests/WebFiori/Framework/Tests/Router/ServiceRouterTest.php @@ -84,7 +84,7 @@ public function testDiscoverSkipsNonServiceClasses() { public function testDiscoverReturnsCount() { $count = ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); - $this->assertEquals(4, $count); // orders, product, legacy, users + $this->assertEquals(6, $count); // orders, product, legacy, users, auth/login, v2/users/profile } /** @test */ @@ -150,15 +150,17 @@ public function testDiscoverNonRecursiveSkipsSubdirectories() { /** @test */ public function testDiscoverSkipsAttributeNameWithSlash() { - // OrderService has #[RestController('orders')] — valid - // If we had one with slash it would be skipped + // Services using 'name' property cannot have slashes. + // Services using 'path' property CAN have slashes. ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); $discovered = ServiceRouter::getDiscovered(); - // All discovered names should not contain slashes (non-recursive) - foreach ($discovered as $name => $entry) { - $this->assertStringNotContainsString('/', $name); - } + // OrderService uses name='orders' — no slash + $this->assertArrayHasKey('orders', $discovered); + $this->assertStringNotContainsString('/', 'orders'); + + // AuthLoginService uses path='auth/login' — slashes allowed via path + $this->assertArrayHasKey('auth/login', $discovered); } /** @test */ @@ -175,4 +177,66 @@ public function testHandleReturns404ForUnknownService() { ServiceRouter::handle('nonexistent', $this->namespace, $this->fixturesDir); $this->assertEquals(404, $response->getCode()); } + + /** + * @test + * Tests that #[RestController(path: 'auth/login')] uses the path property + * as the route key instead of the name. + * @see https://github.com/webfiori/framework/issues/398 + */ + public function testDiscoverUsesPathPropertyForRouting() { + ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); + $discovered = ServiceRouter::getDiscovered(); + + $this->assertArrayHasKey('auth/login', $discovered); + $this->assertEquals('/apis/auth/login', $discovered['auth/login']['path']); + $this->assertEquals('WebFiori\\Tests\\ServiceRouterFixtures\\AuthLoginService', $discovered['auth/login']['class']); + $this->assertEquals('service', $discovered['auth/login']['type']); + } + + /** + * @test + * Tests that path property takes priority over name property. + * @see https://github.com/webfiori/framework/issues/398 + */ + public function testPathPropertyTakesPriorityOverName() { + ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); + $discovered = ServiceRouter::getDiscovered(); + + // AuthLoginService has name='login' and path='auth/login' + // It should be keyed by path, not name + $this->assertArrayHasKey('auth/login', $discovered); + $this->assertArrayNotHasKey('login', $discovered); + } + + /** + * @test + * Tests that a multi-segment path property works without a name. + * @see https://github.com/webfiori/framework/issues/398 + */ + public function testMultiSegmentPathWithoutName() { + ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); + $discovered = ServiceRouter::getDiscovered(); + + $this->assertArrayHasKey('v2/users/profile', $discovered); + $this->assertEquals('/apis/v2/users/profile', $discovered['v2/users/profile']['path']); + $this->assertEquals('WebFiori\\Tests\\ServiceRouterFixtures\\UserProfileService', $discovered['v2/users/profile']['class']); + } + + /** + * @test + * Tests that services without path property still work as before (fallback to name or derived). + * @see https://github.com/webfiori/framework/issues/398 + */ + public function testNoPathPropertyFallsBackToExistingBehavior() { + ServiceRouter::discover($this->namespace, '/apis', [], $this->fixturesDir); + $discovered = ServiceRouter::getDiscovered(); + + // OrderService has name='orders', no path + $this->assertArrayHasKey('orders', $discovered); + $this->assertEquals('/apis/orders', $discovered['orders']['path']); + + // ProductService has no name, no path — derived from class name + $this->assertArrayHasKey('product', $discovered); + } }