Skip to content

Commit

Permalink
v1.6.0 add debugging and fix an issue with files deletion
Browse files Browse the repository at this point in the history
  • Loading branch information
eldertek committed Jan 3, 2025
1 parent 1c6dc75 commit 526228d
Show file tree
Hide file tree
Showing 3 changed files with 285 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
- Filter by file name pattern with wildcard support (e.g., *.tmp, backup_*)
- User-specific filters management through settings
- Persistent filters stored in database
### Changed
- Enhanced logging throughout the application with detailed debug information
### Fixed
- Fix an issue where deleted files were still displayed in the duplicates list

## 1.5.3 - 2024-12-31
### Added
Expand Down
88 changes: 87 additions & 1 deletion lib/Service/FileDuplicateService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use OCP\AppFramework\Db\Entity;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\Files\NotFoundException;
use OCP\Lock\ILockingProvider;

use OCA\DuplicateFinder\AppInfo\Application;
use OCA\DuplicateFinder\Db\FileInfo;
Expand All @@ -28,17 +29,21 @@ class FileDuplicateService
private $originFolderService;
/** @var ?string */
private $currentUserId = null;
/** @var ILockingProvider */
private $lockingProvider;

public function __construct(
LoggerInterface $logger,
FileDuplicateMapper $mapper,
FileInfoService $fileInfoService,
OriginFolderService $originFolderService
OriginFolderService $originFolderService,
ILockingProvider $lockingProvider
) {
$this->mapper = $mapper;
$this->logger = $logger;
$this->fileInfoService = $fileInfoService;
$this->originFolderService = $originFolderService;
$this->lockingProvider = $lockingProvider;
}

public function setCurrentUserId(?string $userId): void {
Expand Down Expand Up @@ -256,12 +261,93 @@ public function getOrCreate(string $hash, string $type = 'file_hash'): FileDupli

public function delete(string $hash, string $type = 'file_hash'): ?FileDuplicate
{
$this->logger->debug('Starting deletion of duplicate', [
'hash' => $hash,
'type' => $type
]);

try {
// Try to find the duplicate first
$fileDuplicate = $this->mapper->find($hash, $type);

$this->logger->debug('Found duplicate to delete', [
'hash' => $hash,
'type' => $type,
'fileCount' => count($fileDuplicate->getFiles())
]);

// Try to delete associated files first
$files = $fileDuplicate->getFiles();
$failedFiles = [];

foreach ($files as $fileInfo) {
try {
$this->logger->debug('Attempting to delete file info', [
'path' => $fileInfo->getPath(),
'hash' => $fileInfo->getFileHash()
]);

// Try to release any locks first
try {
$this->lockingProvider->releaseAll($fileInfo->getPath(), ILockingProvider::LOCK_SHARED);
$this->logger->debug('Released locks for file', [
'path' => $fileInfo->getPath()
]);
} catch (\Exception $e) {
$this->logger->warning('Failed to release locks, continuing anyway', [
'path' => $fileInfo->getPath(),
'error' => $e->getMessage()
]);
}

$this->fileInfoService->delete($fileInfo);
} catch (\Exception $e) {
$this->logger->warning('Failed to delete file info, continuing with others', [
'path' => $fileInfo->getPath(),
'error' => $e->getMessage()
]);
$failedFiles[] = $fileInfo->getPath();
}
}

if (!empty($failedFiles)) {
$this->logger->warning('Some files failed to delete', [
'hash' => $hash,
'failedFiles' => $failedFiles
]);
}

// Now delete the duplicate entry
$this->logger->debug('Deleting duplicate entry', [
'hash' => $hash,
'type' => $type
]);

$this->mapper->delete($fileDuplicate);

$this->logger->debug('Successfully deleted duplicate', [
'hash' => $hash,
'type' => $type,
'failedFiles' => count($failedFiles)
]);

return $fileDuplicate;

} catch (DoesNotExistException $e) {
$this->logger->debug('Duplicate not found for deletion', [
'hash' => $hash,
'type' => $type,
'error' => $e->getMessage()
]);
return null;
} catch (\Exception $e) {
$this->logger->error('Failed to delete duplicate', [
'hash' => $hash,
'type' => $type,
'error' => $e->getMessage(),
'trace' => $e->getTraceAsString()
]);
throw new \RuntimeException('Failed to delete duplicate: ' . $e->getMessage(), 0, $e);
}
}

Expand Down
201 changes: 194 additions & 7 deletions lib/Service/FileInfoService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OCP\Files\Storage\IStorage;
use OCP\Files\IRootFolder;
use OCP\IDBConnection;
use OCP\AppFramework\Db\DoesNotExistException;

use OCA\DuplicateFinder\AppInfo\Application;
use OCA\DuplicateFinder\Db\FileInfo;
Expand Down Expand Up @@ -72,19 +73,79 @@ public function __construct(
*/
public function enrich(FileInfo $fileInfo): FileInfo
{
$this->logger->debug('Starting file info enrichment', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'currentHash' => $fileInfo->getFileHash()
]);

try {
$node = $this->folderService->getNodeByFileInfo($fileInfo);
if ($node) {
$this->logger->debug('Found node for file info', [
'path' => $fileInfo->getPath(),
'nodeId' => $node->getId(),
'nodeType' => get_class($node),
'size' => $node->getSize(),
'mimetype' => $node->getMimetype()
]);

$fileInfo->setNodeId($node->getId());
$fileInfo->setMimetype($node->getMimetype());
$fileInfo->setSize($node->getSize());
} else {
// Handle the case where no node is found
$this->logger->error("No node found for file info ID: " . $fileInfo->getId());
$this->logger->warning('Node not found for file info - file may have been deleted', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'hash' => $fileInfo->getFileHash()
]);

// File no longer exists, we should delete this file info
try {
$this->delete($fileInfo);
$this->logger->info('Deleted file info for non-existent file', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId()
]);
} catch (\Exception $deleteError) {
$this->logger->error('Failed to delete file info for non-existent file', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $deleteError->getMessage(),
'trace' => $deleteError->getTraceAsString()
]);
}
}
} catch (NotFoundException $e) {
$this->logger->warning('File not found during enrichment - file may have been deleted', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $e->getMessage()
]);

