From 1d675007876c3d3ecfdaa806b585d4361fd21a33 Mon Sep 17 00:00:00 2001 From: Joep Meindertsma Date: Tue, 26 Nov 2019 18:46:02 +0100 Subject: [PATCH] Data factory, store, fether types #355 --- src/data-factory-internal.ts | 6 ++++-- src/data-factory-type.ts | 6 +++--- src/fetcher.ts | 27 +++++++++++++++++---------- src/store.ts | 6 +++--- src/update-manager.ts | 28 ++++++++++++++++++++-------- src/wip.md | 9 ++++++++- 6 files changed, 55 insertions(+), 27 deletions(-) diff --git a/src/data-factory-internal.ts b/src/data-factory-internal.ts index d78db4806..87aba4485 100644 --- a/src/data-factory-internal.ts +++ b/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,7 +38,7 @@ function defaultGraph(): NamedNode { * * Equivalent to {Term.hashString} */ -function id (term: Node): string | undefined { +function id (term: TFTerm): string | undefined { if (!term) { return term } @@ -45,7 +46,7 @@ function id (term: Node): string | undefined { 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, diff --git a/src/data-factory-type.ts b/src/data-factory-type.ts index aa8161d28..014f59fbb 100644 --- a/src/data-factory-type.ts +++ b/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, > { /** diff --git a/src/fetcher.ts b/src/fetcher.ts index 0d6cfeef5..96d562f81 100644 --- a/src/fetcher.ts +++ b/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 + handlers: Array static HANDLERS: { [handlerName: number]: Handler } @@ -907,10 +909,11 @@ export default class Fetcher implements CallbackifyInterface { load ( uri: TFNamedNode | string | Array, options: Options = {} - ): Promise | Result[] { + ): Promise | Promise[] { 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,7 +1555,7 @@ export default class Fetcher implements CallbackifyInterface { lookUpThing ( term: TFNamedNode, rterm: TFNamedNode - ): Promise | Response[] { + ): Promise | Promise[] { let uris = this.store.uris(term) // Get all URIs uris = uris.map(u => Uri.docpart(u)) // Drop hash fragments @@ -1560,6 +1563,7 @@ export default class Fetcher implements CallbackifyInterface { 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 | 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 } diff --git a/src/store.ts b/src/store.ts index 54261cadb..59cf9bfa4 100644 --- a/src/store.ts +++ b/src/store.ts @@ -193,7 +193,7 @@ export default class IndexedFormula extends Formula { // IN future - allow pass patch: { delete?: ReadonlyArray, patch?: ReadonlyArray, - 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) diff --git a/src/update-manager.ts b/src/update-manager.ts index 42bd491dc..dd254f9dd 100644 --- a/src/update-manager.ts +++ b/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 { 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 } diff --git a/src/wip.md b/src/wip.md index 50c1c0fa4..9968a2db2 100644 --- a/src/wip.md +++ b/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`. 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[]`. 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