tackle-148-stakeholder-duplication

jonathanvila commented 3 years ago

Issue : https://github.com/konveyor/tackle-pathfinder/issues/43

Features covered

Avoid Stakeholders and Stakeholdergroups duplicity on updating an assessment

Unit , Integration and End2End Tests cases covered

Unit Test ( AssessmentSvcTest.java )
- given_CreatedAssessment_When_UpdateSeveralTimes_Then_StakeholdersAreNotDuplicated
End2End Test on minikube with native image ( check_api.sh )
- 8 Given an assessment, update few values and check return value is 200

Pre steps Minikube:

Install minikube : https://minikube.sigs.k8s.io/docs/start/
Start minikube : minikube start --kubernetes-version=v1.20.2 --cpus 4 --memory 12000
Enable ingress addons : minikube addons enable ingress
Create tackle namespace
```
kubectl create namespace tackle
```

Deploy tackle ( to have the keycloak instance running ) on your K8s cluster

$ kubectl apply -f  https://raw.githubusercontent.com/konveyor/tackle/main/kubernetes/kubernetes-tackle.yaml -n tackle

Edit the Ingress object to allow direct request to Pathfinder API

$ kubectl edit ingress tackle -n tackle

Add this below paths:

 - backend:
      service:
        name: tackle-pathfinder
        port:
          number: 8080 
    path: /pathfinder
    pathType: ImplementationSpecific

Build and push

$ ./mvnw -U -B package -Dquarkus.container-image.push=true -Dquarkus.container-image.group={your quay user} -Dquarkus.container-image.registry=quay.io -Dquarkus.container-image.username={your quay user} -Dquarkus.container-image.password={your quay pwd} -Pnative

Change the image in the deployment tackle-pathfinder to be the one you have pushed

Rollout to update the image

$ kubectl rollout restart deployment tackle-pathfinder -n tackle

Test case with Minikube : Do the full API test

$ .github/scripts/check_api.sh

Expect

+++++ API CHECK SUCCESSFUL ++++++

Pre steps non containerised local test:

run keycloak

podman run -it --name keycloak --rm \     
        -e KEYCLOAK_USER=admin -e KEYCLOAK_PASSWORD=admin -e KEYCLOAK_IMPORT=/tmp/keycloak/quarkus-realm.json \
        -e DB_VENDOR=h2 -p 8180:8080 -p 8543:8443 -v ./src/main/resources/keycloak:/tmp/keycloak:Z \
        jboss/keycloak:12.0.2

run postgresql

podman run -it \                          
        --name postgres-pathfinder -e POSTGRES_USER=pathfinder \                                               
        -e POSTGRES_PASSWORD=pathfinder -e POSTGRES_DB=pathfinder_db \                              
        -p 5433:5432 postgres:10.6

run application
```
./mvnw quarkus:dev
```

Test case with local non containerised : Do the full API test

$ .github/scripts/check_api.sh localhost:8085 localhost:8180

Expect

+++++ API CHECK SUCCESSFUL ++++++

NOTE

Script Test step number involved in this feature : 8

carlosthe19916 commented 3 years ago

@jonathanvila I consider the changes made to replace == by equals and the new DB INDEX you created is correct; however, I am still not able to reproduce the issue using main or your PR. I tried both native containers and the quarkus:dev mode against main and this PR without success because I can not reproduce the bug.

Since I can not reproduce the bug I can not determine whether the bug has been solved by this PR or not. @mrizzi since you reported the issue associated with this PR could you please double-check the instructions to reproduce the bug are correct or just verify the bug has been fixed by this PR?

jonathanvila commented 3 years ago

@jonathanvila I consider the changes made to replace == by equals and the new DB INDEX you created is correct; however, I am still not able to reproduce the issue using main or your PR. I tried both native containers and the quarkus:dev mode against main and this PR without success because I can not reproduce the bug.

Since I can not reproduce the bug I can not determine whether the bug has been solved by this PR or not. @mrizzi since you reported the issue associated with this PR could you please double-check the instructions to reproduce the bug are correct or just verify the bug has been fixed by this PR?

It could be related to this : https://www.baeldung.com/java-compare-long-values This is because Java maintains a constant pool for instances of Long between -128 and 127.

I will check the test to force using values higher than 127

carlosthe19916 commented 3 years ago

Thanks @jonathanvila . This PR looks good to me.

konveyor / tackle-pathfinder