// File no longer exists, we should delete this file info
try {
$this->delete($fileInfo);
$this->logger->info('Deleted file info for non-existent file', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId()
]);
} catch (\Exception $deleteError) {
$this->logger->error('Failed to delete file info for non-existent file', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $deleteError->getMessage(),
'trace' => $deleteError->getTraceAsString()
]);
}
} catch (\Exception $e) {
// Log exception details
$this->logger->error("Error enriching FileInfo: " . $e->getMessage());
$this->logger->error('Error enriching FileInfo', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $e->getMessage(),
'errorClass' => get_class($e),
'trace' => $e->getTraceAsString()
]);
}
return $fileInfo;
}
Expand Down Expand Up @@ -127,7 +188,40 @@ public function findById(int $id, bool $enrich = false): FileInfo
*/
public function findByHash(string $hash, string $type = 'file_hash'): array
{
return $this->mapper->findByHash($hash, $type);
$this->logger->debug('Finding files by hash', [
'hash' => $hash,
'type' => $type
]);

$files = $this->mapper->findByHash($hash, $type);
$existingFiles = [];

foreach ($files as $fileInfo) {
try {
// Enrich and verify file existence
$enrichedFile = $this->enrich($fileInfo);

// If the file still exists after enrichment (wasn't deleted during enrich)
if ($enrichedFile->getNodeId() !== null) {
$existingFiles[] = $enrichedFile;
}
} catch (\Exception $e) {
$this->logger->warning('Error processing file during hash search', [
'path' => $fileInfo->getPath(),
'hash' => $hash,
'error' => $e->getMessage()
]);
// File will be automatically deleted in enrich() if it doesn't exist
}
}

$this->logger->debug('Found files by hash', [
'hash' => $hash,
'count' => count($existingFiles),
'ignored' => 'false'
]);

return $existingFiles;
}

/**
Expand Down Expand Up @@ -206,8 +300,101 @@ public function save(string $path, ?string $fallbackUID = null): FileInfo

public function delete(FileInfo $fileInfo): FileInfo
{
$this->mapper->delete($fileInfo);
return $fileInfo;
$this->logger->debug('Starting deletion of file info', [
'path' => $fileInfo->getPath(),
'hash' => $fileInfo->getFileHash(),
'id' => $fileInfo->getId()
]);

try {
// Try to release any locks first
try {
$this->lockingProvider->releaseAll($fileInfo->getPath(), ILockingProvider::LOCK_SHARED);
$this->logger->debug('Released locks for file', [
'path' => $fileInfo->getPath()
]);
} catch (\Exception $e) {
$this->logger->warning('Failed to release locks, continuing anyway', [
'path' => $fileInfo->getPath(),
'error' => $e->getMessage()
]);
}

// Verify if the file info exists before deletion
if ($fileInfo->getId()) {
$this->logger->debug('Verifying file info exists', [
'id' => $fileInfo->getId(),
'path' => $fileInfo->getPath()
]);

try {
$existingFileInfo = $this->mapper->findById($fileInfo->getId());

$this->logger->debug('Found existing file info', [
'id' => $existingFileInfo->getId(),
'path' => $existingFileInfo->getPath(),
'hash' => $existingFileInfo->getFileHash()
]);
} catch (DoesNotExistException $e) {
$this->logger->debug('File info already deleted', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId()
]);
return $fileInfo;
}
}

// Try to delete the actual file first if it exists
try {
$node = $this->folderService->getNodeByFileInfo($fileInfo);
if ($node) {
$this->logger->debug('Attempting to delete physical file', [
'path' => $fileInfo->getPath(),
'nodeId' => $node->getId()
]);
$node->delete();
$this->logger->debug('Successfully deleted physical file', [
'path' => $fileInfo->getPath()
]);
}
} catch (NotFoundException $e) {
$this->logger->debug('Physical file already deleted or not found', [
'path' => $fileInfo->getPath()
]);
} catch (\Exception $e) {
$this->logger->warning('Failed to delete physical file, continuing with database cleanup', [
'path' => $fileInfo->getPath(),
'error' => $e->getMessage()
]);
}

// Now delete the database entry
$result = $this->mapper->delete($fileInfo);

$this->logger->debug('Successfully deleted file info from database', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId()
]);

return $result;

} catch (DoesNotExistException $e) {
$this->logger->debug('File info not found for deletion', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $e->getMessage()
]);
// Return the original file info since it's already "deleted"
return $fileInfo;
} catch (\Exception $e) {
$this->logger->error('Failed to delete file info', [
'path' => $fileInfo->getPath(),
'id' => $fileInfo->getId(),
'error' => $e->getMessage(),
'trace' => $e->getTraceAsString()
]);
throw new \RuntimeException('Failed to delete file info: ' . $e->getMessage(), 0, $e);
}
}

public function clear(): void
Expand Down

0 comments on commit 526228d

Please sign in to comment.