From 09002808726a02a56f8f9aa0ef03dce21259de35 Mon Sep 17 00:00:00 2001 From: FinalDoom <677609+FinalDoom@users.noreply.github.com> Date: Mon, 1 Jan 2024 21:18:46 -0700 Subject: [PATCH] Fix case sensitivity, add tests, remove shift-click in tree filters --- .../components/sidebar/LocationFilters.tsx | 74 +++++----- .../javascript/stores/TorrentFilterStore.ts | 11 +- server/services/taxonomyService.test.ts | 104 ++++++++++++++ server/services/taxonomyService.ts | 133 +++++------------- server/util/fileUtil.ts | 14 +- shared/types/Taxonomy.ts | 7 +- 6 files changed, 184 insertions(+), 159 deletions(-) create mode 100644 server/services/taxonomyService.test.ts diff --git a/client/src/javascript/components/sidebar/LocationFilters.tsx b/client/src/javascript/components/sidebar/LocationFilters.tsx index ef4b1f8ce..f5c97fd97 100644 --- a/client/src/javascript/components/sidebar/LocationFilters.tsx +++ b/client/src/javascript/components/sidebar/LocationFilters.tsx @@ -6,52 +6,46 @@ import {LocationTreeNode} from '@shared/types/Taxonomy'; import SidebarFilter from './SidebarFilter'; import TorrentFilterStore from '../../stores/TorrentFilterStore'; +const buildLocationFilterTree = (location: LocationTreeNode): ReactNode => { + if (location.children.length === 1 && location.containedCount === location.children[0].containedCount) { + const onlyChild = location.children[0]; + const separator = onlyChild.fullPath.includes('/') ? '/' : '\\'; + return buildLocationFilterTree({ + ...onlyChild, + directoryName: location.directoryName + separator + onlyChild.directoryName, + }); + } + + const children = location.children.map(buildLocationFilterTree); + + return ( + + TorrentFilterStore.setLocationFilters(filter, event) + } + count={location.containedCount} + key={location.fullPath} + isActive={ + (location.fullPath === '' && !TorrentFilterStore.locationFilter.length) || + TorrentFilterStore.locationFilter.includes(location.fullPath) + } + name={location.directoryName} + slug={location.fullPath} + size={location.containedSize} + > + {(children.length && children) || undefined} + + ); +}; + const LocationFilters: FC = observer(() => { const {i18n} = useLingui(); - const locations = Object.keys(TorrentFilterStore.taxonomy.locationCounts); - - if (locations.length === 1 && locations[0] === '') { + if (TorrentFilterStore.taxonomy.locationTree.containedCount === 0) { return null; } - const buildLocationFilterTree = (location: LocationTreeNode): ReactNode => { - if ( - location.children.length === 1 && - TorrentFilterStore.taxonomy.locationCounts[location.fullPath] === - TorrentFilterStore.taxonomy.locationCounts[location.children[0].fullPath] - ) { - const onlyChild = location.children[0]; - const separator = onlyChild.fullPath.includes('/') ? '/' : '\\'; - return buildLocationFilterTree({ - ...onlyChild, - directoryName: location.directoryName + separator + onlyChild.directoryName, - }); - } - - const children = location.children.map(buildLocationFilterTree); - - return ( - - TorrentFilterStore.setLocationFilters(filter, event) - } - count={TorrentFilterStore.taxonomy.locationCounts[location.fullPath] || 0} - key={location.fullPath} - isActive={ - (location.fullPath === '' && !TorrentFilterStore.locationFilter.length) || - TorrentFilterStore.locationFilter.includes(location.fullPath) - } - name={location.directoryName} - slug={location.fullPath} - size={TorrentFilterStore.taxonomy.locationSizes[location.fullPath]} - > - {(children.length && children) || undefined} - - ); - }; - - const filterElements = TorrentFilterStore.taxonomy.locationTree.map(buildLocationFilterTree); + const filterElements = buildLocationFilterTree(TorrentFilterStore.taxonomy.locationTree); const title = i18n._('filter.location.title'); diff --git a/client/src/javascript/stores/TorrentFilterStore.ts b/client/src/javascript/stores/TorrentFilterStore.ts index f14648308..f9c59e12c 100644 --- a/client/src/javascript/stores/TorrentFilterStore.ts +++ b/client/src/javascript/stores/TorrentFilterStore.ts @@ -15,9 +15,7 @@ class TorrentFilterStore { filterTrigger = false; taxonomy: Taxonomy = { - locationCounts: {}, - locationSizes: {}, - locationTree: [], + locationTree: {directoryName: '', fullPath: '', children: [], containedCount: 0, containedSize: 0}, statusCounts: {}, tagCounts: {}, tagSizes: {}, @@ -62,9 +60,8 @@ class TorrentFilterStore { } setLocationFilters(filter: string | '', event: KeyboardEvent | MouseEvent | TouchEvent) { - const locations = Object.keys(this.taxonomy.locationCounts).sort((a, b) => a.localeCompare(b)); - - this.computeFilters(locations, this.locationFilter, filter, event); + // keys: [] to disable shift-clicking as it doesn't make sense in a tree + this.computeFilters([], this.locationFilter, filter, event); this.filterTrigger = !this.filterTrigger; } @@ -103,7 +100,7 @@ class TorrentFilterStore { ) { if (newFilter === ('' as T)) { currentFilters.splice(0); - } else if (event.shiftKey) { + } else if (event.shiftKey && keys.length) { if (currentFilters.length) { const lastKey = currentFilters[currentFilters.length - 1]; const lastKeyIndex = keys.indexOf(lastKey); diff --git a/server/services/taxonomyService.test.ts b/server/services/taxonomyService.test.ts new file mode 100644 index 000000000..1ae0acbe4 --- /dev/null +++ b/server/services/taxonomyService.test.ts @@ -0,0 +1,104 @@ +import {LocationTreeNode} from '@shared/types/Taxonomy'; +import TaxonomyService from '../../server/services/taxonomyService'; +import {UserInDatabase} from '@shared/schema/Auth'; +import os from 'os'; + +type LocationRecord = {[key: string]: LocationRecord | null}; +const toTreeNodes = (locations: LocationRecord, separator = '/', basePath = '') => + Object.keys(locations).reduce((parentNodes, locationKey) => { + const fullPath = locationKey !== '' ? basePath + separator + locationKey : locationKey; + const subLocations = locations[locationKey]; + if (subLocations) { + const parent = { + directoryName: locationKey, + fullPath: fullPath, + children: toTreeNodes(subLocations, separator, fullPath), + containedCount: 0, + containedSize: 0, + }; + for (const child of parent.children) { + parent.containedCount += child.containedCount; + parent.containedSize += child.containedSize; + } + parentNodes.push(parent); + } else { + parentNodes.push({ + directoryName: locationKey, + fullPath: fullPath, + children: [], + containedCount: '' !== locationKey ? 1 : 0, + containedSize: '' !== locationKey ? 10 : 0, + }); + } + return parentNodes; + }, [] as LocationTreeNode[]); + +describe('taxonomyService', () => { + describe('incrementLocationCountsAndSizes() - locationTree', () => { + for (const locationsAndExpected of [ + // No torrents + {locations: [] as string[], expected: toTreeNodes({'': null})[0]}, + // Single root + { + locations: ['/mnt/dir1/file1', '/mnt/dir1/file2', '/mnt/dir2/file3', '/mnt/file4'], + expected: toTreeNodes({ + '': { + mnt: {dir1: {file1: null, file2: null}, dir2: {file3: null}, file4: null}, + }, + })[0], + }, + // Multiple roots including overlapping case + { + locations: ['/mnt/file1', '/mnt/file2', '/mount/directory1/file3', '/Mount/directory2/file4'], + expected: toTreeNodes({ + '': { + mnt: {file1: null, file2: null}, + mount: {directory1: {file3: null}}, + Mount: {directory2: {file4: null}}, + }, + })[0], + }, + ]) { + const {locations, expected} = locationsAndExpected; + + it(`builds Linux-style case-sensitive location tree correctly from ${locations}`, () => { + const taxonomyService = new TaxonomyService({} as UserInDatabase); + const mock = jest.spyOn(os, 'platform'); + mock.mockImplementation(() => 'linux'); + + for (const location of locations) taxonomyService.incrementLocationCountsAndSizes(location, 10); + + expect(taxonomyService.taxonomy.locationTree).toMatchObject(expected); + + mock.mockRestore(); + }); + } + + for (const locationsAndExpected of [ + // Multiple roots including overlapping case + { + locations: ['/mnt/file1', '/mnt/file2', '/mount/directory1/file3', '/Mount/directory2/file4'], + expected: toTreeNodes({ + '': { + mnt: {file1: null, file2: null}, + mount: {directory1: {file3: null}, directory2: {file4: null}}, + }, + })[0], + }, + ]) { + const {locations, expected} = locationsAndExpected; + + it(`builds Mac-style case-insensitive location tree correctly from ${locations}`, () => { + const taxonomyService = new TaxonomyService({} as UserInDatabase); + const mock = jest.spyOn(os, 'platform'); + mock.mockImplementation(() => 'darwin'); + + for (const location of locations) taxonomyService.incrementLocationCountsAndSizes(location, 10); + + expect(taxonomyService.taxonomy.locationTree).toMatchObject(expected); + + mock.mockRestore(); + }); + } + }); +}); diff --git a/server/services/taxonomyService.ts b/server/services/taxonomyService.ts index fdc67ffec..c5fc2b044 100644 --- a/server/services/taxonomyService.ts +++ b/server/services/taxonomyService.ts @@ -2,10 +2,11 @@ import jsonpatch, {Operation} from 'fast-json-patch'; import BaseService from './BaseService'; import torrentStatusMap from '../../shared/constants/torrentStatusMap'; +import {isInsensitiveOs} from '../util/fileUtil'; -import type {Taxonomy, LocationTreeNode} from '../../shared/types/Taxonomy'; -import type {TorrentStatus} from '../../shared/constants/torrentStatusMap'; -import type {TorrentProperties, TorrentList} from '../../shared/types/Torrent'; +import type {Taxonomy, LocationTreeNode} from '@shared/types/Taxonomy'; +import type {TorrentStatus} from '@shared/constants/torrentStatusMap'; +import type {TorrentProperties, TorrentList} from '@shared/types/Torrent'; type TaxonomyServiceEvents = { TAXONOMY_DIFF_CHANGE: (payload: {id: number; diff: Operation[]}) => void; @@ -13,9 +14,7 @@ type TaxonomyServiceEvents = { class TaxonomyService extends BaseService { taxonomy: Taxonomy = { - locationCounts: {'': 0}, - locationSizes: {}, - locationTree: [], + locationTree: {directoryName: '', fullPath: '', children: [], containedCount: 0, containedSize: 0}, statusCounts: {'': 0}, tagCounts: {'': 0, untagged: 0}, tagSizes: {}, @@ -64,9 +63,7 @@ class TaxonomyService extends BaseService { handleProcessTorrentListStart = () => { this.lastTaxonomy = { - locationCounts: {...this.taxonomy.locationCounts}, - locationSizes: {...this.taxonomy.locationCounts}, - locationTree: [...this.taxonomy.locationTree], + locationTree: {...this.taxonomy.locationTree}, statusCounts: {...this.taxonomy.statusCounts}, tagCounts: {...this.taxonomy.tagCounts}, tagSizes: {...this.taxonomy.tagSizes}, @@ -78,9 +75,7 @@ class TaxonomyService extends BaseService { this.taxonomy.statusCounts[status] = 0; }); - this.taxonomy.locationCounts = {'': 0}; - this.taxonomy.locationSizes = {}; - this.taxonomy.locationTree = []; + this.taxonomy.locationTree = {directoryName: '', fullPath: '', children: [], containedCount: 0, containedSize: 0}; this.taxonomy.statusCounts[''] = 0; this.taxonomy.tagCounts = {'': 0, untagged: 0}; this.taxonomy.tagSizes = {}; @@ -88,70 +83,9 @@ class TaxonomyService extends BaseService { this.taxonomy.trackerSizes = {}; }; - buildLocationTree = () => { - const locations = Object.keys(this.taxonomy.locationCounts); - const sortedLocations = locations.slice().sort((a, b) => { - if (a === '') { - return -1; - } - if (b === '') { - return 1; - } - - // Order slashes before similar paths with different symbols, eg. /files/PC/ should come before /files/PC-98/ for treeing - const aa = a.replace(/[^\\/\w]/g, '~'); - const bb = b.replace(/[^\\/\w]/g, '~'); - if (aa < bb) { - return -1; - } - if (aa == bb) { - return 0; - } - return 1; - }); - - const separator = sortedLocations.length < 2 || sortedLocations[1].includes('/') ? '/' : '\\'; - let previousLocation: LocationTreeNode; - this.taxonomy.locationTree = sortedLocations.reduce((tree, filter) => { - const directory = filter.split(separator).slice(-1)[0]; - const parentPath = filter.substring(0, filter.lastIndexOf(separator + directory)); - const location: LocationTreeNode = {directoryName: directory, fullPath: filter, children: []}; - while (previousLocation) { - // Move up the tree to a matching parent - if (!previousLocation.parent || previousLocation.fullPath === parentPath) { - break; - } - previousLocation = previousLocation.parent; - } - if (previousLocation && previousLocation.fullPath === parentPath && parentPath !== '') { - // Child - location.parent = previousLocation; - previousLocation.children.push(location); - } else if (previousLocation && previousLocation.parent && previousLocation.parent.fullPath === parentPath) { - // Sibling - location.parent = previousLocation.parent; - previousLocation.parent.children.push(location); - } else { - // Root - tree.push(location); - } - previousLocation = location; - return tree; - }, [] as LocationTreeNode[]); - }; - - cleanLocationTree = (location: LocationTreeNode) => { - location.parent = undefined; - location.children.forEach(this.cleanLocationTree); - }; - handleProcessTorrentListEnd = ({torrents}: {torrents: TorrentList}) => { const {length} = Object.keys(torrents); - this.buildLocationTree(); - this.taxonomy.locationTree.forEach(this.cleanLocationTree); - - this.taxonomy.locationCounts[''] = length; this.taxonomy.statusCounts[''] = length; this.taxonomy.tagCounts[''] = length; this.taxonomy.trackerCounts[''] = length; @@ -179,34 +113,35 @@ class TaxonomyService extends BaseService { directory: TorrentProperties['directory'], sizeBytes: TorrentProperties['sizeBytes'], ) { - const separator = directory.includes('/') ? '/' : '\\'; - const parts = directory.startsWith(separator) ? directory.split(separator).slice(1) : directory.split(separator); - let heirarchy = ''; - - if (this.taxonomy.locationCounts[heirarchy] != null) { - this.taxonomy.locationCounts[heirarchy] += 1; - } else { - this.taxonomy.locationCounts[heirarchy] = 1; - } - if (this.taxonomy.locationSizes[heirarchy] != null) { - this.taxonomy.locationSizes[heirarchy] += sizeBytes; - } else { - this.taxonomy.locationSizes[heirarchy] = sizeBytes; - } - - parts.forEach((part) => { - heirarchy += separator + part; - if (this.taxonomy.locationCounts[heirarchy] != null) { - this.taxonomy.locationCounts[heirarchy] += 1; - } else { - this.taxonomy.locationCounts[heirarchy] = 1; + const separator = directory.includes('\\') ? '\\' : '/'; + + const countSizeAndBytesForHierarchy = (parent: LocationTreeNode, pathSplit: string[]) => { + const [nodeName, ...restOfPath] = pathSplit; + let nodeRoot = parent.children.find((treeNode) => treeNode.directoryName === nodeName); + if (!nodeRoot) { + nodeRoot = { + directoryName: nodeName, + fullPath: parent.fullPath + separator + nodeName, + children: [], + containedCount: 0, + containedSize: 0, + }; + parent.children.push(nodeRoot); } - if (this.taxonomy.locationSizes[heirarchy] != null) { - this.taxonomy.locationSizes[heirarchy] += sizeBytes; - } else { - this.taxonomy.locationSizes[heirarchy] = sizeBytes; + nodeRoot.containedCount += 1; + nodeRoot.containedSize += sizeBytes; + + if (restOfPath.length) { + countSizeAndBytesForHierarchy(nodeRoot, restOfPath); } - }); + }; + + const path = isInsensitiveOs() ? directory.toLocaleLowerCase() : directory; + const pathSplit = path.startsWith(separator) ? path.split(separator).slice(1) : path.split(separator); + + countSizeAndBytesForHierarchy(this.taxonomy.locationTree, pathSplit); + this.taxonomy.locationTree.containedCount += 1; + this.taxonomy.locationTree.containedSize += sizeBytes; } incrementStatusCounts(statuses: Array) { diff --git a/server/util/fileUtil.ts b/server/util/fileUtil.ts index 0e7617e15..e7b010354 100644 --- a/server/util/fileUtil.ts +++ b/server/util/fileUtil.ts @@ -1,6 +1,6 @@ import fs from 'fs'; import {promises as fsp} from 'fs'; -import {homedir} from 'os'; +import {homedir, platform} from 'os'; import path from 'path'; import config from '../../config'; @@ -37,10 +37,7 @@ export const isAllowedPath = (resolvedPath: string) => { } return config.allowedPaths.some((allowedPath) => { - if (realPath?.startsWith(allowedPath)) { - return true; - } - return false; + return !!realPath?.startsWith(allowedPath); }); }; @@ -64,10 +61,7 @@ export async function isAllowedPathAsync(resolvedPath: string) { } return config.allowedPaths.some((allowedPath) => { - if (realPath?.startsWith(allowedPath)) { - return true; - } - return false; + return !!realPath?.startsWith(allowedPath); }); } @@ -94,3 +88,5 @@ export const sanitizePath = (input?: string): string => { return path.resolve(input.replace(/^~/, homedir()).replace(controlRe, '')); }; + +export const isInsensitiveOs = (): boolean => ['darwin', 'win32'].includes(platform()); diff --git a/shared/types/Taxonomy.ts b/shared/types/Taxonomy.ts index a3abe5314..de3f459c2 100644 --- a/shared/types/Taxonomy.ts +++ b/shared/types/Taxonomy.ts @@ -2,13 +2,12 @@ export interface LocationTreeNode { directoryName: string; fullPath: string; children: LocationTreeNode[]; - parent?: LocationTreeNode; + containedCount: number; + containedSize: number; } export interface Taxonomy { - locationCounts: Record; - locationSizes: Record; - locationTree: LocationTreeNode[]; + locationTree: LocationTreeNode; statusCounts: Record; tagCounts: Record; tagSizes: Record;