Fixed major code smells (#10)

* Fixed major code smells * Fixes fetch using file:/// urls * Fixed Deno version typo * Replaced deprecated test deps
petruki · Oct 2, 2023 · 05ce984 · 05ce984
1 parent 093d9ef
commit 05ce984
Show file tree

Hide file tree

Showing 15 changed files with 243 additions and 298 deletions.
diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml
@@ -18,10 +18,10 @@ jobs:
         with:
           fetch-depth: 0
 
-      - name: Setup Deno v1.33.4
+      - name: Setup Deno v1.37.0
         uses: denoland/setup-deno@v1
         with:
-          deno-version: v1.33.4
+          deno-version: v1.37.0
 
       - name: Setup LCOV        
         run: sudo apt install -y lcov

diff --git a/deno.jsonc b/deno.jsonc
@@ -1,9 +1,11 @@
 {
   "tasks": {
-    "fmt": "deno fmt mod.ts src/ --check",
+    "fmt": "deno fmt mod.ts src/ --options-single-quote --options-line-width=120 --check",
+    "fmt:fix": "deno fmt mod.ts test/ src/ example/ --options-single-quote --options-line-width=120",
     "test": "deno test --unstable --allow-read --allow-net --coverage=coverage",
     "lcov": "deno coverage coverage --lcov --output=coverage/report.lcov",
     "cover": "deno task clean && deno task test && deno task lcov && genhtml -o coverage/html coverage/report.lcov",
-    "clean": "rm -rf ./npm ./coverage"
+    "clean": "rm -rf ./npm ./coverage",
+    "cache-reload": "deno cache --reload --lock=deno.lock --lock-write mod.ts test/deps.ts"
   }
 }
diff --git a/deno.lock b/deno.lock
diff --git a/example/app.ts b/example/app.ts
@@ -1,10 +1,8 @@
-import { Context, Output, Skimming } from "../mod.ts";
+import { Context, Output, Skimming } from '../mod.ts';
 
 function printResult(entries: Output[]) {
   entries.forEach((output) => {
-    console.log(
-      `- File: ${output.file} - Found: ${output.found} - Cache: ${output.cache}`,
-    );
+    console.log(`- File: ${output.file} - Found: ${output.found} - Cache: ${output.cache}`);
     output.segment.forEach((data, index) => {
       console.log(
         `\n- Output #${index} -----------------------------\n${data}`,
@@ -14,32 +12,31 @@ function printResult(entries: Output[]) {
 }
 
 async function main() {
-  const files = ["README.md"];
+  const files = ['README.md'];
   const context: Context = {
-    url:
-      "https://raw.githubusercontent.com/petruki/skimming/master/test/fixtures/",
+    url: `file:///${Deno.cwd()}/../test/fixtures/`,
     files,
   };
 
   const skimmer = new Skimming({ expireDuration: 2, size: 10 });
   skimmer.setContext(context);
 
-  console.log("##############################################");
-  let output = await skimmer.skim("Skimming({", {
+  console.log('##############################################');
+  let output = await skimmer.skim('Skimming({', {
     previewLength: 100,
     trimContent: true,
   });
   printResult(output);
 
-  console.log("##############################################");
-  output = await skimmer.skim("Skimming({", {
+  console.log('##############################################');
+  output = await skimmer.skim('Skimming({', {
     previewLength: -1,
     trimContent: true,
   });
   printResult(output);
 
-  console.log("##############################################");
-  output = await skimmer.skim("#{3}", { previewLength: -1, regex: true });
+  console.log('##############################################');
+  output = await skimmer.skim('#{3}', { previewLength: -1, regex: true });
   printResult(output);
 }
 

diff --git a/mod.ts b/mod.ts
@@ -1,7 +1,2 @@
-export { Skimming } from "./src/skimming.ts";
-export type {
-  CacheOptions,
-  Context,
-  FetchOptions,
-  Output,
-} from "./src/lib/types.ts";
+export { Skimming } from './src/skimming.ts';
+export type { CacheOptions, Context, FetchOptions, Output } from './src/lib/types.ts';
diff --git a/sonar-project.properties b/sonar-project.properties
@@ -1,7 +1,7 @@
 sonar.projectKey=petruki_skimming
 sonar.projectName=skimming
 sonar.organization=petruki
-sonar.projectVersion=1.0.9
+sonar.projectVersion=1.0.10
 
 sonar.javascript.lcov.reportPaths=coverage/report.lcov
 

diff --git a/src/lib/cache.ts b/src/lib/cache.ts
@@ -1,9 +1,5 @@
-import { Cache, CacheOptions, FetchOptions, Output } from "./types.ts";
-import {
-  DEFAULT_IGNORE_CASE,
-  DEFAULT_PREVIEW_LENGTH,
-  DEFAULT_TRIM,
-} from "../skimming.ts";
+import { Cache, CacheOptions, FetchOptions, Output } from './types.ts';
+import { DEFAULT_IGNORE_CASE, DEFAULT_PREVIEW_LENGTH, DEFAULT_TRIM } from '../skimming.ts';
 
 const DEFAULT_CACHE_SIZE = 60;
 const DEFAULT_CACHE_DURATION = 60; // 1 min
@@ -70,9 +66,7 @@ export default class CacheHandler {
     const cachedData = this.cache.filter((cache) => cache.query === query);
 
     if (cachedData.length) {
-      cachedData[0].output = cachedData[0].output.filter((cachedOutput) =>
-        cachedOutput.file != output.file
-      );
+      cachedData[0].output = cachedData[0].output.filter((cachedOutput) => cachedOutput.file != output.file);
       cachedData[0].output.push(output);
       cachedData[0].exp = Date.now() + (1000 * this.cacheExpireDuration);
       cachedData[0].previewLength = previewLength;
@@ -142,12 +136,9 @@ export default class CacheHandler {
     storedData: Cache,
     { previewLength, ignoreCase, trimContent }: FetchOptions,
   ): boolean {
-    return storedData.previewLength !=
-        (previewLength != undefined ? previewLength : DEFAULT_PREVIEW_LENGTH) ||
-      storedData.ignoreCase !=
-        (ignoreCase != undefined ? ignoreCase : DEFAULT_IGNORE_CASE) ||
-      storedData.trimContent !=
-        (trimContent != undefined ? trimContent : DEFAULT_TRIM);
+    return storedData.previewLength !== (previewLength ?? DEFAULT_PREVIEW_LENGTH) ||
+      storedData.ignoreCase !== (ignoreCase ?? DEFAULT_IGNORE_CASE) ||
+      storedData.trimContent !== (trimContent ?? DEFAULT_TRIM);
   }
 
   /**
@@ -156,8 +147,6 @@ export default class CacheHandler {
   private updateCache(): void {
     this.cache = this.cache.filter((storedData) => storedData.exp > Date.now());
 
-    this.cache = this.cache.sort((cachea: Cache, cacheb: Cache) =>
-      cachea.exp > cacheb.exp ? 1 : -1
-    );
+    this.cache.sort((cachea: Cache, cacheb: Cache) => cachea.exp > cacheb.exp ? 1 : -1);
   }
 }
diff --git a/src/lib/exceptions.ts b/src/lib/exceptions.ts
@@ -1,4 +1,4 @@
-import { FetchOptions } from "./types.ts";
+import { FetchOptions } from './types.ts';
 
 export class InvalidQuery extends Error {
   constructor(cause: string) {
@@ -26,7 +26,7 @@ export class NonMappedInstruction extends Error {
 
   constructor(query: string, options: FetchOptions) {
     super(
-      `Request exit to prevent crashing - query: ${query} - options: ${options}`,
+      `Request exit to prevent crashing - query: ${query} - options: ${String(options)}`,
     );
     this.name = this.constructor.name;
   }

diff --git a/src/lib/utils.ts b/src/lib/utils.ts
@@ -1,19 +1,14 @@
-import { Context } from "./types.ts";
-import { InvalidContext, InvalidQuery } from "./exceptions.ts";
-import {
-  DEFAULT_NEXT,
-  DEFAULT_PREVIEW_LENGTH,
-  DEFAULT_REGEX,
-  DEFAULT_TRIM,
-} from "../skimming.ts";
+import { Context } from './types.ts';
+import { InvalidContext, InvalidQuery } from './exceptions.ts';
+import { DEFAULT_NEXT, DEFAULT_PREVIEW_LENGTH, DEFAULT_REGEX, DEFAULT_TRIM } from '../skimming.ts';
 
-export const LINE_BREAK = "\n";
-export const LAST_LINE = "$";
+export const LINE_BREAK = '\n';
+export const LAST_LINE = '$';
 export const REGEX_ESCAPE = /[.*+?^${}()|[\]\\]/g;
 
 export function validateQuery(query: string, limit?: number): void {
   if (!query.length) {
-    throw new InvalidQuery("it is empty");
+    throw new InvalidQuery('it is empty');
   }
 
   if (limit && query.length > limit) {
@@ -24,19 +19,13 @@ export function validateQuery(query: string, limit?: number): void {
 export function validateContext(context: Context): void {
   if (!context?.url.length) {
     throw new InvalidContext(
-      "context is not defined properly - use setContext() to define the context",
+      'context is not defined properly - use setContext() to define the context',
     );
   }
 
   context.files.forEach((file) => {
     if (!file.length) {
-      throw new InvalidContext("file name is empty");
-    } else {
-      if (!context.url.endsWith("/") && !file.startsWith("/")) {
-        throw new InvalidContext(
-          `this enpoint might not work: ${context.url}${file}`,
-        );
-      }
+      throw new InvalidContext('file name is empty');
     }
   });
 }
@@ -63,18 +52,16 @@ export function extractSegment(
       0,
       lastPos > 0 ? lastPos + 1 : from.search(LAST_LINE),
     );
+  } else if (trimContent) {
+    offset = from.trimStart().substring(
+      0,
+      from.trimStart().indexOf(LINE_BREAK) + previewLength,
+    );
   } else {
-    if (trimContent) {
-      offset = from.trimStart().substring(
-        0,
-        from.trimStart().indexOf(LINE_BREAK) + previewLength,
-      );
-    } else {
-      offset = from.trimStart().substring(
-        0,
-        previewLength === 0 ? query.length : previewLength,
-      );
-    }
+    offset = from.trimStart().substring(
+      0,
+      previewLength === 0 ? query.length : previewLength,
+    );
   }
 
   // Find the last line position in a multiline content
@@ -104,14 +91,12 @@ export function findFirstPos(
   regex: boolean = DEFAULT_REGEX,
   next: boolean = DEFAULT_NEXT,
 ): number {
-  const foundIndex = regex
-    ? contentToFetch.search(query)
-    : contentToFetch.search(query.replace(REGEX_ESCAPE, "\\$&"));
+  const foundIndex = regex ? contentToFetch.search(query) : contentToFetch.search(query.replace(REGEX_ESCAPE, '\\$&'));
 
   if (trimContent && foundIndex != -1) {
     let firstPos = next ? foundIndex : 0;
     for (let index = foundIndex; index >= 0; index--) {
-      if (contentToFetch.charAt(index) == "\n") {
+      if (contentToFetch.charAt(index) == '\n') {
         firstPos = index;
         break;
       }

diff --git a/src/skimming.ts b/src/skimming.ts
@@ -1,16 +1,7 @@
-import { CacheOptions, Context, FetchOptions, Output } from "./lib/types.ts";
-import {
-  extractSegment,
-  findFirstPos,
-  validateContext,
-  validateQuery,
-} from "./lib/utils.ts";
-import {
-  InvalidQuery,
-  NonMappedInstruction,
-  NotContentFound,
-} from "./lib/exceptions.ts";
-import CacheHandler from "./lib/cache.ts";
+import { CacheOptions, Context, FetchOptions, Output } from './lib/types.ts';
+import { extractSegment, findFirstPos, validateContext, validateQuery } from './lib/utils.ts';
+import { InvalidQuery, NonMappedInstruction, NotContentFound } from './lib/exceptions.ts';
+import CacheHandler from './lib/cache.ts';
 
 export const DEFAULT_PREVIEW_LENGTH = 200;
 export const DEFAULT_TRIM = true;
@@ -125,7 +116,7 @@ export class Skimming {
         const from = content.substring(foundIndex);
         const segment = extractSegment(from, query, previewLength, trimContent);
         segments.push(segment);
-        content = content.replace(segment, "");
+        content = content.replace(segment, '');
         contentToFetch = ignoreCase ? content.toLowerCase() : content;
         foundIndex = findFirstPos(
           contentToFetch,
@@ -153,7 +144,7 @@ export class Skimming {
    * Fetch document online located at the provided url
    */
   private async readDocument(url: string, doc: string): Promise<string> {
-    const result = await fetch(`${url}${doc}`);
+    const result = await fetch(`${url}/${doc}`);
     if (result?.body != null) {
       if (result.status === 200) {
         return await result.text().then((data: string) => {