Skip to content

Conversation

@georg-schwarz
Copy link
Member

@georg-schwarz georg-schwarz commented May 25, 2022

Open points

  • Replace knex with our own postgres implementation
  • Code styling and clean-up (Linter)

DONT MERGE YET!

f3l1x98 and others added 19 commits January 13, 2022 10:39
* added endpoints basic

* adapterEndpoint added + utils + routes for all adapter endpoints with @PVahldiek

* service adapterendpoint added / model interpreter stub added / getallformats test implementation with @PVahldiek

* lines

* Test

* test

* revert

* added other service methods for adapterservice and models with @PVahldiek

* added formatconfig, format and protocol with @PVahldiek

* interpreter start with @PVahldiek

* added jackson-js package for jsonproperty annotations with @PVahldiek

* validator for first adapterendpoint method with @PVahldiek

* adapterconfigvalidator with @PVahldiek

* validator for protocolConfig with @MarcoDoell

* removed jackson-js with @MarcoDoell

* import ProtocolConfigValidator in adapterEndpoint with @MarcoDoell

* added interpreter implementation classes with @MarcoDoell

* added interpreter implementation classes with @MarcoDoell

* export classes

* emulate object enums with class since typescript does only support numeric or string based enums with @PVahldiek

* protocol class instead of enum with @PVahldiek

* refacotring code review with @PVahldiek

* get all formats and get all protocols + added importer classes

* importer classes with @PVahldiek

* protocol with @PVahldiek

* importer implement methods with @PVahldiek

* interpreter with @PVahldiek

* inherit methods from abstract class with @PVahldiek

* service with @PVahldiek

* Interpreter validateParameters() and InterpreterErrors with @MarcoDoell

* Importer validateParameters() and ImporterErrors with @MarcoDoell

* implementation for interpreter and importer with @MarcoDoell

* add first implementation of dataImportEndpoint

* refactoring of project structure with @PVahldiek

* override abstract methods in httpimporter with @PVahldiek

* httimporter parameter list with @PVahldiek

* added axios for http calls with @PVahldiek

* implemented doFtech and validateParameters in HttpImporter with @PVahldiek

* added abstract methods for interpreter + implement in subclasses with @PVahldiek

* added overrides with @PVahldiek

* added xml2js for decoding of xml string with @PVahldiek

* finished implementation of xmlinterpreter with @PVahldiek

* parameters for csv interpreter with @PVahldiek

* csvinterpreter implementation + added csvtojson package with @PVahldiek

* comment deleted

* finished csvinterpreter implementation with @PVahldiek

* implementation of adapterservice finished with @PVahldiek

* implementation of adapterservice finished with @PVahldiek

* comment

* add dataSourceManager skeleton

* first working impl of dataImportEndpoint

* add todo

* strange behaviour on npm not refreshing. Test on other machine

* add changes

* fix strange knex connection error with @MarcoDoell

* rename

* add todos and implement getDatasource

* knex import

* Fixed startup of docker container - make adapter service run with docker-compose - adjusted not running tests - disabled linter and some tests with @MarcoDoell

* Singleton (for export)

* Fixed integration test /formats with @MarcoDoell

* fixed integration-test for /protocols and started with /preview with @MarcoDoell

* weiterarbeit adapterservice /preview  with @PVahldiek

* add deleteDatasource

* complete add deleteDatasource

* complete put & updateDatasource

* start with implementation of trigger. Wait for fix of adapterService

* preview endpoint läuft jetzt with @ChristoffSchaub

* implementation of triggerworks after fix of adapterService

* previewraw endpoint fixed

* start refactoring

* refactoring state: different repositories for database calls

* refactoring state: split up dataImportEndpoint.ts and dataSourceEndpoint.ts

* refactoring state: remove helper methods from Endpoint classes

* refactoring state: finished first refactoring

* implement outboxing for update & addDatasource

* string parser with @PVahldiek

* package lock

* resolve promise

* test for fetch importer with @PVahldiek

* test for xml raw with @PVahldiek

