Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfixes for Directory and PUT as PATCH #140

Merged
merged 5 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions lib/Controller/DirectoryController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -75,25 +76,18 @@ 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');
}


rjzondervan marked this conversation as resolved.
Show resolved Hide resolved
return new JSONResponse(data: ['message' => $exception->getMessage()], statusCode: 400);
}

// Return JSON response with the sync result
return new JSONResponse($data);
Expand Down
16 changes: 10 additions & 6 deletions lib/Controller/ListingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/AttachmentMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/Db/CatalogMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/ListingMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/OrganizationMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/PublicationMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/PublicationTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/Db/ThemeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions lib/Exception/DirectoryUrlException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace OCA\OpenCatalogi\Exception;

use Exception;

class DirectoryUrlException extends Exception
{
public function setMessage(string $message): void
{
$this->message = $message;
}

}
74 changes: 61 additions & 13 deletions lib/Service/DirectoryService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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([]);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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));
Expand All @@ -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
*
Expand All @@ -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);
}
Expand All @@ -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'])) {
Expand All @@ -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
);
Expand All @@ -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
Expand Down Expand Up @@ -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']);
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/ObjectService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down