-
Notifications
You must be signed in to change notification settings - Fork 0
Code Review Week 2 #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: diff2
Are you sure you want to change the base?
Changes from all commits
194dd4c
5a28af0
4e08fce
eac4fdb
2ff5f50
d3dc280
2f85da1
c57f073
db8f7c0
2a8574f
e617146
103e071
9f0aae6
fbc2203
4b01a04
74690cc
f3673cb
398ce77
c737c6b
178a71e
f2ce925
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| import { ImporterParameterError } from "../model/exceptions/ImporterParameterError"; | ||
| import { Importer } from "./Importer"; | ||
| import { ImporterParameterDescription } from "./ImporterParameterDescription"; | ||
| const axios = require('axios'); | ||
|
|
||
| export class HttpImporter extends Importer { | ||
|
|
||
| //TODO RuntimeParameters type is probably wrong | ||
| parameters:ImporterParameterDescription[] = [new ImporterParameterDescription({name:"location", description:"String of the URI for the HTTP call", type:"string"}), | ||
| new ImporterParameterDescription({name:"encoding", description:"Encoding of the source. Available encodings: ISO-8859-1, US-ASCII, UTF-8", type:"string"}), | ||
| new ImporterParameterDescription({name:"defaultParameters", description:"Default values for open parameters in the URI", required:false, type:"RuntimeParameters"})] | ||
|
|
||
| // Override annotation is not necessary, but will be used for a better understanding of the code | ||
| override getType(): string { | ||
| return "HTTP"; | ||
| } | ||
|
|
||
| override getDescription(): string { | ||
| return "Plain HTTP"; | ||
| } | ||
|
|
||
| override getAvailableParameters(): ImporterParameterDescription[] { | ||
| return this.parameters; | ||
| } | ||
|
|
||
| override validateParameters(inputParameters: Map<string, unknown>): void { | ||
| super.validateParameters(inputParameters); | ||
| let encoding: string = inputParameters.get("encoding") as string; | ||
|
|
||
| // TODO CHECK IF ENCODING ARE WRITTEN CORRECT | ||
| if (encoding !== "ISO-8859-1" && encoding !== "US-ASCII" && encoding !== "UTF-8") { | ||
| throw new Error(this.getType() + " interpreter requires parameter encoding to have value " + | ||
| "ISO-8859-1" + ", " + | ||
| "US-ASCII" + ", " + | ||
| "UTF-8" | ||
| + ". Your given value " + encoding + " is invalid!"); | ||
| } | ||
| } | ||
| /** | ||
| protected void validateParameters(Map<String, Object> inputParameters) throws ImporterParameterException { | ||
| super.validateParameters(inputParameters); | ||
|
|
||
| String encoding = (String) inputParameters.get("encoding"); | ||
| if (!encoding.equals(StandardCharsets.ISO_8859_1.name()) && !encoding.equals(StandardCharsets.US_ASCII.name()) && !encoding.equals(StandardCharsets.UTF_8.name())) { | ||
| throw new IllegalArgumentException(getType() + " interpreter requires parameter encoding to have value " + | ||
| StandardCharsets.ISO_8859_1 + ", " + | ||
| StandardCharsets.US_ASCII + ", " + | ||
| StandardCharsets.UTF_8 | ||
| + ". Your given value " + encoding + " is invalid!"); | ||
| } | ||
| } | ||
| */ | ||
| override doFetch(parameters: Map<string, unknown>): string { | ||
| let uri = parameters.get("location") | ||
| let encoding = parameters.get("encoding") | ||
| // TODO see if encoding from response is good | ||
| axios({ | ||
| method: 'get', | ||
| url: uri, | ||
| responseEncoding: encoding | ||
| }).then(function (response: any) { | ||
| console.log(response.data) | ||
| return response.data | ||
| }).catch(function (error: any) { | ||
| console.error(error) | ||
| throw new ImporterParameterError("Could not Fetch from URI:" + uri) | ||
| }); | ||
| throw new ImporterParameterError("Could not Fetch from URI:" + uri) | ||
| } | ||
|
|
||
| /* | ||
|
|
||
| @Override | ||
| protected String doFetch(Map<String, Object> parameters) throws ImporterParameterException { | ||
| String location = parameters.get("location").toString(); | ||
| try { | ||
| URI uri = URI.create(location); | ||
| byte[] rawResponse = restTemplate.getForEntity(uri, byte[].class).getBody(); | ||
| return new String(rawResponse, Charset.forName((String) parameters.get("encoding"))); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new ImporterParameterException(e.getMessage()); | ||
| } | ||
| } | ||
| */ | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { ImporterParameterDescription } from "./ImporterParameterDescription"; | ||
| import { ImporterParameterError } from "../model/exceptions/ImporterParameterError"; | ||
|
|
||
| export abstract class Importer { | ||
| type: string | undefined; | ||
| description: string | undefined; | ||
|
|
||
| getRequiredParameters(): Array<ImporterParameterDescription> { | ||
| return this.getAvailableParameters().filter((item: any) => item.required) as Array<ImporterParameterDescription> | ||
| } | ||
|
|
||
| //@JsonProperty("parameters") | ||
| abstract getAvailableParameters() :Array<ImporterParameterDescription>; | ||
|
|
||
| fetch(parameters:Map<string, unknown> ): string { //throws ImporterParameterException | ||
| this.validateParameters(parameters); | ||
| return this.doFetch(parameters); | ||
| } | ||
|
|
||
| abstract getType(): string; | ||
| abstract getDescription(): string; | ||
|
|
||
| abstract doFetch(parameters: Map<string, unknown>): string; //throws ImporterParameterException | ||
|
|
||
| validateParameters(inputParameters: Map<string, unknown>) { //throws ImporterParameterException; | ||
|
|
||
| let illegalArguments: boolean = false; | ||
| let illegalArgumentsMessage: string = ""; | ||
|
|
||
| let possibleParameters: Array<ImporterParameterDescription> = this.getAvailableParameters(); | ||
| // TODO is that OK? | ||
| let unnecessaryArguments = Array.from(inputParameters.values()).filter((item: any) => possibleParameters.includes(item)) | ||
| if(unnecessaryArguments.length > 0){ | ||
| illegalArguments = true; | ||
| for(let argument of unnecessaryArguments){ | ||
| illegalArgumentsMessage += argument + " is not needed by importer \n" | ||
| } | ||
| } | ||
| for (let requiredParameter of this.getRequiredParameters()){ | ||
| if (inputParameters.get(requiredParameter.name) == null){ | ||
| illegalArguments = true; | ||
| illegalArgumentsMessage = illegalArgumentsMessage + this.type + "importer requires parameter " + requiredParameter.name + "\n"; | ||
| } | ||
| // TODO is that OK? | ||
| else if((inputParameters.get(requiredParameter.name) as any).constructor.name != requiredParameter.type){ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That looks weird. What are you trying to compare here? |
||
| illegalArguments = true; | ||
| illegalArgumentsMessage = illegalArgumentsMessage + this.type + " importer requires parameter " | ||
| + requiredParameter.name + " to be type " + (requiredParameter.type as string) + "\n"; | ||
| } | ||
| } | ||
| if(illegalArguments){ | ||
| throw new ImporterParameterError(illegalArgumentsMessage); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| export class ImporterParameterDescription { | ||
| name: string; | ||
| description: string; | ||
| required: boolean; | ||
|
|
||
| //TODO @Georg: need to check how to store the class in typescript, can use instance.constructor.name to get the name probably -> is stored as string | ||
| type: unknown; | ||
|
|
||
| constructor ({name,description,required=true,type}: {name:string, description:string, required?:boolean, type:unknown}) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| this.name = name; | ||
| this.description = description; | ||
| this.required = required; | ||
| this.type = type; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import {Interpreter} from "./Interpreter"; | ||
| import { InterpreterParameterDescription } from "./InterpreterParameterDescription"; | ||
| const csv=require('csvtojson') | ||
|
|
||
|
|
||
| export class CsvInterpreter extends Interpreter { | ||
|
|
||
| parameters: InterpreterParameterDescription[] = [new InterpreterParameterDescription("columnSeparator", "Column delimiter character, only one character supported", "string"), | ||
| new InterpreterParameterDescription("lineSeparator", "Line delimiter character, only \\r, \\r\\n, and \\n supported", "string",), | ||
| new InterpreterParameterDescription("skipFirstDataRow", "Skip first data row (after header)", "boolean"), | ||
| new InterpreterParameterDescription("firstRowAsHeader", "Interpret first row as header for columns", "boolean")] | ||
|
|
||
|
|
||
|
|
||
| override getType(): string { | ||
| return "CSV" | ||
| } | ||
| override getDescription(): string { | ||
| return "Interpret data as CSV data"; | ||
| } | ||
| override getAvailableParameters(): InterpreterParameterDescription[] { | ||
| return this.parameters; | ||
| } | ||
|
|
||
| override doInterpret(data: string, parameters: Map<string, unknown>): string { | ||
| let columnSeparator = (parameters.get("columnSeparator") as string).charAt(0) | ||
| let lineSeparator: string = parameters.get("lineSeparator") as string; | ||
| let firstRowAsHeader: boolean = parameters.get("firstRowAsHeader") as boolean; // True = With header, False = WithoutHeader | ||
| let skipFirstDataRow: boolean = parameters.get("skipFirstDataRow") as boolean; | ||
|
|
||
| let json: any[] = []; | ||
| let count = 0; | ||
| csv({ | ||
| noheader: !firstRowAsHeader, // Be Careful: Need to Invert the boolean here | ||
| output: "json", | ||
| delimiter: columnSeparator, | ||
| eol: lineSeparator | ||
| }) | ||
| .fromString(data) | ||
| .then((csvRow: any)=>{ | ||
| // Todo need to test if this works | ||
| if(skipFirstDataRow && ((count == 0 && !firstRowAsHeader) || (count == 1 && firstRowAsHeader))) { | ||
| // Skip First Row | ||
| } | ||
| else { | ||
| json.push(csvRow); | ||
| } | ||
|
|
||
| count = count+1; | ||
| }) | ||
|
|
||
| return JSON.stringify(json); | ||
| } | ||
|
|
||
| override validateParameters(inputParameters: Map<string, unknown>): void { | ||
| super.validateParameters(inputParameters); | ||
| let lineSeparator: string = inputParameters.get("lineSeparator") as string; | ||
|
|
||
| if (lineSeparator !== "\n" && lineSeparator !== "\r" && lineSeparator !== "\r\n") { | ||
| throw new Error(this.getType() + " interpreter requires parameter lineSeparator to have" + | ||
| " value \\n, \\r, or \\r\\n. Your given value " + lineSeparator + " is invalid!"); | ||
| } | ||
|
|
||
| var columnSeparator: string = inputParameters.get("columnSeparator") as string; | ||
| if (columnSeparator.length !== 1) { | ||
| throw new Error(this.getType() + " interpreter requires parameter columnSeparator to have" + | ||
| " length 1. Your given value " + columnSeparator + " is invalid!"); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { stringifiers } from "@jvalue/node-dry-basics"; | ||
| import { InterpreterParameterError } from "../model/exceptions/InterpreterParameterError"; | ||
| import { InterpreterParameterDescription } from "./InterpreterParameterDescription"; | ||
|
|
||
| export abstract class Interpreter { | ||
| type: string | undefined; | ||
| description: string | undefined; | ||
|
|
||
|
|
||
| interpret(data: string, parameters: Map<string, unknown>): string { | ||
| this.validateParameters(parameters); | ||
| return this.doInterpret(data, parameters); | ||
| } | ||
|
|
||
| abstract getType(): string; | ||
| abstract getDescription(): string; | ||
| abstract doInterpret(data: string, parameters: Map<string, unknown>): string | ||
| abstract getAvailableParameters(): Array<InterpreterParameterDescription>; | ||
|
|
||
| validateParameters(inputParameters: Map<string, unknown>) { | ||
| let illegalArguments: boolean = false; | ||
| let illegalArgumentsMessage: string = ""; | ||
|
|
||
| for (let requiredParameter of this.getAvailableParameters()){ | ||
| if (inputParameters.get(requiredParameter.name) == null){ | ||
| illegalArguments = true; | ||
| illegalArgumentsMessage = illegalArgumentsMessage + this.type + "interpreter requires parameter " + requiredParameter.name + "\n"; | ||
| } | ||
| // TODO is that OK? | ||
| else if((inputParameters.get(requiredParameter.name) as any).constructor.name != requiredParameter.type){ | ||
| illegalArguments = true; | ||
| illegalArgumentsMessage = illegalArgumentsMessage + this.type + " interpreter requires parameter " | ||
| + requiredParameter.name + " to be type " + (requiredParameter.type as string) + "\n"; | ||
| } | ||
| } | ||
| if(illegalArguments){ | ||
| throw new InterpreterParameterError(illegalArgumentsMessage); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| export class InterpreterParameterDescription{ | ||
|
|
||
| name: string; | ||
| description: string; | ||
| // TODO default value? is not used in Interpreters | ||
| required: boolean | undefined; | ||
| //TODO @Georg: need to check how to store the class in typescript, can use instance.constructor.name to get the name probably -> is stored as string -> Same as in Importer | ||
| type: unknown; | ||
|
|
||
| constructor (name: string, description: string, type: unknown) { | ||
| this.name = name; | ||
| this.description = description; | ||
| this.type = type; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import {Interpreter} from "./Interpreter"; | ||
| import { InterpreterParameterDescription } from "./InterpreterParameterDescription"; | ||
|
|
||
| export class JsonInterpreter extends Interpreter { | ||
|
|
||
| parameters: InterpreterParameterDescription[] = [] | ||
|
|
||
| override getType(): string { | ||
| return "JSON"; | ||
| } | ||
|
|
||
| override getDescription(): string { | ||
| return "Interpret data as JSON data"; | ||
| } | ||
|
|
||
| override getAvailableParameters(): InterpreterParameterDescription[] { | ||
| return this.parameters; | ||
| } | ||
|
|
||
| override doInterpret(data: string, parameters: Map<string, unknown>): string { | ||
| return JSON.parse(data); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import {Interpreter} from "./Interpreter"; | ||
| import { InterpreterParameterDescription } from "./InterpreterParameterDescription"; | ||
| const xml2js = require('xml2js'); | ||
|
|
||
| export class XmlInterpreter extends Interpreter{ | ||
|
|
||
| parameters: InterpreterParameterDescription[] = [] | ||
|
|
||
| override getType(): string { | ||
| return "XML"; | ||
| } | ||
|
|
||
| override getDescription(): string { | ||
| return "Interpret data as XML data"; | ||
| } | ||
|
|
||
| override getAvailableParameters(): InterpreterParameterDescription[] { | ||
| return this.parameters; | ||
| } | ||
|
|
||
| // TODO @Georg check if this package can be used.. | ||
| override doInterpret(data: string, parameters: Map<string, unknown>): string { | ||
| xml2js.parseString(data, (err: any, result: any) => { | ||
| if(err) { | ||
| throw err; | ||
| } | ||
|
|
||
| // `result` is a JavaScript object | ||
| // convert it to a JSON string | ||
| const json = JSON.stringify(result); | ||
| return json; | ||
| }); | ||
| throw Error("could not convert data into json"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative design idea:
Use an interface instead of a class for parameter descriptions (since no behavior, just plain data)
TypeScriptexport interface ImporterParamDescription {
name: string;
description: string;
type: string;
}