Merge pull request #85 from vjuranek/PERFREPO-273

[PERFREPO-273] Update value instead of recreating it

Author: pmacik

README.md

@@ -123,7 +123,14 @@ Also you have to add test datasource `PerfRepoTestDS`into the WildFly, ideally p
                 </datasource>
 ```
 
-
-
-
-
+For testing session bean, authentication is require and thus you have to also add appropriate security domain:
+```xml
+                <security-domain name="Arquillian-Testing" cache-type="default">
+                    <authentication>
+                        <login-module code="UsersRoles" flag="required">
+                            <module-option name="usersProperties" value="users.properties"/>
+                            <module-option name="rolesProperties" value="roles.properties"/>
+                        </login-module>
+                    </authentication>
+                </security-domain>
+```

web/pom.xml

@@ -294,6 +294,26 @@
                </execution>
             </executions>
          </plugin>
+         
+         <!-- for tests -->
+         <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>copy-test-libs</id>
+                        <phase>process-test-resources</phase>
+                        <configuration>
+                            <includeScope>test</includeScope>
+                            <outputDirectory>${project.build.directory}/test-libs</outputDirectory>
+                            <stripVersion>true</stripVersion>
+                        </configuration>
+                        <goals>
+                            <goal>copy-dependencies</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
 
       </plugins>
 

web/src/main/java/org/perfrepo/web/service/TestServiceBean.java

@@ -860,12 +860,9 @@ private void validateTestExecution(TestExecution testExecution) throws ServiceEx
     * @throws ServiceException
     */
    private void updateValues(TestExecution freshTestExecution, TestExecution updatedTestExecution) throws ServiceException {
-      freshTestExecution.getValues().stream().forEach(valueDAO::remove);
-      freshTestExecution.setValues(new ArrayList<>());
-
       for (Value value: updatedTestExecution.getValues()) {
          value.setTestExecution(freshTestExecution);
-         addValue(value);
+         updateValue(value);
       }
    }
 

web/src/test/java/org/perfrepo/test/JBossLoginContextFactory.java

@@ -111,7 +111,7 @@ private AppConfigurationEntry createClientLoginModuleConfigEntry() {
     * @return the configured LoginContext.
     */
    public static LoginContext createLoginContext(final String username, final String password) throws LoginException {
-      final String configurationName = "Arquillian Testing";
+      final String configurationName = "Arquillian-Testing";
 
       CallbackHandler cbh = new JBossLoginContextFactory.NamePasswordCallbackHandler(username, password);
       Configuration config = new JBossJaasConfiguration(configurationName);

web/src/test/java/org/perfrepo/test/TestServiceBeanTest.java

@@ -14,6 +14,22 @@
  */
 package org.perfrepo.test;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.security.PrivilegedAction;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Date;
+import java.util.concurrent.Callable;
+
+import javax.ejb.EJBException;
+import javax.inject.Inject;
+import javax.security.auth.Subject;
+import javax.security.auth.login.LoginContext;
+
 import org.apache.log4j.Logger;
 import org.jboss.arquillian.container.test.api.Deployment;
 import org.jboss.arquillian.junit.Arquillian;
@@ -23,55 +39,55 @@
 import org.jboss.shrinkwrap.api.asset.EmptyAsset;
 import org.jboss.shrinkwrap.api.spec.WebArchive;
 import org.junit.After;
-import org.junit.Ignore;
 import org.junit.runner.RunWith;
+import org.perfrepo.model.Alert;
 import org.perfrepo.model.Metric;
 import org.perfrepo.model.MetricComparator;
 import org.perfrepo.model.Test;
+import org.perfrepo.model.TestExecution;
+import org.perfrepo.model.Value;
 import org.perfrepo.model.builder.TestBuilder;
-import org.perfrepo.model.to.TestExecutionSearchTO;
+import org.perfrepo.model.builder.TestExecutionBuilder;
+import org.perfrepo.web.alerting.ConditionChecker;
+import org.perfrepo.web.controller.TestController;
 import org.perfrepo.web.dao.DAO;
 import org.perfrepo.web.security.Secured;
 import org.perfrepo.web.service.TestService;
 import org.perfrepo.web.service.TestServiceBean;
 import org.perfrepo.web.service.exceptions.ServiceException;
 import org.perfrepo.web.session.TEComparatorSession;
-
-import javax.ejb.EJBException;
-import javax.inject.Inject;
-import javax.security.auth.Subject;
-import javax.security.auth.login.LoginContext;
-import java.security.PrivilegedAction;
-import java.util.concurrent.Callable;
+import org.perfrepo.web.util.MultiValue;
 
 /**
  * Tests for {@link TestServiceBean}
  *
  * @author Michal Linhard (mlinhard@redhat.com)
  */
 @RunWith(Arquillian.class)
-@Ignore
 public class TestServiceBeanTest {
 
    private static final Logger log = Logger.getLogger(TestServiceBeanTest.class);
 
-   private static String testUserRole = System.getProperty("perfrepo.test.role", "testuser");
+   private static String testUserRole = System.getProperty("perfrepo.test.role", "perfrepouser");
 
    @Deployment
    public static Archive<?> createDeployment() {
       WebArchive war = ShrinkWrap.create(WebArchive.class, "test.war");
-      war.addPackage(DAO.class.getPackage());
-      war.addPackage(TestService.class.getPackage());
+      war.addPackages(true, Alert.class.getPackage());
+      war.addPackages(true, TestService.class.getPackage());
+      war.addPackages(true, TestController.class.getPackage());
       war.addPackage(Secured.class.getPackage());
       war.addPackage(TEComparatorSession.class.getPackage());
-      war.addPackage(Test.class.getPackage());
-      war.addPackage(TestBuilder.class.getPackage());
-      war.addPackage(TestExecutionSearchTO.class.getPackage());
       war.addPackage(JBossLoginContextFactory.class.getPackage());
+      war.addPackage(ConditionChecker.class.getPackage());
+      war.addPackage(MultiValue.class.getPackage());
+      war.addPackage(DAO.class.getPackage());
+      war.addPackages(true, Alert.class.getPackage());
+      war.addAsLibrary(new File("target/test-libs/antlr-runtime.jar"));
+      war.addAsLibrary(new File("target/test-libs/maven-artifact.jar"));
       war.addAsResource("test-persistence.xml", "META-INF/persistence.xml");
       war.addAsResource("users.properties");
       war.addAsResource("roles.properties");
-      war.addAsWebInfResource("test-jbossas-ds.xml");
       war.addAsWebInfResource(EmptyAsset.INSTANCE, ArchivePaths.create("beans.xml"));
       return war;
    }
@@ -130,6 +146,15 @@ public Exception run() {
    protected TestBuilder test(String nameAndUid) {
       return Test.builder().name(nameAndUid).groupId(testUserRole).uid(nameAndUid).description("description for " + nameAndUid);
    }
+   
+   protected TestExecutionBuilder testExec(String nameAndUid, boolean createTest) throws Exception {
+      if (createTest) {
+         testService.createTest(test(nameAndUid).description("test test").metric("metric1", MetricComparator.HB, "").build());
+      }
+      //test must exists otherwise it will fail anyway
+      Test test = testService.getTestByUID(nameAndUid);
+      return TestExecution.builder().name(nameAndUid).testId(test.getId()).started(Date.from(Instant.now())).tag("tag");
+   }
 
    @org.junit.Test
    public void testCreateDeleteTest() throws Exception {
@@ -192,7 +217,7 @@ public void testAddMetric() throws Exception {
          public Void call() throws Exception {
             assert testService.getAllFullTests().isEmpty();
             Test test = testService.createTest(test("test1").metric("metric1", "desc").metric("metric2", "desc").build());
-            testService.addMetric(test, Metric.builder().name("metric3").description("metric 3").build());
+            testService.addMetric(test, Metric.builder().name("metric3").description("metric 3").comparator(MetricComparator.HB).build());
             assert testService.getAllFullTests().size() == 1;
             return null;
          }
@@ -207,7 +232,7 @@ public Void call() throws Exception {
             assert testService.getAllFullTests().isEmpty();
             Test createdTest = testService.createTest(test("test1").metric("A", "desc").build());
             try {
-               testService.addMetric(createdTest, Metric.builder().name("A").description("desc").build());
+               testService.addMetric(createdTest, Metric.builder().name("A").description("desc").comparator(MetricComparator.HB).build());
                assert false;
             } catch (ServiceException e) {
                // ok
@@ -246,7 +271,7 @@ public Void call() throws Exception {
             testService.createTest(test("test1").metric("A", "desc").build());
             Test test2 = testService.createTest(test("test2").metric("B", "desc").build());
             try {
-               testService.addMetric(test2, Metric.builder().name("A").description("desc").build());
+               testService.addMetric(test2, Metric.builder().name("A").description("desc").comparator(MetricComparator.HB).build());
                assert false;
             } catch (ServiceException e) {
                // ok
@@ -256,4 +281,34 @@ public Void call() throws Exception {
          }
       });
    }
+   
+   @org.junit.Test //PERFREPO-273
+   public void testUpdateTestExecutionValue() throws Exception {
+      asUser(testUserRole, new Callable<Void>() {
+         @Override
+         public Void call() throws Exception {
+            assertTrue(testService.getAllFullTests().isEmpty());
+            testService.createTestExecution(testExec("test1", true).value("metric1", 100.0).build());
+            Collection<TestExecution> execs = testService.getTestExecutions(Arrays.asList("tag"), Arrays.asList("test1"));
+            assertEquals(1, execs.size());
+            TestExecution te = execs.iterator().next();
+            Collection<Value> values = te.getValues();
+            assertEquals(1, values.size());
+            Value val = values.iterator().next();
+            assertEquals(100.0, val.getResultValue().doubleValue(), 0.001);
+            
+            TestExecution newTe = te.clone();
+            Value newVal = val.clone();
+            newVal.setResultValue(200.0);
+            newTe.setValues(Arrays.asList(newVal));
+            testService.updateTestExecution(newTe);
+            execs = testService.getTestExecutions(Arrays.asList("tag"), Arrays.asList("test1"));
+            assertEquals(1, execs.size());
+            values = execs.iterator().next().getValues();
+            assertEquals(1, values.size());
+            assertEquals(200.0, values.iterator().next().getResultValue().doubleValue(), 0.001);
+            return null;
+         }
+      });
+   }
 }

web/src/test/resources/roles.properties

@@ -1,4 +1,2 @@
+perfrepouser=perfrepouser
 testuser=testuser,perfrepouser
-testuser1=testuser1,perfrepouser
-testuser2=testuser2,perfrepouser
-testuser3=testuser3,perfrepouser

web/src/test/resources/users.properties

@@ -1,4 +1,2 @@
+perfrepouser=perfrepouser
 testuser=testuser
-testuser1=testuser1
-testuser2=testuser2
-testuser3=testuser3