Skip to content

Commit

Permalink
fix: increase performance while purging execution (#547)
Browse files Browse the repository at this point in the history
* fix: increase performance while purging execution

* chore: remove sonar code smells

* chore: generate liquibase delete cascade on problem occurrence

* fix: bug liquibase gen

* chore: update doc

* test: update integration test coverage

* fix: ci

* chore: update release candidate version

Co-authored-by: Thomas GRUSON <thomgrus@gmail.com>
  • Loading branch information
omar-chahbouni-decathlon and Thomgrus authored Dec 2, 2021
1 parent d66d91a commit 673d522
Show file tree
Hide file tree
Showing 64 changed files with 616 additions and 239 deletions.
38 changes: 13 additions & 25 deletions .github/workflows/build-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ on:
- cron: '30 1 * * 0'

jobs:
code_api_change:
code_app_change:
# continue-on-error: true # Uncomment once integration is finished
runs-on: ubuntu-latest
# Map a step output to a job output
Expand All @@ -32,13 +32,12 @@ jobs:
uses: fkirc/skip-duplicate-actions@v3.4.0
with:
github_token: ${{ github.token }}
paths: '["code/api/**"]'
paths: '["code/api/**", ".github/workflows/build-api.yaml"]'
cancel_others: 'true'
do_not_skip: '["push", "workflow_dispatch", "schedule"]'
build-api:
needs:
- code_api_change
if: ${{ needs.code_api_change.outputs.should_skip != 'true' }}
needs: code_app_change
if: ${{ needs.code_app_change.outputs.should_skip != 'true' }}
runs-on: ubuntu-latest
steps:
-
Expand All @@ -56,7 +55,7 @@ jobs:
name: Initialize CodeQL
uses: github/codeql-action/init@v1
with:
lanquages: java
languages: java
config-file: ./.github/codeql/codeql-api-config.yml
-
name: Cache SonarCloud packages
Expand All @@ -77,11 +76,9 @@ jobs:
run: sudo echo "127.0.0.1 oauth2.dev.localhost" >> sudo tee -a /etc/hosts
-
name: Set up QEMU
if: ${{ steps.skip_check.outputs.docker_skip != 'true' }}
uses: docker/setup-qemu-action@v1
-
name: Set up Docker Buildx
if: ${{ steps.skip_check.outputs.docker_skip != 'true' }}
uses: docker/setup-buildx-action@v1
-
name: start oauth2-dev-server
Expand All @@ -101,49 +98,39 @@ jobs:
-
name: Install xmllint
run: sudo apt-get install libxml2-utils
-
id: skip_check
run: |
if [[ $GITHUB_REF == refs/pull/* ]]; then
API_EXISTS=false
else
pushd code
API_EXISTS=$(make -s check-api-image)
popd
fi
echo ::set-output name=docker_skip::${API_EXISTS}
-
name: Prepare
id: prep
if: ${{ steps.skip_check.outputs.docker_skip != 'true' }}
run: |
PUSH=FALSE
pushd code
API_EXISTS=$(make -s check-api-image)
popd
if [[ $GITHUB_REF == refs/heads/* ]]; then
BRANCH_NAME=$(echo ${GITHUB_REF#refs/heads/} | sed -r 's#/+#-#g')
if [ "${{ github.event.repository.default_branch }}" = "$BRANCH_NAME" ]; then
if [ "${{ github.event.repository.default_branch }}" = "$BRANCH_NAME" ] && [ "$API_EXISTS" != 'true' ]; then
PUSH=TRUE
fi
elif [[ $GITHUB_REF == refs/pull/* ]]; then
BRANCH_NAME=$(echo ${{ github.event.pull_request.head.ref }} | sed -r 's#/+#-#g')
if [[ $BRANCH_NAME == release-* ]]; then
PUSH=TRUE
SUFFIX=rc
SUFFIX=-rc
else
SUFFIX=pr-${{ github.event.number }}
SUFFIX=-pr.${{ github.event.number }}
fi
fi
echo ::set-output name=push::${PUSH}
echo ::set-output name=suffix::${SUFFIX}
-
name: Login to DockerHub
if: ${{ steps.skip_check.outputs.docker_skip != 'true' && steps.prep.outputs.push == 'true' }}
if: ${{ steps.prep.outputs.push == 'true' }}
uses: docker/login-action@v1
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
-
name: Build and push
if: ${{ steps.skip_check.outputs.docker_skip != 'true' }}
run: |
pushd code
make build-api SUFFIX=${{ steps.prep.outputs.suffix }} PUBLISH=${{ steps.prep.outputs.push }}
Expand All @@ -161,3 +148,4 @@ jobs:
make test-karate BATCH=true
make destroy-local
popd
cat code/tests/target/results.txt | grep SUCCESS
2 changes: 1 addition & 1 deletion charts/candidate/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v2
name: ara-candidate
version: 10.0.0-rc.8
version: 10.0.0-rc.9
home: https://github.com/Decathlon/ara
description: |
ARA helps you to fight against regressions by letting it preanalyze your non-regression tests runs,
Expand Down
2 changes: 1 addition & 1 deletion charts/candidate/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ api:
image:
registry: docker.io
repository: decathlon/ara-api
tag: 11.1.3
tag: 11.1.4
imagePullPolicy: IfNotPresent
loggingMode:
- logging-console
Expand Down
2 changes: 1 addition & 1 deletion code/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ build-api: ## Build ara-api
ifeq ($(FULL),true)
$(MAKE) -C api mvn-install
endif
$(MAKE) -C api mvn-build-image FULL_IMAGE=$(FULL_IMAGE)
$(MAKE) -C api mvn-build-image FULL_IMAGE="$(FULL_IMAGE)"
@./bin/docker-tags-version.sh "$(API_IMAGE)" "$(API_VERSION)" "$(PUBLISH)"

build-oads: ## Build the ara oauth2-dev-server
Expand Down
6 changes: 4 additions & 2 deletions code/api/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#####################################

JAVA_VERSION=16
VERSION=$(shell make -s get-version)

#####################################
## DEVELOP ##
Expand Down Expand Up @@ -47,10 +48,11 @@ dkr-run: ## DEV - run api with docker
#####################################

mvn-build-image: ## Build image with maven
ifdef $(FULL_IMAGE)
ifdef FULL_IMAGE
@echo 'Building with specfic image name: $(FULL_IMAGE)'
endif
@./mvnw spring-boot:build-image -Dmaven.test.skip=true -Dspring-boot.build-image.imageName=$(FULL_IMAGE) -pl api
@./mvnw spring-boot:build-image -Dmaven.test.skip=true -pl api
@docker tag docker.io/decathlon/ara-api:$(VERSION) $(FULL_IMAGE)

#####################################
## UTILS ##
Expand Down
2 changes: 1 addition & 1 deletion code/api/api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

<groupId>com.decathlon.ara</groupId>
<artifactId>ara-api</artifactId>
<version>11.1.3</version>
<version>11.1.4</version>

<name>ARA API</name>
<description>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.decathlon.ara.purge.service;

import com.decathlon.ara.domain.Execution;
import com.decathlon.ara.repository.ExecutionRepository;
import com.decathlon.ara.service.ProjectService;
import com.decathlon.ara.service.SettingService;
Expand Down Expand Up @@ -87,13 +88,14 @@ public void purgeExecutionsByProjectId(long projectId) {
return;
}

var executedScnToDelete = executionRepository.findByCycleDefinitionProjectIdAndTestDateTimeBefore(projectId, startDate.get());
var executionsToDelete = executionRepository.findByCycleDefinitionProjectIdAndTestDateTimeBefore(projectId, startDate.get());

var numberOfDeletedExecutions = executedScnToDelete.size();
var numberOfDeletedExecutions = executionsToDelete.size();
var executionsPlural = numberOfDeletedExecutions > 1 ? "s" : "";

log.info("Preparing to delete {} execution{}...", numberOfDeletedExecutions, executionsPlural);
executionRepository.deleteAllInBatch(executedScnToDelete);
var executionIdsToDelete = executionsToDelete.stream().map(Execution::getId).toList();
executionRepository.deleteAllByIdInBatch(executionIdsToDelete);
log.info("{} execution{} successfully deleted", numberOfDeletedExecutions, executionsPlural);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@

package com.decathlon.ara.service;

import com.decathlon.ara.domain.QProblem;
import com.decathlon.ara.repository.CountryRepository;
import com.decathlon.ara.repository.ErrorRepository;
import com.decathlon.ara.repository.ProblemPatternRepository;
import com.decathlon.ara.repository.ProblemRepository;
import com.decathlon.ara.repository.TypeRepository;
import com.decathlon.ara.Entities;
import com.decathlon.ara.Messages;
import com.decathlon.ara.domain.Error;
import com.decathlon.ara.domain.*;
import com.decathlon.ara.repository.*;
import com.decathlon.ara.repository.custom.util.JpaCacheManager;
import com.decathlon.ara.repository.custom.util.TransactionAppenderUtil;
import com.decathlon.ara.service.dto.error.ErrorWithExecutedScenarioAndRunAndExecutionDTO;
Expand All @@ -34,16 +33,6 @@
import com.decathlon.ara.service.mapper.ErrorWithExecutedScenarioAndRunAndExecutionMapper;
import com.decathlon.ara.service.mapper.ProblemMapper;
import com.decathlon.ara.service.mapper.ProblemPatternMapper;
import com.decathlon.ara.Entities;
import com.decathlon.ara.Messages;
import com.decathlon.ara.domain.Country;
import com.decathlon.ara.domain.Error;
import com.decathlon.ara.domain.Problem;
import com.decathlon.ara.domain.ProblemPattern;
import com.decathlon.ara.domain.Type;
import java.util.Collections;
import java.util.Set;
import java.util.stream.Collectors;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -52,6 +41,10 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Collections;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Service for managing ProblemPattern.
*/
Expand Down Expand Up @@ -161,7 +154,7 @@ public Page<ErrorWithExecutedScenarioAndRunAndExecutionDTO> getProblemPatternErr
if (problemPattern == null) {
throw new NotFoundException(Messages.NOT_FOUND_PATTERN, Entities.PROBLEM_PATTERN);
}
Page<Error> errors = errorRepository.findDistinctByProblemPatternsInOrderById(Collections.singletonList(problemPattern), pageable);
Page<Error> errors = errorRepository.findDistinctByProblemOccurrencesProblemOccurrenceIdProblemPatternInOrderById(Collections.singletonList(problemPattern), pageable);
return errors.map(errorWithExecutedScenarioAndRunAndExecutionMapper::toDto);
}

Expand Down Expand Up @@ -215,12 +208,13 @@ public ProblemPatternDTO update(long projectId, ProblemPatternDTO dtoToUpdate) t
* cache-evicted after transaction commit
*/
private void evictErrorProblemPatternsCacheFor(ProblemPattern problemPattern) {
Set<Long> errorIds = problemPattern.getErrors()
Set<Long> errorIds = problemPattern.getProblemOccurrences()
.stream()
.map(ProblemOccurrence::getError)
.map(Error::getId)
.collect(Collectors.toSet());
transactionService.doAfterCommit(() ->
jpaCacheManager.evictCollections(Error.PROBLEM_PATTERNS_COLLECTION_CACHE, errorIds));
jpaCacheManager.evictCollections(Error.PROBLEM_OCCURRENCES_COLLECTION_CACHE, errorIds));
}

void assignExistingEntities(long projectId, ProblemPattern problemPattern) throws NotFoundException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,12 +448,14 @@ public void delete(long projectId, long id) throws NotFoundException {
private void evictErrorProblemPatternsCacheFor(Problem problem) {
final Set<Long> errorIds = problem.getPatterns()
.stream()
.flatMap(p -> p.getErrors().stream())
.map(ProblemPattern::getProblemOccurrences)
.flatMap(Collection::stream)
.map(ProblemOccurrence::getError)
.map(Error::getId)
.collect(Collectors.toSet());

transactionService.doAfterCommit(() ->
jpaCacheManager.evictCollections(Error.PROBLEM_PATTERNS_COLLECTION_CACHE, errorIds));
jpaCacheManager.evictCollections(Error.PROBLEM_OCCURRENCES_COLLECTION_CACHE, errorIds));
}

/**
Expand Down Expand Up @@ -508,7 +510,7 @@ public Page<ErrorWithExecutedScenarioAndRunAndExecutionDTO> getProblemErrors(lon
}

List<ProblemPattern> problemPatterns = problem.getPatterns();
Page<Error> errors = errorRepository.findDistinctByProblemPatternsInOrderByIdDesc(problemPatterns, pageable);
Page<Error> errors = errorRepository.findDistinctByProblemOccurrencesProblemOccurrenceIdProblemPatternInOrderByIdDesc(problemPatterns, pageable);
if (errors == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@
package com.decathlon.ara.service.mapper;

import com.decathlon.ara.domain.Error;
import com.decathlon.ara.domain.ProblemOccurrence;
import com.decathlon.ara.domain.ProblemPattern;
import com.decathlon.ara.service.dto.error.ErrorWithProblemsDTO;
import com.decathlon.ara.service.dto.problem.ProblemDTO;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import org.mapstruct.AfterMapping;
import org.mapstruct.Mapper;
import org.mapstruct.MappingTarget;
import org.springframework.beans.factory.annotation.Autowired;

import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;

/**
* Mapper for the entity Error and its DTO ErrorWithProblemsDTO.
*/
Expand All @@ -49,8 +51,9 @@ public abstract class ErrorWithProblemsMapper implements EntityMapper<ErrorWithP
@AfterMapping
protected void populateProblems(Error error, @MappingTarget ErrorWithProblemsDTO errorDto) {
final List<ProblemDTO> problems = problemMapper.toDto(
error.getProblemPatterns()
error.getProblemOccurrences()
.stream()
.map(ProblemOccurrence::getProblemPattern)
.map(ProblemPattern::getProblem)
.sorted() // By name, to display them in a consistent fashion
.distinct() // A problem can have two patterns matching the same error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@

import com.decathlon.ara.domain.Error;
import com.decathlon.ara.domain.Problem;
import com.decathlon.ara.domain.ProblemOccurrence;
import com.decathlon.ara.domain.ProblemPattern;
import com.decathlon.ara.service.dto.error.ErrorWithProblemsDTO;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

/**
* This service provide transformation utilities (DTO - DO and DO - DTO) for the Error.
Expand Down Expand Up @@ -59,7 +61,8 @@ ErrorWithProblemsDTO toDto(Error error) {
result.setStepLine(error.getStepLine());
result.setException(error.getException());

List<Problem> problems = error.getProblemPatterns().stream()
List<Problem> problems = error.getProblemOccurrences().stream()
.map(ProblemOccurrence::getProblemPattern)
.map(ProblemPattern::getProblem)
.sorted(Comparator.nullsLast(Problem::compareTo))
.distinct()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@

package com.decathlon.ara.domain;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;

import com.decathlon.ara.domain.enumeration.Handling;
import com.decathlon.ara.domain.enumeration.ProblemStatus;
import org.junit.jupiter.api.Test;

import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;

public class ExecutedScenarioTest {

Expand Down Expand Up @@ -56,9 +57,8 @@ public void getHandling_ShouldReturnUNHANDLED_WhenOnlyErrorsWithoutProblem() {
public void getHandling_ShouldReturnHANDLED_WhenAtLeastOneErrorWithProblem() {
// GIVEN
final Error errorWithProblem = new Error().withStepLine(2);
errorWithProblem.addProblemPattern(new ProblemPattern()
.withProblem(new Problem()
.withStatus(ProblemStatus.OPEN)));
var problemPattern = new ProblemPattern().withProblem(new Problem().withStatus(ProblemStatus.OPEN));
errorWithProblem.setProblemOccurrences(Set.of(new ProblemOccurrence(new Error(), problemPattern)));
final ExecutedScenario executedScenarioWithoutError = new ExecutedScenario();
executedScenarioWithoutError.addError(new Error().withStepLine(1));
executedScenarioWithoutError.addError(errorWithProblem);
Expand Down
Loading

0 comments on commit 673d522

Please sign in to comment.