Added two batch job tests in the restaurant sample #442

sroeger · 2016-07-04T12:10:30Z

This PR adds:

OfferImportJobTest with XML batch configuration. A file offers.csv is read and processed to save valid records to the database. This job utilizes an ItemProcessor in order to import data types "Money" and "OfferState" (enum). In this example this is done via an additional OfferCsv entity (see the example below for an alternative to this). This example also demonstrates error or skip handling with wrongly formatted input lines. An attached SkipListener reports skip occuraces in the log WARN console.
StaffImportJobTest with exemplary Java configuration. A bigger file with 2000 entries is read and processed to save valid records to the database. This example utilizes a CustomEditor "RoleEditor" to import the data type "Role" which is an enum value. In Java configuration it is also possible to define skip handling with badly formatted lines. Also it is possible to use variables defined in the application.properties file in the configuration.

@maihacke can you review the changes?

oasp-ci · 2016-07-04T12:13:49Z

Can one of the admins verify this patch?

maybeec · 2016-07-29T05:05:20Z

...core/src/main/java/io/oasp/gastronomy/restaurant/general/configuration/BeansBatchConfig.java

 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.DependsOn;
 import org.springframework.transaction.PlatformTransactionManager;

 /**
+ * This class contains the configuration like jobLauncher,Jobrepository etc.
+ * @author ssarmoka
+ */


duplicated javadoc?

please consolidate with class javadoc. This doc is currently connected to an import, which is obviously a mistake.

maybeec · 2016-07-29T05:11:06Z

@maihacke can you have a look at this PR as you have been strongly involved into the batch design?

sroeger · 2016-07-29T13:03:43Z

btw @maybeec is it recommended to merge current changes of oasp4j/develop into PRs when they are outdated? This branch has some conflicts now that I could resolve with a merge. Is there a general best practice?
I could not find any information about this in the wiki.

maybeec · 2016-07-29T17:30:19Z

For updating pull requests, there is basically just one rule:

If you can resolve the merge without guessing what might be the right solution. Go ahead!
But if you are not totally sure about it, get in contact or let it simply be done by someone who has the backgrounds of the changes done on the main repository. :)

Von: Simon Röger [email protected]
Gesendet: 29.07.2016 6:34 nachm.
An: oasp/oasp4j
Cc: Brunnlieb, Malte; Mention
Betreff: Re: [oasp/oasp4j] Added two batch job tests in the restaurant sample (#442)

btw @maybeechttps://github.com/maybeec is it recommended to merge current changes of oasp4j/develop into PRs when they are outdated? This branch has some conflicts now that I could resolve with a merge. Is there a general best practice?
I could not find any information about this in the wiki.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/442#issuecomment-236174578, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABXHNwm4r3cSuLEYPZi9LTRYRhvdEUPXks5qafowgaJpZM4JEUIV.

Firma: Capgemini Deutschland GmbH
Aufsichtsratsvorsitzender: Antonio Schnieder • Geschäftsführer: Dr. Michael Schulte (Sprecher) • Jost Förster • Dr. Peter Lempp • Dr. Volkmar Varnhagen

Amtsgericht Berlin-Charlottenburg, HRB 98814
This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.

maybeec · 2016-08-01T07:12:48Z

...p/gastronomy/restaurant/staffmanagement/batch/impl/staffimport/writer/StaffMemberWriter.java

+  /**
+   * {@inheritDoc}
+   */
+  @Override


remove unnecessary @inheritdoc javaDoc comments. They are simply not needed as @OverRide is sufficient for the IDE.

sroeger · 2016-08-01T09:13:36Z

Thanks for the comments.
It should also be discussed which way is best practice when importing data with custom type fields or enums as they are not automatically converted from strings.
I proposed two ways as mentioned in my first post

use a converter in the batch job that goes the way from the import csv string to an OfferCsv to the OfferEto as done in the offer import or
use a custom editor that is defined in the job configuration (this means one has to define a class extending PropertyEditorSupport) as done in the staff import.

maihacke · 2016-08-09T07:45:46Z

...java/io/oasp/gastronomy/restaurant/general/configuration/CustomBeanFactoryPostProcessor.java

+  @Override
+  public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
+
+    beanFactory.registerScope("step", new StepScope());


muss dieser Scope wirklich so manuell registriert werden? Gibt es da keinen eleganteren Weg?

For the two ways of ENUM-Mapping. I think both ways are eligible. It's more or less a matter of taste and I don't think we need standardize this in OASP. Just leave it as it is...

Prinzipiell nein, man könnte den ganzen Stack an Einstellungen und Konfigurationen über @EnableBatchProcessing beziehen. Ich hab angenommen, wir machen das absichtlich alles manuell, um mehr Kontrolle zu haben. Oder wäre es im Zuge der Umstellung auf Spring Boot auch besser, alles über die Annotation zu laden?

Just for remark. Ticket for migration to java config created by @sroeger #472

maihacke · 2016-08-09T08:17:03Z

Pull requests looks verý good overall. There are some minor issues/questions left. After "fixing" these, PR can be merged...

maybeec · 2016-08-09T16:24:07Z

Please stick to english language :)

sroeger · 2016-08-29T06:23:48Z

The two concerns regarding

