Added conversation import web service #37

ruKurz · 2017-08-04T14:12:32Z

This commit adds an importer web service to the Smarti Backend application.
This implementation is a first draft and not yet final. Any comments are appreciated.

solves #36

Usage:

1. Importing a JSON file that has been exported from a Rocket.Chat MongoDB

curl -X GET "http://localhost:8080/import/test.assistify.de/rocketChatJSONFile" -H "accept: application/json" -H "content-type: application/json" -d "{ \"json_file_url\": \"file:///Users/rudiger/projects/smarti/application/src/test/resources/export-sapsus-faqs.json\"}"

The file import can handle Rocket.Chat MongoDB exported JSON that can be crated like this:

db.rocketchat_room.aggregate([
    { $match: { "name": { $regex : "faq" } } }, 
    { $lookup: {
        from: "rocketchat_message",
        localField: "_id",
        foreignField: "rid",
        as: "messages"
    } }
])

2. Importing direct from RocketChat MongoDB

curl -X GET "http://localhost:8080/import/test.localhost/rocketChatMongoDB" -H "accept: application/json" -H "content-type: application/json" -d "{ \"dbname\": \"rocketchat\", \"filter_field\": \"expertise\", \"filter_value\": \"assistify\", \"host\": \"localhost\", \"message_collection\": \"rocketchat_message\", \"port\": 27017, \"room_collection\": \"rocketchat_room\"}"

3. Importing from Rocket.Chat Webservice
The idea is to use the REST API of Rocket.Chat to get conversations that will be than imported into Smarti. In the current commit this importer is not finished yet.

4. Importing from Slack or other chat platforms

5. Importing from E-Mail server (Pop/Imap)

6. Importing via web crawler, e.g. FAQ-websites

- initial check-in

- correct version in pom files

…Kurz/smarti into feature/json-message-importer

- correct version in pom files

tkurz · 2017-08-07T14:57:28Z

application/src/main/java/io/redlink/smarti/services/ImporterService.java

+	private static final DateFormat ISODateFormatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");
+
+    @Autowired
+    private ConversationWebservice conversationWebservice;


Do not use Webservices in a Service. Use the conversationService instead.

tkurz · 2017-08-07T15:18:34Z

...n/src/main/java/io/redlink/smarti/services/importer/ConversationSourceRocketChatMongoDB.java