* response type in axios with @PVahldiek

* importer and interpreter angepasst with @PVahldiek

* xml parser options with @PVahldiek

* test daten raus

* csv interpreter modified with @PVahldiek

* Anpassungen an integration-tests für import and format (json, xml, csv)

* package-lock with @MarcoDoell

* Adjusted for BAD REQUEST Tests

* const instead of let / var + docker compose port with @PVahldiek

* undefined + typo + constructor raus with @PVahldiek

* linter first fixes with @PVahldiek

* preview raw linter with @PVahldiek

* linter with @PVahldiek

* linter in datasource with @PVahldiek

* linter with @PVahldiek

* csv interpreter fixed with @PVahdliek

* linter with @PVahldiek

* linter + xmlinterpreter with @PVahldiek

* xmlInterpreter with new function for converting xml to record

* xml interpreter with @PVahldiek

* re-changed to json.parse for json.stringify decoding with @MarcoDoell

* additional test in stateless with @PVahldiek

* unterscheidung internal server error und bad request für httpimporter with @PVahldiek

* convertXmlToJson new function with @PVahldiek

* unit tests for adapter Endpoint with @PVahldiek

* JEST Unit Tests for JsonInterpreter with @PVahldiek

* added csv interpreter jest file with @PVahldiek

* test push

* jest unit test for xmlinterpreter with @MarcoDoell

* jest tests formatted and xmlinterpreter unit tests with @MarcoDoell

* CSV Interpreter Unit test with @MarcoDoell

* linter finished for adapter with @PVahldiek

* folder structure for tests with @PVahldiek

* firstRowHeader fixed in CSV Format with @PVahldiek

* line seperator for test in csv interpreter with @PVahldiek

* further tests for xmlinterpreter with @PVahldiek

* jest Unit Tests for HTTP Importer with @PVahldiek

* further tests for validateParameters in HTTP Importer with @PVahldiek

* added fast xml parser with @PVahldiek

* new xmlparser library + manuelle löschung vom root element with @PVahldiek

* unnötige dependencies raus with @PVahldiek

* additional test for csv interpreter + validateparameters changed with @PVahldiek

* validateParameters in CSV Interpreter angepasst - weitere Tests für Interpreter with @PVahldiek

* adapted tests for new xml parser with @PVahldiek

* interpreter when there are no parameters present with @PVahldiek

* interpreter bug fix with @PVahldiek

* fixed integration test with "root" element with @MarcoDoell

* dataSourceEndpoint.ts - publish deletion to outbox repository

* todo raus with @PVahldiek

* method descriptions for adapterService with @PVahldiek

* Method descriptions for configs with @MarcoDoell

* More method declarations with @MarcoDoell

* #add amqp consumer, publish all deletions of datasources

* #add parameters to triggerDataimport

* #trigger dataimport

* #replace most hardcoded parts with env variables to run integration tests

* AMQP Connection with Docker with @ChristoffSchaub

* #test script for setting env variables to let nodejs run locally

* #set env for local testing

* #correct config for local testing

* Create Table Datasource if not exists with @ChristoffSchaub

* # add database creation statements

* fixed Postgres Connection with @ChristoffSchaub

* # should reject datasources with specified id

* # Should create datasources [POST /datasources] with @PVahldiek

* # Should reject @PVahldiek

* Validator for datasource-configs with @MarcoDoell and @ChristoffSchaub

* linter for datasource with @PVahldiek

* linter + multiple bugfixes in dataimportendpoint: check for null / undefined values in return types with @PVahldiek

* linter with @PVahldiek

* autolinter

* linter

* linter with @PVahldiek

* More Datasource integration tests with @MarcoDoell

* return 404 for deleting unknown datasource with @MarcoDoell

* fixed for delete all datasources

* fix for GET /datasources with @MarcoDoell

* fix for PUT integration test with @MarcoDoell

* change routing key of event consuming in datasourceAmqpConsumer

* refactor triggerDataImportForDatasource

* refactor triggerDataImportForDatasource and implement amqpConsumer for triggering

