Data factory, store, fether types linkeddata#355

src/data-factory-internal.ts

@@ -11,6 +11,7 @@ import {
   GraphType,
   TermType,
   TFDataFactory,
+  TFTerm,
 } from './types'
 import { Feature, IdentityFactory, Indexable } from './data-factory-type'
 import { Node } from './index'
@@ -37,15 +38,15 @@ function defaultGraph(): NamedNode {
  *
  * Equivalent to {Term.hashString}
  */
-function id (term: Node): string | undefined {
+function id (term: TFTerm): string | undefined {
   if (!term) {
     return term
   }
   if (Object.prototype.hasOwnProperty.call(term, "id") && typeof (term as NamedNode).id === "function") {
     return (term as NamedNode).id()
   }
   if (Object.prototype.hasOwnProperty.call(term, "hashString")) {
-    return term.hashString()
+    return (term as Node).hashString()
   }
 
   switch (term.termType) {
@@ -142,6 +143,7 @@ const CanonicalDataFactory: TFDataFactory<
   namedNode,
   quad,
   variable,
+  // @ts-ignore id should return an Indexable value, but in RDFlib can return `undefined`
   id,
   supports: {
     [Feature.collections]: false,

src/data-factory-type.ts

@@ -1,4 +1,4 @@
-import { TFNamedNode, TFBlankNode, TFLiteral, TFQuad, TFTerm, TFDataFactory, TFDefaultGraph, TFSubject, TFPredicate, TFObject, TFGraph } from "./types"
+import { TFNamedNode, TFBlankNode, TFLiteral, TFQuad, TFTerm, TFDataFactory, TFSubject, TFPredicate, TFObject, TFGraph, TFVariable } from "./types"
 
 /**
  * Defines a strict subset of the DataFactory as defined in the RDF/JS: Data model specification
@@ -46,11 +46,11 @@ export interface DataFactory<
   toNQ(term: FactoryTypes): string
 }
 
-export type TFIDFactoryTypes = TFNamedNode | TFBlankNode | TFLiteral | TFQuad
+export type TFIDFactoryTypes = TFNamedNode | TFBlankNode | TFLiteral | TFQuad | TFVariable | TFTerm
 
 export interface IdentityFactory<
   Quad = TFQuad,
-  IDFactoryTypes = TFNamedNode | TFBlankNode | TFLiteral | Quad,
+  IDFactoryTypes = TFNamedNode | TFBlankNode | TFLiteral | Quad | TFVariable | TFTerm,
   IndexType = Indexable,
 > {
   /**

src/fetcher.ts

@@ -38,9 +38,11 @@ import { isTFNamedNode, isCollection, nodeValue } from './utils'
 import * as Util from './util'
 import serialize from './serialize'
 
+// @ts-ignore This is injected
 import { fetch as solidAuthCli } from 'solid-auth-cli'
+// @ts-ignore This is injected
 import { fetch as solidAuthClient } from 'solid-auth-client'
-import { TFBlankNode, TFNamedNode, TFTerm, TFGraph, TFSubject, ContentType } from './types'
+import { TFBlankNode, TFNamedNode, TFGraph, TFSubject, ContentType } from './types'
 import Literal from './literal'
 
 // This is a special fetch which does OIDC auth, catching 401 errors
@@ -691,7 +693,7 @@ export default class Fetcher implements CallbackifyInterface {
   /** Keep track of explicit 404s -> we can overwrite etc */
   nonexistent: BooleanMap
   lookedUp: BooleanMap
-  handlers: Array<Handler>
+  handlers: Array<typeof Handler>
   static HANDLERS: {
     [handlerName: number]: Handler
   }
@@ -907,10 +909,11 @@ export default class Fetcher implements CallbackifyInterface {
   load (
     uri: TFNamedNode | string | Array<string | TFNamedNode>,
     options: Options = {}
-  ): Promise<Result> | Result[] {
+  ): Promise<Result> | Promise<Result>[] {
     options = Object.assign({}, options) // Take a copy as we add stuff to the options!!
     if (uri instanceof Array) {
       return Promise.all(
+        // @ts-ignore Returns an array of promises. Without this ignore, the type is recursive
         uri.map(x => { return this.load(x, Object.assign({}, options)) })
       )
     }
@@ -1552,14 +1555,15 @@ export default class Fetcher implements CallbackifyInterface {
   lookUpThing (
     term: TFNamedNode,
     rterm: TFNamedNode
-  ): Promise<Response> | Response[] {
+  ): Promise<Response> | Promise<Response>[] {
     let uris = this.store.uris(term)  // Get all URIs
     uris = uris.map(u => Uri.docpart(u))  // Drop hash fragments
 
     uris.forEach(u => {
       this.lookedUp[u] = true
     })
 
+    // @ts-ignore Recursive type
     return this.load(uris, { referringTerm: rterm })
   }
 
@@ -1583,7 +1587,7 @@ export default class Fetcher implements CallbackifyInterface {
       if (request !== undefined) {
         let response = kb.any(request, ns.link('response')) as TFSubject
 
-        if (response !== undefined && kb.anyValue(response, ns.http('status')) && kb.anyValue(response, ns.http('status')).startsWith('2')) {
+        if (response !== undefined && kb.anyValue(response, ns.http('status')) && (kb.anyValue(response, ns.http('status')) as string).startsWith('2')) {
           // Only look at success returns - not 401 error messagess etc
           let results = kb.each(response, ns.httph(header.toLowerCase()))
 
@@ -1733,12 +1737,12 @@ export default class Fetcher implements CallbackifyInterface {
 
   unload (term: TFNamedNode) {
     this.store.removeDocument(term)
-    delete this.requested[term.value] // So it can be loaded again
+    delete this.requested[term.value] // So it can be load2ed again
   }
 
-  addHandler (handler: Handler) {
+  addHandler (handler: typeof Handler) {
     this.handlers.push(handler);
-    (handler as N3Handler).register(this)
+    (handler as any).register(this)
   }
 
   retryNoCredentials (
@@ -1852,6 +1856,7 @@ export default class Fetcher implements CallbackifyInterface {
     docuri: string,
     options: AutoInitOptions
   ): Promise<FetchError | ExtendedResponse> | ExtendedResponse {
+
     const kb = this.store
     const headers: Headers = (response as Response).headers
 
@@ -1942,7 +1947,8 @@ export default class Fetcher implements CallbackifyInterface {
       return this.doneFetch(options, response)
     }
 
-    return response.text()
+    return response
+      .text()
       .then(responseText => {
         response.responseText = responseText
         return (handler as N3Handler).parse(this, responseText, options, response)
@@ -1968,10 +1974,11 @@ export default class Fetcher implements CallbackifyInterface {
       return null
     }
 
-    let Handler = this.handlers.find((handler: Handler) => {
+    let Handler = this.handlers.find(handler => {
       return contentType.match(handler.pattern)
     })
 
+    // @ts-ignore in practice all Handlers have constructors.
     return Handler ? new Handler(response) : null
   }
 

src/store.ts

@@ -193,7 +193,7 @@ export default class IndexedFormula extends Formula { // IN future - allow pass
     patch: {
         delete?: ReadonlyArray<Statement>,
         patch?: ReadonlyArray<Statement>,
-        where?: unknown
+        where?: any
     },
     target: NamedNode,
     patchCallback: (errorString: string) => void
@@ -331,7 +331,7 @@ export default class IndexedFormula extends Formula { // IN future - allow pass
    * @param why - The document in which the triple (S,P,O) was or will be stored on the web
    * @returns The statement added to the store, or the store
    */
-  //@ts-ignore differs from signature in Formula
+  // @ts-ignore differs from signature in Formula
   add (
     subj: TFSubject | TFQuad | TFQuad[] | Statement | Statement[],
     pred?: TFPredicate,
@@ -355,7 +355,7 @@ export default class IndexedFormula extends Formula { // IN future - allow pass
     var st: TFQuad
     if (!why) {
       // system generated
-      why = this.fetcher ? this.fetcher.appNode : this.defaultGraph()
+      why = this.fetcher ? this.fetcher.appNode : this.rdfFactory.defaultGraph()
     }
     if (typeof subj == 'string') {
       subj = new NamedNode(subj)

src/update-manager.ts

@@ -15,7 +15,7 @@ import { isStore, isNamedNode, isTFBlankNode, nodeValue } from './utils'
 import * as Util from './util'
 import Statement from './statement';
 import { NamedNode } from './index'
-import { TFNamedNode, TFQuad, TFBlankNode, TFSubject } from './types'
+import { TFNamedNode, TFQuad, TFBlankNode, TFSubject, TFPredicate, TFObject, TFGraph, TFTerm } from './types'
 
 interface UpdateManagerFormula extends IndexedFormula {
   fetcher: Fetcher
@@ -423,11 +423,14 @@ export default class UpdateManager {
         var query = this.where
         query += 'DELETE DATA { ' + this.statementNT + ' } ;\n'
         query += 'INSERT DATA { ' +
+          // @ts-ignore `this` might refer to the wrong scope. Does this work?
           this.anonymize(this.statement[0]) + ' ' +
+          // @ts-ignore
           this.anonymize(this.statement[1]) + ' ' +
+          // @ts-ignore
           this.anonymize(obj) + ' ' + ' . }\n'
 
-        updater.fire(this.statement[3].value, query, callbackFunction)
+        updater.fire((this.statement as [TFSubject, TFPredicate, TFObject, TFGraph])[3].value, query, callbackFunction)
       }
     }
   }
@@ -547,14 +550,14 @@ export default class UpdateManager {
           }
         } else {
           control.reloading = false
-          if (response.status === 0) {
+          if ((response as Response).status === 0) {
             console.log('Network error refreshing the data. Retrying in ' +
               retryTimeout / 1000)
             control.reloading = true
             retryTimeout = retryTimeout * 2
             setTimeout(tryReload, retryTimeout)
           } else {
-            console.log('Error ' + response.status + 'refreshing the data:' +
+            console.log('Error ' + (response as Response).status + 'refreshing the data:' +
               message + '. Stopped' + doc)
           }
         }
@@ -860,7 +863,12 @@ export default class UpdateManager {
     }
   }
 
-  updateDav (doc, ds, is, callbackFunction) {
+  updateDav (
+    doc: TFSubject,
+    ds,
+    is,
+    callbackFunction
+  ): null | Promise<void> {
     let kb = this.store
     // The code below is derived from Kenny's UpdateCenter.js
     var request = kb.any(doc, this.ns.link('request'))
@@ -872,7 +880,7 @@ export default class UpdateManager {
     if (!response) {
       return null // throw "No record HTTP GET response for document: "+doc
     }
-    var contentType = kb.the(response, this.ns.httph('content-type')).value
+    var contentType = (kb.the(response, this.ns.httph('content-type'))as TFTerm).value
 
     // prepare contents of revised document
     let newSts = kb.statementsMatching(undefined, undefined, undefined, doc).slice() // copy!
@@ -958,7 +966,9 @@ export default class UpdateManager {
     console.log('Writing back: <<<' + documentString + '>>>')
     var filename = doc.value.slice(7) // chop off   file://  leaving /path
     // console.log("Writeback: Filename: "+filename+"\n")
+    // @ts-ignore Where does Component come from? Perhaps deprecated?
     var file = Components.classes[ '@mozilla.org/file/local;1' ]
+    // @ts-ignore Where does Component come from? Perhaps deprecated?
       .createInstance(Components.interfaces.nsILocalFile)
     file.initWithPath(filename)
     if (!file.exists()) {
@@ -970,7 +980,9 @@ export default class UpdateManager {
     // }
     // create file output stream and use write/create/truncate mode
     // 0x02 writing, 0x08 create file, 0x20 truncate length if exist
+    // @ts-ignore Where does Component come from? Perhaps deprecated?
     var stream = Components.classes[ '@mozilla.org/network/file-output-stream;1' ]
+    // @ts-ignore Where does Component come from? Perhaps deprecated?
       .createInstance(Components.interfaces.nsIFileOutputStream)
 
     // Various JS systems object to 0666 in struct mode as dangerous
@@ -1120,6 +1132,6 @@ export default class UpdateManager {
 }
 
 interface docReloadType extends TFNamedNode {
-  reloadTimeCount: number
-  reloadTimeTotal: number
+  reloadTimeCount?: number
+  reloadTimeTotal?: number
 }

src/wip.md

@@ -41,6 +41,8 @@ Builds upon the approved #363 PR for [typescript migration](https://github.com/l
 - `Fetcher.refreshIfExpired` passed an array of headers, but it needs only one string.
 - `Fethcer` uses `Headers` a lot. I've changed empty objects to empty `new Headers` instances, which enhances compatibility with default `Fetch` behavior.
 - `Serializer.tripleCallback` had an unused third argument.
+- `UpdateManager.update` checked an undefined `secondTry` variable. Since later in the same function, `.update` is called with a 4th argument, I assume this is `secondTry`. I've added it as an optional argument. Perhaps this is
+- `Formula.add()` now uses `this.rdfFactory.defaultGraph` instead of the non-existent `this.defaultGraph`
 
 ## Possible bugs not fixed by this PR
 
@@ -49,9 +51,13 @@ Builds upon the approved #363 PR for [typescript migration](https://github.com/l
 - `store.add()` accepts many types of inputs, but this will lead to invalid statements (e.g. a Literal as a Subject). I suggest we make this more strict and throw more errors on wrong inputs. Relates to #362. We could still make the allowed inputs bigger by allowing other types with some explicit behavior, e.g. in subject arguments, create `NamedNodes` from `URL` objects and `strings` that look like URLs . In any case, I thinkg the `Node.fromValue` behavior is too unpredictable for `store.add`. For now, I've updated the docs to match its behavior.
 - The types for `Node.fromValue` and `Literal.fromValue` show how unpredictable these methods are. I suggest we make them more strict (also relates to #362), so they either return a `TFTerm` (`node`) or throw an error - they should not return `undefined` or `null`. Also, I think they should be converted to functions in `Utils`: this would fix the circular dependency issue (why we need `node_internal`) and it would fix the type issues in `Literal.fromValue` (which tends to give issues since it's signature does not correctly extend from `Node.fromValue`)
 - In `Fetcher.addtype`, the final logic will allways return `true`, since `redirection` is a `NamedNode`. Should it call `.value`?
+- Various `Hanlder.parse()` functions in `Fetcher` return either a `Response` or a `Promise<Error>`. This seems like weird behavior - should it not always return an array?
 - The `defaultGraph` iri is set to `chrome:theSession`, but this errors in Firefox. I suggest we change it to something else. See #370.
 - The `Parse.executeErrorCallback` conditional logic is always `true`.
-- Many `// @ts-ignore` comments for possible bugs.
+- I've added many `// @ts-ignore` comments. Ideally, these should be resolved by fixing the underlying type issues.
+- `UpdateManager.update_statement` seems to refer to the wrong `this`. It calls `this.anonimize`, but it is not available in that scope.
+- `UpdateManager.updateLocalFile` uses `Component`, but this is not defined anywhere. Is this deprecated?
+- `Data-factory-internal.id()` returns `string | undefined`, I feel like undefined should not be possible - it should throw an error. This would resolve the type incompatibility on line 146.
 
 ## Unused code
 
@@ -74,6 +80,7 @@ Builds upon the approved #363 PR for [typescript migration](https://github.com/l
 - Many functions in `Fetcher` assume that specific `Opts` are defined. I've included all these in a single `Options` type and added documentation for the props I understood. I've also created an `AutoInitOptions` type, which sets auto-initialized. I extended Options in each function where specific opts seemed to be required. I'm not entirely confident about the types I've set. I feel like the truly required items should never be `Opts`, since they aren't optional. Refactoring this requires a seperate PR.
 - `Fetcher.load` allows arrays as inputs. This causes the output types to be more unpredictable. `Promise<Result> | Result[]`. I suggest splitting this in two functions, e.g. add `loadMany`
 - `Utils.callbackify` seems to be used only in `Fetcher`.
+- `UpdateManager.editable` has a confusing return type (`string | boolean | undefined`). I suggest we refactor it to always return one of multiple pre-defined strings,.
 
 ## Some thoughts on simplifying language