+			StringBuffer input = new StringBuffer();
+			input.append(I_ImporterService.JSON_RESULT_WRAPPER_START);
+
+			while (iterator.hasNext()) {


Could be more elegant with guava, java 8 ..., like e.g.:
ImmutableList.copyOf(iterator).stream().map(Document::toJson).collect(Collectors.joining(","));
Or you could use JacksonMapper (as the Document implements map) for the whole thing like:

ObjectMapper mapper = new ObjectMapper(); return mapper.writeValueAsString(Collections.singletonMap("export",ImmutableList.copyOf(iterator)));

(untested code !;)

tkurz · 2017-08-08T08:41:37Z

application/src/main/java/io/redlink/smarti/services/I_ImporterService.java

+import io.redlink.smarti.services.importer.I_ConversationSource;
+import io.redlink.smarti.services.importer.I_ConversationTarget;
+
+public interface I_ImporterService {


We should find a common way of classnaming. In other projects we used to use NAME for interfaces and NAMEImpl for implementations. But I like this approach. But at least we should bring everything to a one naming scheme.

I would put the Interface in another package (e.g. api)

tkurz · 2017-08-08T08:44:53Z

application/src/main/java/io/redlink/smarti/services/ImporterService.java

+    private RocketChatEndpoint rcEndpoint;
+
+    public ImporterService() {
+    		rcEndpoint = new RocketChatEndpoint(null, 0, null);


Use spring to manage your service beans. The best way is:

private RocketChatEndpoint rcEndpoint; public ImporterService(@Autowired RocketChatEndpoint rcEndpoint) { this.rcEndpoint = rcEndpoint; }

tkurz · 2017-08-08T08:47:32Z

application/src/main/java/io/redlink/smarti/services/ImporterService.java

+			log.info("Start export at    : " + (new Date().getTime() - time) + " ms");
+			String export = source.exportConversations();
+			log.info("Start parsing at   : " + (new Date().getTime() - time) + " ms");
+			JSONObject eportedJSON = (JSONObject) new JSONParser().parse(export);


Why source.exportConversation does not create an Object directly?

The idea was to avoid duplicated code, since
JSONObject eportedJSON = (JSONObject) new JSONParser().parse(export);
would be the same in each Source implementation.

Could you please provide some arguments why you'll expect the Source itself should create the Object?

tkurz · 2017-08-08T09:00:07Z

application/src/main/java/io/redlink/smarti/services/importer/I_ConversationSource.java

+	 * 
+	 * @throws Exception if something goes wrong
+	 */
+	String exportConversations() throws Exception;


Could return an object (as already mentioned)

tkurz · 2017-08-08T09:02:23Z

application/src/main/java/io/redlink/smarti/webservice/ImporterServiceEndpoint.java

+    	ObjectMapper mapper = new ObjectMapper();
+
+		try {
+	    		E_SourceType type = E_SourceType.valueOf(sourceType);


Could be done nicer with a factory

tkurz · 2017-08-08T09:08:47Z

application/src/main/java/io/redlink/smarti/webservice/pojo/A_SourceConfiguration.java

+	public JSONObject asJSON() {
+
+		try {
+			return (JSONObject) new JSONParser().parse(new ObjectMapper().writeValueAsString(this));


Why do you need this? As far as I can see you always use the toJsonString method on things produces by this method. So a direct serialization with ObjectMapper is more straight forward.

tkurz · 2017-08-08T09:09:37Z

application/src/main/java/io/redlink/smarti/webservice/pojo/RocketMongoConfig.java

+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
+
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonPropertyOrder({


Why the order of properties matters?

tkurz · 2017-08-08T09:14:01Z

application/src/main/test/io/redlink/smarti/webservice/ConversationImporterTest.java

+    }
+
+	@Test
+	public void testJSONFile() throws ClientProtocolException, IOException, Exception {


There is not test right ;) So no assertions oa. If you want to do a nearly full integration which is still testable you could mock the conversation repository. Or test the units and mock the services. e.g. with Mockito.

tkurz · 2017-08-08T09:18:28Z

One thing is absolutely necessary to change: There are services which uses webservices (which is not a good style) which itself use services again. You should use services directly. Some things can be more elegant. The import interface could be rethought.

- corrections from review - Do not use Webservices in a Service. - Use spring to manage your service beans. - Just use the ConversationService and Message object. - Use JacksonMapper / direct serialization with ObjectMapper - Removed order from JSON/Beans

ruKurz · 2017-08-29T05:24:14Z

What's missing here, let me summarize:

Major implementation issues

for capabilities of mass imports, a 'MessageStream' should be implemented
the implementation over the Rocket.Chat API is incomplete

Minor implementation issues

the creation of the specific 'source' exporter should be done by a factory
the 'export' should produce some 'Object' instead a 'String'
the generalized import function is superfluous

Missing documentation and tests

API /docs for external usage
JavaDoc, especially for interfaces
Meaningful tests should be implemented

Overall comment

Naming of interfaces, class names and packaging as well as
code style has not been defined yet.

A good importer ....

The code for export as well as a scheduler should not be part of the Smarti project.
E.g. the knowledge about how to export Rocket.Chat via API is not part of Smart's core.
Maybe it's is a good idea to just implement the importer API and leave it open how to get an export.
This importer should be capable for a generic message streams following different schemas and support a mapping.

A interface could look like:

package io.redlink.smarti.services;

public interface I_Importer {

  /**
   * Imports messages from a message input stream.<p>
   *
   * Provides capability to addapt/import a conversation source to Smarti.
   * Smarti does not need to know the message's structure of the source.
   * A maaping tells how the source message fromat can be transfered.
   *
   * @param input streams the messages from an external conversaion source
   * @param schema descripes the structure of a single message of the source
   * @param mapping maps the external message format to Smarti message format
   *
   * @return the report of the import: Success / Fail / Summary / Loginfos, ...
   */
  Report import (MessageStream input, MessageSchema schema, MessageMapping mapping);
}

Implementing such an interface would allow to externalize the knowledge of exporting different source systems into third party libs. What would keep the code of Smarti clean!

ruKurz · 2017-09-27T06:50:14Z

@tkurz did you have a chance to have a closer look into this PR?

Thomas Kurz and others added 8 commits July 26, 2017 12:12

[maven-release-plugin] prepare release smarti-0.2.0-RC1

dc97914

[maven-release-plugin] prepare for next development iteration

66d4969

Merge branch 'develop' into release-0.2.0

42378dd

[maven-release-plugin] prepare release smarti-0.2.0-RC2

8103672

redlink-gmbh#36 - Add a web service that provides conversation imports

1a48dec

- initial check-in

redlink-gmbh#36 - Add a web service that provides conversation imports

26dcc5e

- correct version in pom files

Merge branch 'feature/json-message-importer' of https://github.com/ru…

41ad44e

…Kurz/smarti into feature/json-message-importer

redlink-gmbh#36 - Add a web service that provides conversation imports

03d7554

- correct version in pom files

ghost assigned ruKurz Aug 4, 2017

ghost added the in progress label Aug 4, 2017

ruKurz added enhancement api labels Aug 4, 2017

tkurz self-requested a review August 7, 2017 09:46

tkurz suggested changes Aug 8, 2017

View reviewed changes

tkurz added this to the Delta-Sprint milestone Sep 7, 2017

ja-fra removed this from the v0.6.0 milestone Nov 27, 2017

ruKurz removed the in progress label Feb 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added conversation import web service #37

Added conversation import web service #37

ruKurz commented Aug 4, 2017 •

edited

Loading

tkurz Aug 7, 2017

tkurz Aug 7, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

ruKurz Aug 9, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz Aug 8, 2017

tkurz commented Aug 8, 2017

ruKurz commented Aug 29, 2017 •

edited

Loading

ruKurz commented Sep 27, 2017

Added conversation import web service #37

Are you sure you want to change the base?

Added conversation import web service #37

Conversation

ruKurz commented Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkurz commented Aug 8, 2017

ruKurz commented Aug 29, 2017 • edited Loading

ruKurz commented Sep 27, 2017

ruKurz commented Aug 4, 2017 •

edited

Loading

ruKurz commented Aug 29, 2017 •

edited

Loading