* refactor service location

* make methods in dataImportTriggerService.ts private

* implement adapterConfigWithoutRuntimeParameters

* add errorHandling to triggerImport

* small refactoring

* automatic increment of id in dataimport table with @PVahldiek

* push to test outboxRepository.ts

* push to test outboxRepository.ts

* push to test outboxRepository.ts

* @PVahldiek @MarcoDoell fix runtimeparameter

* @PVahldiek @MarcoDoell createDataImportFromResult

* @PVahldiek @MarcoDoell createDataImportFromResult

* @PVahldiek @MarcoDoell createDataImportFromResult

* npm doesnt update sources anymore

* dist ordnerstruktur angepasst + version types/jest

* fix for location in triggerImport with @ChristoffSchaub and @MarcoDoell

* lock

* Default/runtimeparameters in trigger datasource with @MarcoDoell, @ChristoffSchaub

* get data with parameters with @MarcoDoell

* bugfix in adapter with @PVahldiek

* integration tests with @PVahldiek

* revert with @PVahldiek

* integration tests fixing + small bugfixes in datasource with @PVahldiek

* amqp producer helper endpoint with @PVahldiek

* Todo with @MarcoDoell

* added runtimeparameter to dataimport get with @PVahldiek

* bugfix default parameters in dataimport with @PVahldiek

* dataimport repo undefined check

* default parameters with @PVahldiek

* trigger with default parameters with @PVahldiek

* dataimport id with @PVahldiek

* triggercount added with @PVahldiek

* id with @PVahldiek

* TriggerCount with @MarcoDoell

* #errorhandling with @PVahldiek for amqp and rest

* Error handling for AMQP trigger with @ChristoffSchaub

* #errorhandling with @PVahldiek for rest

* Errorhandling + data format with @MarcoDoell and @ChristoffSchaub

* linter with @MarcoDoell

* lint with @MarcoDoell

* lint with @MarcoDoell

* lint with @MarcoDoell

* lint with @MarcoDoell

* linter with @MarcoDoell

* fixes for run container with @MarcoDoell

* triggercount bugfix

* bugfix and integration tests

Co-authored-by: Pascal Vahldiek <pascal.vahldiek@fau.de>
Co-authored-by: Christoff <christoff.schaub@t-online.de>
Co-authored-by: ChristoffSchaub <christoff.schaub@gmail.com>
Co-authored-by: = <=>
- Rewrote Endpoints and Services using DBLayer
- Added/Corrected types in interpreter impl
- Added/Corrected types of methods etc
- Extended interfaces
- Fixed parsing and stringifing of JSONs
For tests:
- Linter fixed line breaks
- Fixed last linter errors
- Merged the two getAdapterConfig methods
- Specified 'DatasourceDTO.protocal.parameter' in order to avoid having to cast when accessing 'defaultParameter'
@georg-schwarz
Copy link
Member Author

// The old impl retrieved data as byte array and then converted using encoding:
// Return new String(rawResponse, Charset.forName((String) parameters.get("encoding")));
// Unfortunately there does not seem to be a universal method .toString(encoding?: string) in javascript
const result = await axios
.get(uri, { responseEncoding: encoding })
.catch((error: AxiosError) => {
if (error.response) {
console.log(error.response);
throw new Error('Could not Fetch from URI:' + uri);
}
throw new ImporterParameterError('Could not Fetch from URI:' + uri);
});
// Check if data is object/array -> return stringified (because this method returns string)
if (typeof result.data === 'object' || Array.isArray(result.data)) {
return JSON.stringify(result.data);
}
return result.data as string;

For this one I wanted to ask, whether somebody knows a better way, because I also do not know one.
The old Java impl got the response as a byte[] and then created a new String from it using the passed encoding:

URI uri = URI.create(location);
byte[] rawResponse = restTemplate.getForEntity(uri, byte[].class).getBody();
return new String(rawResponse, Charset.forName((String) parameters.get("encoding")));

I think you would need to use fetch API instead of Axios... There you can get the byte stream and interpret based on encoding.

