Merge pull request #140 from ConductionNL/hotfix/directory-and-patch

Hotfixes for Directory and PUT as PATCH

lib/Controller/DirectoryController.php

@@ -6,6 +6,7 @@
 use OCA\OpenCatalogi\Db\ListingMapper;
 use OCA\OpenCatalogi\Service\DirectoryService;
 use OCA\OpenCatalogi\Service\ObjectService;
+use OCA\OpenCatalogi\Exception\DirectoryUrlException;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
@@ -75,25 +76,17 @@ public function update(): JSONResponse
 	{
 		// Get the URL from the request parameters
 		$url = $this->request->getParam('directory');
-
-		// http://directory.opencatalogi.nl
-		// Check if the URL parameter is provided
-		if (empty($url) === true) {
-			return new JSONResponse(['error' => 'directory parameter is required'], 400);
-		}
-
-		// Check if URL contains 'local' and throw exception if it does
-		if (str_contains(strtolower($url), 'local')) {
-			return new JSONResponse(['error' => 'Local URLs are not allowed'], 400);
-		}
-
-		// Validate the URL
-		if (!filter_var($url, FILTER_VALIDATE_URL)) {
-			return new JSONResponse(['error' => 'Invalid URL provided'], 400);
-		}	
 
 		// Sync the external directory with the provided URL
-		$data = $this->directoryService->syncExternalDirectory($url);
+		try {
+			$data = $this->directoryService->syncExternalDirectory($url);
+		} catch (DirectoryUrlException $exception) {
+			if($exception->getMessage() === 'URL is required') {
+				$exception->setMessage('Property "directory" is required');
+			}
+
+			return new JSONResponse(data: ['message' => $exception->getMessage()], statusCode: 400);
+		}
 
 		// Return JSON response with the sync result
 		return new JSONResponse($data);

lib/Controller/ListingsController.php

@@ -6,6 +6,7 @@
 use OCA\OpenCatalogi\Db\ListingMapper;
 use OCA\OpenCatalogi\Service\ObjectService;
 use OCA\OpenCatalogi\Service\DirectoryService;
+use OCA\OpenCatalogi\Exception\DirectoryUrlException;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
@@ -186,13 +187,16 @@ public function add(): JSONResponse
         // Get the URL parameter from the request
         $url = $this->request->getParam('url');
 
-		// Check if the URL parameter is provided
-		if (empty($url) === true) {
-			return new JSONResponse(['error' => 'URL parameter is required'], 400);
-		}
-
         // Add the new listing using the provided URL
-        $result = $this->directoryService->syncExternalDirectory($url);
+		try {
+			$result = $this->directoryService->syncExternalDirectory($url);
+		} catch (DirectoryUrlException $exception) {
+			if($exception->getMessage() === 'URL is required') {
+				$exception->setMessage('Property "url" is required');
+			}
+
+			return new JSONResponse(data: ['message' => $exception->getMessage()], statusCode: 400);
+		}
 
         // Return the result as a JSON response
         return new JSONResponse(['success' => $result]);

lib/Db/AttachmentMapper.php

@@ -126,7 +126,7 @@ public function createFromArray(array $object): Attachment
 	 * @throws DoesNotExistException If the entity is not found
 	 * @throws MultipleObjectsReturnedException|\OCP\DB\Exception If multiple entities are found
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): Attachment
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): Attachment
 	{
 		$attachment = $this->find($id);
 		// Fallback to create if the attachment does not exist

lib/Db/CatalogMapper.php

@@ -140,9 +140,9 @@ public function createFromArray(array $object): Catalog
 	 *
 	 * @return Catalog The updated Catalog entity
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): Catalog
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): Catalog
 	{
-		$catalog = $this->find($id);		
+		$catalog = $this->find($id);
 		// Fallback to create if the catalog does not exist
 		if ($catalog === null) {
 			$object['uuid'] = $id;

lib/Db/ListingMapper.php

@@ -234,7 +234,7 @@ public function createFromArray(array $object): Listing
 	 * @return Listing The updated Listing entity
 	 * @throws Exception
 	 */
-	public function updateFromArray(int|string $id, array $object, bool $updateVersion = true): Listing
+	public function updateFromArray(int|string $id, array $object, bool $updateVersion = true, bool $patch = false): Listing
 	{
 		$listing = $this->find($id);
 		// Fallback to create if the listing does not exist

lib/Db/OrganizationMapper.php

@@ -135,7 +135,7 @@ public function createFromArray(array $object): Organization
 	 *
 	 * @return Organization The updated Organization entity
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): Organization
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): Organization
 	{
 		$organization = $this->find($id);
 		// Fallback to create if the organization does not exist

lib/Db/PublicationMapper.php

@@ -224,7 +224,7 @@ public function createFromArray(array $object): Publication
 	 *
 	 * @return Publication The updated Publication entity
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): Publication
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): Publication
 	{
 		$publication = $this->find(id: $id);
 		// Fallback to create if the publication does not exist

lib/Db/PublicationTypeMapper.php

@@ -155,7 +155,7 @@ public function createFromArray(array $object): PublicationType
 	 * @throws DoesNotExistException If the entity is not found
 	 * @throws MultipleObjectsReturnedException|\OCP\DB\Exception If multiple entities are found
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): PublicationType
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): PublicationType
 	{
 		$publicationType = $this->find($id);
 		// Fallback to create if the publication type does not exist

lib/Db/ThemeMapper.php

@@ -141,7 +141,7 @@ public function createFromArray(array $object): Theme
 	 * @throws DoesNotExistException If the entity is not found
 	 * @throws MultipleObjectsReturnedException|\OCP\DB\Exception If multiple entities are found
 	 */
-	public function updateFromArray(int $id, array $object, bool $updateVersion = true): Theme
+	public function updateFromArray(int $id, array $object, bool $updateVersion = true, bool $patch = false): Theme
 	{
 		$theme = $this->find($id);
 		// Fallback to create if the theme does not exist

lib/Exception/DirectoryUrlException.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace OCA\OpenCatalogi\Exception;
+
+use Exception;
+
+class DirectoryUrlException extends Exception
+{
+	public function setMessage(string $message): void
+	{
+		$this->message = $message;
+	}
+
+}

lib/Service/DirectoryService.php

@@ -4,18 +4,24 @@
 
 use DateTime;
 use GuzzleHttp\Client;
+use GuzzleHttp\Exception\ClientException;
 use GuzzleHttp\Exception\GuzzleException;
+use GuzzleHttp\Exception\RequestException;
+use GuzzleHttp\Exception\ServerException;
 use OCA\OpenCatalogi\Db\Catalog;
 use OCA\OpenCatalogi\Db\CatalogMapper;
 use OCA\OpenCatalogi\Db\Listing;
 use OCA\OpenCatalogi\Db\ListingMapper;
 use OCA\OpenCatalogi\Service\BroadcastService;
+use OCA\OpenCatalogi\Exception\DirectoryUrlException;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
+use OCP\AppFramework\Http\JSONResponse;
 use OCP\IAppConfig;
 use OCP\IURLGenerator;
 use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\NotFoundExceptionInterface;
+use Symfony\Component\Routing\Generator\UrlGenerator;
 use Symfony\Component\Uid\Uuid;
 
 /**
@@ -40,15 +46,15 @@ class DirectoryService
 	 * @param ObjectService $objectService Object service for handling objects
 	 * @param CatalogMapper $catalogMapper Mapper for catalog objects
 	 * @param ListingMapper $listingMapper Mapper for listing objects
-	 * @param BroadcastService $broadcastService Broadcast service for broadcasting	
+	 * @param BroadcastService $broadcastService Broadcast service for broadcasting
 	 */
 	public function __construct(
 		private readonly IURLGenerator $urlGenerator,
 		private readonly IAppConfig $config,
 		private readonly ObjectService $objectService,
 		private readonly CatalogMapper $catalogMapper,
 		private readonly ListingMapper $listingMapper,
-		private readonly BroadcastService $broadcastService
+		private readonly BroadcastService $broadcastService,
 	)
 	{
 		$this->client = new Client([]);
@@ -228,16 +234,20 @@ public function doCronSync(): array {
 		// Extract unique directory URLs
 		// Get unique directories from listings
 		$uniqueDirectories = array_unique(array_column($listings, 'directory'));
-		
+
 		// Add default OpenCatalogi directory if not already present
 		$defaultDirectory = 'https://directory.opencatalogi.nl/apps/opencatalogi/api/directory';
 		if (!in_array($defaultDirectory, $uniqueDirectories)) {
 			$uniqueDirectories[] = $defaultDirectory;
-		}		
+		}
 
 		// Sync each unique directory
 		foreach ($uniqueDirectories as $directoryUrl) {
-			$result = $this->syncExternalDirectory($directoryUrl);
+			try {
+				$result = $this->syncExternalDirectory($directoryUrl);
+			} catch (DirectoryUrlException $exception) {
+				continue;
+			}
 			$results = array_merge_recursive($results, $result);
 		}
 
@@ -269,7 +279,7 @@ public function validateExternalListing(array $listing): bool
 	 * @return array The updated listing
 	 * @throws DoesNotExistException|MultipleObjectsReturnedException|ContainerExceptionInterface|NotFoundExceptionInterface
 	 */
-	public function updateListing(array $newListing, array $oldListing): array{		
+	public function updateListing(array $newListing, array $oldListing): array{
 		// Let's see if these changed by checking them against the hash
 		$newHash = hash('sha256', json_encode($newListing));
 		$oldHash = hash('sha256', json_encode($oldListing));
@@ -283,7 +293,36 @@ public function updateListing(array $newListing, array $oldListing): array{
 		return $newListing->jsonSerialize();
 	}
 
-
+	/**
+	 * Checks if the URL complies to basic rules.
+	 *
+	 * @param string $url The url to check.
+	 * @return void
+	 * @throws DirectoryUrlException Thrown if the url is invalid.
+	 */
+	private function checkConditions(string $url): void
+	{
+		if (empty($url) === true) {
+			throw new DirectoryUrlException('URL is required');
+		}
+
+		// Check if URL contains the base url of this instance.
+		if (str_contains(haystack: strtolower($url), needle: $this->urlGenerator->getBaseUrl()) === true) {
+			throw new DirectoryUrlException('Cannot load current directory');
+		}
+
+		// Check if URL contains 'local' and throw exception if it does
+		if (str_contains(strtolower($url), 'local') === true) {
+			throw new DirectoryUrlException('Local urls are not allowed');
+		}
+
+		// Validate the URL
+		if (empty(filter_var($url, FILTER_VALIDATE_URL)) === false) {
+			throw new DirectoryUrlException('Invalid URL provided');
+		}
+	}
+
+
 	/**
 	 * Synchronize with an external directory
 	 *
@@ -292,25 +331,32 @@ public function updateListing(array $newListing, array $oldListing): array{
 	 * @return array An array containing synchronization results
 	 * @throws DoesNotExistException|MultipleObjectsReturnedException|ContainerExceptionInterface|NotFoundExceptionInterface
 	 * @throws GuzzleException|\OCP\DB\Exception
+	 * @throws DirectoryUrlException
 	 */
 	public function syncExternalDirectory(string $url): array
 	{
 		// Log successful broadcast
 		\OC::$server->getLogger()->info('Synchronizing directory with ' . $url);
 
+		$this->checkConditions($url);
+
 		try {
+			$checkUrls[] = $url;
 			// Get the directory data
 			$result = $this->client->get($url);
 
 			// Fallback to the /api/directory endpoint if the result is not JSON
 			if (str_contains($result->getHeader('Content-Type')[0], 'application/json') === false) {
+
+				$checkUrls[] = $url.'/index.php/apps/opencatalogi/api/directory';
 				$url = rtrim($url, '/').'/apps/opencatalogi/api/directory';
 				$result = $this->client->get($url);
+				$checkUrls[] = $url;
 			}
-		} catch (\GuzzleHttp\Exception\ClientException $e) {
+		} catch (ClientException|RequestException|ServerException $e) {
 			// If we get a 404, the directory no longer exists
 			if ($e->getResponse()->getStatusCode() === 404) {
-				// Delete all listings for this directory since it no longer exists				
+				// Delete all listings for this directory since it no longer exists
 				$this->deleteListingsByDirectory('listing', $url);
 				throw new \Exception('Directory no longer exists at ' . $url);
 			}
@@ -324,10 +370,12 @@ public function syncExternalDirectory(string $url): array
 		$currentListings = $this->objectService->getObjects(
 			objectType: 'listing',
 			filters: [
-				'directory'=>$url,
+				'directory'=>$checkUrls,
 			]
 		);
 
+
+
 		// Remove any listings without a catalog ID from the database
 		foreach ($currentListings as $listing) {
 			if (empty($listing['catalog'])) {
@@ -342,7 +390,7 @@ public function syncExternalDirectory(string $url): array
 		// array_column() with null as second parameter returns complete array entries
 		// This will return complete listing objects indexed by their catalog ID
 		$oldListings = array_column(
-			$currentListings, 
+			$currentListings,
 			null, // null returns complete array entries rather than a specific column
 			'catalog' // Index by catalog ID
 		);
@@ -368,7 +416,7 @@ public function syncExternalDirectory(string $url): array
 			// If no existing listing found, prepare the new listing data
 			if ($oldListing === null) {
 				$listing['hash'] = hash('sha256', json_encode($listing));
-				$listing['catalog'] = $listing['id']; 
+				$listing['catalog'] = $listing['id'];
 				unset($listing['id']);
 			} else {
 				// Update existing listing
@@ -420,7 +468,7 @@ private function deleteListingsByDirectory(string $directoryUrl): void {
 			]
 		);
 		// Delete all listings
-		foreach ($currentListings as $listing) {	
+		foreach ($currentListings as $listing) {
 			$this->objectService->deleteObject('listing', $listing['id']);
 		}
 	}

lib/Service/ObjectService.php

@@ -302,7 +302,7 @@ public function saveObject(string $objectType, array $object, bool $updateVersio
 		$mapper = $this->getMapper($objectType);
 		// If the object has an id, update it; otherwise, create a new object
 		if (isset($object['id']) === true) {
-			return $mapper->updateFromArray($object['id'], $object, $updateVersion);
+			return $mapper->updateFromArray($object['id'], $object, $updateVersion, patch: true);
 		}
 		else {
 			return $mapper->createFromArray($object);