java config vs. xml config (see Batch change xml-config to java config #472)
code-style changes (see Check oasp4j code formatting #454)

are documented in the given issues. Other than these issues the only remaining point for this PR is the registering of the scope step. Imho the only other (maybe more elegant) option is to use Spring Boot's annotation @EnableBatchProcessing but this would change many of our configurations. Maybe someone can think of another solution for CustomBeanFactoryPostProcessor registered in
BeansBatchConfig

kiran-vadla · 2017-01-04T09:23:53Z

@maihacke , As discussed , assigned it to you for the review . Thanks for your time. Just to keep you posted , this has to be merged onto develop-2.3.0 as this is the branch that is used for oasp4j 2.3.0 release.

Thanks,
Kiran.

maihacke

Looks good. Questions were answered. No required changes since last review.

kiran-vadla · 2017-01-06T10:44:40Z

@maihacke , Thanks for your time for the review . As discussed , I will take care of the merging.

kiran-vadla · 2017-01-10T05:53:19Z

Changes submitted as part of this PR are submitted on to a branch under develop-2.3.0 and PR #532 is created . Refer PR #532 . Closing this PR.

Röger added 10 commits June 7, 2016 11:08

newchanges

49312b6

mychange

298c35b

dailysave

fb1a606

090616

324327a

090616

b05527e

usage of builder by cobigen

867a1ad

added functionality by accessing application.properties in batch job

f7bb804

added several assertThat's for StaffImportJobTest

2f15a9a

added skip functionality to csv

f10ecf2

Fixed merge conflicts

f77126b

Röger added 4 commits July 4, 2016 14:36

removed unused javaconfig, try to fix travis build

74833e8

try to fix travis build 2nd try

1c004c5

trying to fix travs 3rd try - surpressing INFO log in StaffImportJobTest

c70bd00

4th try to fix travis build - error introduced by not refactoring in xml

728e92b

sroeger changed the title ~~Added two batch job tests in the restaurant samle~~ Added two batch job tests in the restaurant sample Jul 12, 2016

maybeec added enhancement test batch labels Jul 28, 2016

maybeec added this to the oasp:2.2.0 milestone Jul 28, 2016

maybeec reviewed Jul 29, 2016
View reviewed changes

maybeec self-assigned this Aug 1, 2016

maybeec reviewed Aug 1, 2016
View reviewed changes

maybeec removed their assignment Aug 1, 2016

Röger added 2 commits August 1, 2016 10:45

Changes demanded in PR comments

b6ebc17

fixed merge conflict

0ccb804

removed inheritDoc

599b845

maihacke reviewed Aug 9, 2016
View reviewed changes

sroeger mentioned this pull request Aug 9, 2016

Batch change xml-config to java config #472

Open

jomora modified the milestones: oasp:2.3.0, oasp:2.2.0 Nov 30, 2016

kiran-vadla requested a review from maihacke January 4, 2017 09:20

kiran-vadla assigned maihacke Jan 4, 2017

maihacke approved these changes Jan 6, 2017

View reviewed changes

kiran-vadla self-assigned this Jan 6, 2017

kiran-vadla mentioned this pull request Jan 9, 2017

Push changes submittted in PR#442 to a new branch under develop-2.3.0 #532

Closed

kiran-vadla closed this Jan 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added two batch job tests in the restaurant sample #442

Added two batch job tests in the restaurant sample #442

sroeger commented Jul 4, 2016 •

edited

Loading

oasp-ci commented Jul 4, 2016

maybeec Jul 29, 2016

maybeec Aug 1, 2016

maybeec commented Jul 29, 2016

sroeger commented Jul 29, 2016

maybeec commented Jul 29, 2016

maybeec Aug 1, 2016

sroeger commented Aug 1, 2016

maihacke Aug 9, 2016 •

edited

Loading

maihacke Aug 9, 2016 •

edited

Loading

sroeger Aug 9, 2016

maihacke Jan 6, 2017

maihacke commented Aug 9, 2016

maybeec commented Aug 9, 2016

sroeger commented Aug 29, 2016 •

edited

Loading

kiran-vadla commented Jan 4, 2017

maihacke left a comment

kiran-vadla commented Jan 6, 2017

kiran-vadla commented Jan 10, 2017

Added two batch job tests in the restaurant sample #442

Added two batch job tests in the restaurant sample #442

Conversation

sroeger commented Jul 4, 2016 • edited Loading

oasp-ci commented Jul 4, 2016

maybeec Jul 29, 2016

Choose a reason for hiding this comment

maybeec Aug 1, 2016

Choose a reason for hiding this comment

maybeec commented Jul 29, 2016

sroeger commented Jul 29, 2016

maybeec commented Jul 29, 2016

maybeec Aug 1, 2016

Choose a reason for hiding this comment

sroeger commented Aug 1, 2016

maihacke Aug 9, 2016 • edited Loading

Choose a reason for hiding this comment

maihacke Aug 9, 2016 • edited Loading

Choose a reason for hiding this comment

sroeger Aug 9, 2016

Choose a reason for hiding this comment

maihacke Jan 6, 2017

Choose a reason for hiding this comment

maihacke commented Aug 9, 2016

maybeec commented Aug 9, 2016

sroeger commented Aug 29, 2016 • edited Loading

kiran-vadla commented Jan 4, 2017

maihacke left a comment

Choose a reason for hiding this comment

kiran-vadla commented Jan 6, 2017

kiran-vadla commented Jan 10, 2017

sroeger commented Jul 4, 2016 •

edited

Loading

maihacke Aug 9, 2016 •

edited

Loading

maihacke Aug 9, 2016 •

edited

Loading

sroeger commented Aug 29, 2016 •

edited

Loading