@georg-schwarz
Copy link
Member Author

This is the part that was rewritten from the old impl and I have kept it. Not sure if we want to rewrite it again to match old Java impl.

// This is 'new' solution (instead of overriding url -> override params here and url in importer)
datasource.protocol.parameters.defaultParameters = replacementParameters;
const parameters = {
...datasource.protocol.parameters,
};

// Originally this was done in dataImportTrigger during creation of AdapterConfig
// -> RuntimeParameter are used, because the defaultParams are overriden during AdapterConfig instead
if (parameters.defaultParameters !== undefined) {
const defaultParameters = parameters.defaultParameters as Record<
string,
unknown
>;
const keys = Object.keys(defaultParameters);
for (const entry of keys) {
const value = defaultParameters[entry] as string;
const regex = new RegExp('{' + entry + '}', 'g');
uri = uri.replace(regex, value);
}
}

I think the old solution was better. The importer should not know that there are differences in parameters but just blindly execute what it gets as input.

@georg-schwarz georg-schwarz force-pushed the adapter-nodejs-refactoring branch 7 times, most recently from 222d4bb to 1583029 Compare August 22, 2022 15:52
@georg-schwarz georg-schwarz force-pushed the adapter-nodejs-refactoring branch from 1583029 to d3833fc Compare August 22, 2022 15:55
@georg-schwarz
Copy link
Member Author

@f3l1x98 I refactored everything except the two open points I replied to. Would be nice if you could take care of them.
CI is currently not working somehow for Adapter integration tests.. doesn't get the exit code somehow.

@f3l1x98
Copy link
Contributor

f3l1x98 commented Aug 23, 2022

// The old impl retrieved data as byte array and then converted using encoding:
// Return new String(rawResponse, Charset.forName((String) parameters.get("encoding")));
// Unfortunately there does not seem to be a universal method .toString(encoding?: string) in javascript
const result = await axios
.get(uri, { responseEncoding: encoding })
.catch((error: AxiosError) => {
if (error.response) {
console.log(error.response);
throw new Error('Could not Fetch from URI:' + uri);
}
throw new ImporterParameterError('Could not Fetch from URI:' + uri);
});
// Check if data is object/array -> return stringified (because this method returns string)
if (typeof result.data === 'object' || Array.isArray(result.data)) {
return JSON.stringify(result.data);
}
return result.data as string;

For this one I wanted to ask, whether somebody knows a better way, because I also do not know one.
The old Java impl got the response as a byte[] and then created a new String from it using the passed encoding:

URI uri = URI.create(location);
byte[] rawResponse = restTemplate.getForEntity(uri, byte[].class).getBody();
return new String(rawResponse, Charset.forName((String) parameters.get("encoding")));

I think you would need to use fetch API instead of Axios... There you can get the byte stream and interpret based on encoding.

This is not quite possible, because the native fetch api is not available in Node.JS before version 18.0.0.
But I have found a way in which axios returns its data as an ArrayBuffer so I have choosen to use this together with the TextDecoder for converting to string:

const decoder = new TextDecoder(encoding);
// Fetch as ArrayBuffer
const result = await axios
.get(uri, { responseType: 'arraybuffer' })
.catch((error: AxiosError) => {
if (error.response) {
console.log(error.response);
throw new Error('Could not Fetch from URI:' + uri);
}
throw new ImporterParameterError('Could not Fetch from URI:' + uri);
});
// Convert ArrayBuffer response to string using encoding
return decoder.decode(result.data);

@f3l1x98
Copy link
Contributor

f3l1x98 commented Aug 23, 2022

@georg-schwarz I have changed it, but the first issue I have solved differently than proposed, so please have a look at the comment above: #408 (comment)
The other issue has been solved:

// Fill queryParameters in url
const parameters = this.fillQueryParameters(datasource, runtimeParameters);

@georg-schwarz
Copy link
Member Author

LGTM!

Do you want to tackle the CI issue as well? I can also do it but will get to it end of week soonest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants