Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIGENG-417 replacing One-To-One with a Many-to-One #75

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

jonathanvila
Copy link
Member

https://issues.redhat.com/browse/MIGENG-417

No real impact on functionality

Jonathan Vila and others added 7 commits January 9, 2020 10:09
* WINDUP-2398 changed mimetype and created sample available json

* changed kafka topic

* changed kafka topic and category

* refactored code

* replaced seda by direct as it seemed not to work

* Converted calculator into Bean
Added Sonar + Jacoco execution

* Created first test for a simple route in order to know how to properly test Camel

* Created unit test for AnalysticsCalculator
Added JUnit 5 and AssertJ to pom
Aplied some clean-code suggestions

* Removed sonar token from pom

* WINDUP-2398
Added support for JUnit 5 on old maven versions
Added constructors on model
Fixed AnalyticsCalculatorTest test
Added Sonar info to README

* WINDUP-2398
Added lombok file to add Generated annotation that will be avoided by JaCoCo
Added sample file to test multipart
Added test for the multipart
Removed accessors/modifiers for cloudforms classes
Cleaned CustomizedMultipartDataFormat

* WINDUP-2398
Renamed test methods

* WINDUP-2398
Renamed test methods

* WINDUP-2398
Moved back to JUnit 4 as Camel Test support doesnt support JUnit 5 yet

* WINDUP-2398
Added very simple test for direct:store

* WINDUP-2398
Added test for direct-upload with individual files and zip file

* WINDUP-2398
Added test for direct:insights route

* WINDUP-2398
minor change on xmlroutestest

* WINDUP-2398 Applied changes from travis PR

* WNDUP-2398 added test for kafka listener route

* WNDUP-2398 added a test for the direct-download route ( not finished )

* WNDUP-2398 fixed test for direct:download route

* WNDUP-2398 added test for direct:calculate
added cloudforms export file

* WNDUP-2398 testing the rest endpoint

* WNDUP-2398 testing the rest-upload endpoint

* WNDUP-2398 testing the rest report endpoint

* WNDUP-2398 switched to regular endpoints for rest, as rest DSL has some "issues"

* WNDUP-2398 renamed kie helper methods with no Random, and removed unused code

* WNDUP-2404 testing call-kie-extract-reports route
removed testing route

* WNDUP-2398 testing the  direct:decisionserver endpoint

* WNDUP-2404 changed kie endpoint from http4 to http as a ClientProtocolException appears

* WINDUP-2404 adjusted the tests to consider the new context-path

* windup-2398 considering non file params as headers for all messages

* windup-2398 testing the param from multi part form

* WINDUP-2398 removed customerid from the url on the rest endpoint

* WINDUP-2398 removed customerid from the test of rest endpoint

* WINDUP-2398 modified tests to work with dynamic metadata properties and filtering for messages having all those properties

* MIGENG-23 added new version of Cloudforms file, refactored Calculator

* MIGENG-23 replaced the object model to be sent to JMS with the one in xavier-analytics

* MIGENG-23 adapted tests to new report model

* MIGENG-23 changed cloudforms model types Integer to Long

* MIGENG-23 changed the way we obtain RequestId as before it was always the same

* MIGENG-23 fixed tests according to the new model

* MIGENG-23 replaced cloudforms test files

* MIGENG-23 made host.type filter configurable with a property

* MIGENG-23 added tar and gz support to the route

* MIGENG-23 added TAR.GZ support and added test for that

* MIGENG-23 added exception handling when the json crashes on the unmarshal

* MIGENG-23 fixed mocking log endpoint in test

* MIGENG-23 fixed attributes and filter names

* MIGENG-23 fixed tests with the new "MA_metadata" header
now we use the RHIdentity object passed as x-rh-identity header if it exists and we populate our params

* MIGENG-106 Modified the routes in order to do the unzip after receiving the message from kafka and not in the upload

* MIGENG-106 Fixed tests in order to work with the unzipping after the kafka message

* MIGENG-106 Removed all structure for CloudForms
Modified test and added paths to properties

* MIGENG-106 Small refactors

* MIGENG-106 Renamed test

* end2end added test to allow an End To End test
using TestContainers and real layers
using WireMock to simulate Rest InsightsUpload service

* end2end working on issue with Ryuk and test containers

* MIGENG-106 code prepared to use the version on the manifest

* MIGENG-106 added test to check versioning fallback

* MIGENG-106 fixed tests according the new payload taxonomy

* MIGENG-106 added tar.gz file for only 1 json inside

* MIGENG-106 modified getRHIdentity to modify only the identity.internal element

* MIGENG-106 fixes to issues during deployment and test

* MIGENG-106 fixed the calculation method for the number of hypervisors

* MIGENG-106 fixing and adding tests to changes on getRHIdentity and extractMAmetadata

* MIGENG-106 replaced artemis with activemq
Improvements in the E2E test with the containers

* end2end added hoverfly to simulate insights upload service, firsts attempts

* end2end typo fix

* end2end added logging for testing, and changed to mock-server to mock rest API

* end2end Fixed the test with correct multipart sending, correct jpa retrieving, and kie sample files as response

* end2end Fix the End2End test with correct version of TestContainers
Fixed test for Kie Service with proper expectation
Fixed test for Direct Download for considering the proper endpoint having query params

* end2end Removed analytics dependency

* end2end Removed Debug LOG lines

* end2end Removed hoverfly logging

* MIGENG-150 Creation of initial classes and routes for VMWorkloadInventory

* end2end renamed the EndToEnd to a regular Test

* end2end fixing the filtering to camel headers , and switching back to previous false state

* end2end making the identity of the ..with-targz.json file the full one

* end2end making the identity of the platform.upload.xavier.json.gz and .json file the full one

* end2end fixing base64 identity to have the same
fix the type of the file loaded to be consistent with the one in the base64 identity

* end2end fixed test dependencies

* end2end removed Spring Boot ActiveMQ dependency

* MIGENG-150 Added Test for the VMWorkloadInventoryCalculator
Changed Ids for second host in sample file (.json)
Added multicast route to send message to both calculators
Renamed cost savings route
Added paths to obtain workload inventory properties

* MIGENG-150 Fixed test

* MIGENG-150 removed parallel processing

* MIGENG-150 added test for direct:calculate-vmworkloadinventory

* MIGENG-150 adapted route using decisionServer to use the right methods

* MIGENG-150 set right values for kie command and session for WKI

* MIGENG-150 fixed value for test

* MIGENG-150 fixed multicast and split issue as seen on CI deployment

* MIGENG-150 changed the method call from setWorkload ... to AddWorkLoad

* MIGENG-150 changed the test for calculate vmworkload to consider individual elements return instead of a collection

* MIGENG-150 added Serializable to the WorkloadInventory Model object

* MIGENG-150 moved class VMWorkloadinventory to another package

* MIGENG-150 fixed depending classes after moved class VMWorkloadinventory to another package

* end2end added info about Ryuk and Testcontainers property

* MIGENG-151 adding methods to get the collection fields

* MIGENG-151 added methods to set values for the collection fields
added info to the test payload

* MIGENG-150 working on the Files map retrieval

* MIGENG-150 fixed the path for the files map
fixed the lambda to get the files map value pairs

* MIGENG-151 working on the Files map retrieval

* MIGENG-151 fixed the path for the files map
fixed the lambda to get the files map value pairs

* MIGENG-151 changed expected values considering also the collection fields for the workloadinventory

* MIGENG-155 saving and persisting the entity

* MIGENG-155 adding methods on the analysisService to get and update the analysismodel

* MIGENG-155 modified WLI route to use the new method in analysisService
modified test on DirectStore

* MIGENG-155 renamed resources for inticial cost savings report

* end2end Added S3 client bean to customise its endpoint url
Added PostgreSQL testcontainers container
Added Localstack testcontainers container
Fixed analysisid in json file as response from kafka

* end2end Added reports files for demolab payload
added tests for the 3 reports
added test for S3
added performance test

* end2end Added demolab payloads files
added report file for demolab

* end2end Added Drools and KIE containers
Added Ingress image build and container creation

* end2end reordered code

* end2end Modified KAFKA estrategy to earliest to take all pending messages
Added properties for kieserver to connect to local container
Added properties for KIE container to connect to DROOLS container
Added docker-compose containers for Ingress
Removed mocks
Added commands to import projecto into drools

* end2end added more env vars
added bash script file with docker and curl commands to simulate the infrastructure deployment

* end2end add env vars

* end2end adjusted testcontainers env vars
added sleeps in the bash script

* end2end using shell commands to execute docker images for drools and kie

* end2end added travis stuff for docker
changed the way we get test files from FS

* end2end fixed ocd analytics-template version

* end2end fixed ScanRunModel accessors of Type

* end2end fixed date value for summary report scan run model test file

* end2end fixed Spring Boot error for Port 8080 already in use
modified folder on ignored test for S3
added live replacement on ingress docker-compose to make it work
increased performance test timeout to 1 minute

* end2end addded changes to run Testcontainers in Travis

* end2end added again H2 as test database
configured the env properties for End2End Test with postgresql

* end2end changes to travis to fix the issue with Testcontainers

* end2end setting log level to debug on tests to check on travis

* end2end log level back to info

* end2end increasing time to wait until initialcostsavings report is generated

* end2end increased time for CSR to 3 minutes, and used awaitillity to continue earlier if its finished

* end2end added log level to hibernate to check connections to DB

* end2end added commands to upgrade docker version at Travis
removed testcontainers kafka dependency as it's not used

* end2end making kafka to fail in the connection

* end2end added print out of docker logs
added explicit naming of kafka container

* end2end using master branch of ingress at 20191115

* end2end renamed folder of downloaded insights as it causes issues with docker-compose

* end2end added docker logs in travis

* end2end added logger output to critical containers

* end2end added time before the test starts

* end2end added logger to activemq
switched back to old version of ingress

* end2end added more logs to containers and increased times

* end2end added advertising for kafka

* end2end enabled again ryuk

* end2end removed -DtestLogRootLevel=warn -Pcoverage from the test script

* end2end adding log and more time to start the test

* end2end added zookeeper logs

* end2end moved docker compose containers to Testcontainers
added temporary hacks in the download-file route to test

* end2end enabled the download of ingress-go project from github

* end2end printing ScanRunModels and using sameDay as comparison between dates in ScanRunModel

* end2end not considering scanrunmodel.date in the test

* end2end removing tracing because Travis kills the job if we exceed

* end2end removed log of docker containers in travis yaml

* end2end fixed unit tests
set all tests to be run in travis
refactored comparisson of sets

* end2end reduced log level to reduce logs in Travis

* end2end changed hardcoded values
set the logging level in the travis pipeline
removed unused files

* end2end re-added Profile Coverage to mvn execution

* end2end some PR clean tasks
- fixed Testcontainers name in Readme
- removed unused files
- refactored ...DirectCalculateVMWorkloadInventoryModelTest assert on scanRunModel
- fixed typo on test properties
- removed comments on ScanRunModel

* end2end added minio.start

* end2end replaced timeouts with something way shorter (in  milliseconds)

* end2end added Github action to build project and red hat repositories for dependencies

* Update maven.yml

* MIGENG-365 removed github pipeline

* MIGENG-365 cleanup downloaded files

* MIGENG-365 added timouts to the test properties and increased the value
fixed not available minio.host prop on runtime

* MIGENG-365 as we are not specifying profile, moved the regular SpringConfiguration to any profile except test

* MIGENG-365 added delay between Minio and MINIOMC containers

* MIGENG-365 removed test code from the production route, and moved to the test

* MIGENG-365 cleaned the route on the download-file side, and moved parts to the test

* change `type` by `smartStateEnabled`

* restore `type` by `smartStateEnabled` since it will modify the database

* removed unused import

* fix test

* MIGENG-365 [PR] removed unused jmsTemplate

* MIGENG-365 [PR] moved Docker only test files to subfolder

* MIGENG-365 [PR] renamed minio shell script

* end2end removed minio-mc commands setting policies for the buckets as they are wrong and apparently not needed

Co-authored-by: Carlos E. Feria Vila <[email protected]>
* MIGENG-327 added try-catch to the list of values json reader

* MIGENG-327 added null check on the vmStructMap

* end2end adding conditionals to avoid division by zero

* MIGENG-327 added test to cover null and 0 on calculation of hypervisors

* MIGENG-327 added test to cover missing attributes on FlagSharedDisksCalculator

* MIGENG-327 fixed test on missing atts for FlagSharedDisksCalculator

* MIGENG-327 added test to cover missing attributes on VMWorkloadInventoryCalculator
added abstract clause to AbstractVMWorkloadInventoryCalculator

* MIGENG-327 added test for EndToEnd with a file with missing attributes

* MIGENG-327 changed the approach to not set 0 as hypervisors in order to avoid division By Zero

* MIGENG-327 added individual test files with each use case

* MIGENG-327 avoided issue with Localstack and json files being uploaded

* MIGENG-327 reduced output on Exceptions reading from JSON
Added method to avoid ClassCastException on reading from JSON
Added conditional to avoid setting a default hasRdmDisk

* MIGENG-327 upgraded Testcontainers version

* MIGENG-327 Created Handler for the issues recording

* MIGENG-327 Introduced the errors in the 3 test files for the issues use cases

* MIGENG-327 Introduced the issues recorder in the calculators

* MIGENG-327 Fixed calculator test method call adding analysisId

* MIGENG-327 Added values tests on End2End verifying the reports on the 3 issues use cases

* MIGENG-327 Refactored the record method to include entity ( VM, HOST, ... )

* MIGENG-327 Refactored End2End report calls

* MIGENG-327 added asserts on the total of VMs
modified asserts on "No ..... defined"

* MIGENG-327 [PR] refactored variable name
@jonathanvila jonathanvila added the Ready for review The PR is ready to be reviewed label Feb 6, 2020
@codecov-io
Copy link

codecov-io commented Feb 6, 2020

Codecov Report

Merging #75 into master will increase coverage by 0.37%.
The diff coverage is 86.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #75      +/-   ##
============================================
+ Coverage     78.85%   79.22%   +0.37%     
- Complexity      732      743      +11     
============================================
  Files            71       72       +1     
  Lines          2038     2094      +56     
  Branches         93       94       +1     
============================================
+ Hits           1607     1659      +52     
- Misses          373      376       +3     
- Partials         58       59       +1
Impacted Files Coverage Δ Complexity Δ
...vier/integrations/jpa/service/AnalysisService.java 95.16% <ø> (+5.76%) 17 <0> (ø) ⬇️
...ss/xavier/analytics/pojo/output/AnalysisModel.java 88.37% <ø> (ø) 25 <0> (ø) ⬇️
.../pojo/output/workload/summary/ComplexityModel.java 93.33% <100%> (+1.02%) 15 <3> (+2) ⬆️
...avier/analytics/pojo/PayloadDownloadLinkModel.java 50% <50%> (ø) 3 <3> (?)
...ss/xavier/integrations/route/MainRouteBuilder.java 98.34% <95.45%> (-0.65%) 36 <6> (+6)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 186a847...f14ff2d. Read the comment docs.

@@ -65,7 +65,7 @@
)
private Long id;

@OneToOne(fetch = FetchType.LAZY)
@ManyToOne(fetch = FetchType.LAZY)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanvila Since you are enhancing this relation I suggest we can add optional = false so we are 100% sure that there won't exist any WorkloadInventoryReportModel without an AnalysisModel. this change should add not null constraint to the column analysis_id into the workload_inventory_report_model table.

Suggested change
@ManyToOne(fetch = FetchType.LAZY)
@ManyToOne(fetch = FetchType.LAZY, optional = false)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it won't make any harm, but it's totally impossible to have a null there as it has a foreign key against analysis_id where id is the primary therefore not null
"fkt966e5qfj0hwmb0oecel9qbyw" FOREIGN KEY (analysis_id) REFERENCES analysis_model(id)

Copy link
Member

@carlosthe19916 carlosthe19916 Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonathanvila Let me clarify my previous comment. Right now, we have a foreign key in workload_inventory_report_model which points to analysis_model; however, having a foreign key in workload_inventory_report_model does not force the table to contain not null values into the column analysis_id. For example you can execute these commands:

insert into workload_inventory_report_model(id) values (-1);
select * from workload_inventory_report_model where id=-1;

Screenshot from 2020-02-06 17-12-50

You will see that the previous command can be executed; it means: "I've created a workload_inventory_report_model without an analysis_model"

This is the current definition of the table, and as you can see the column analysis_id accept null values.

Screenshot from 2020-02-06 17-13-45

I think we missed this kind of small detail in many of our current Models; however, we can improve it by the time so we have concise model definitions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right.
We should revisit our whole model design .... obviously that foreign key should not be nullable
This concerns me more then, that we can find other issues in the model :(

So I would suggest not doing this small change you suggests in this PR, but open another issue of revisiting the whole model adding this and all other changes needed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's create a ticket for analyzing all our models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for review The PR is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants