Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Replace QPF with QueryExecution mockable module #1579

Merged
merged 4 commits into from
Jul 23, 2018
Merged

Conversation

l4l
Copy link
Contributor

@l4l l4l commented Jul 19, 2018

Description of the Change

QueryProcessingFactory cannot be replaced with other module, thus require more awful tests. It was replaced with QueryExecution interface and QueryExecutionImpl class (just copied previous implementation)

Benefits

QueryExecution can be mocked, simpler tests, fuzzing test are fixed (also find_fuzz is now correct)

Possible Drawbacks

Explicit mutex lock on each query

Signed-off-by: Kitsu <mail@kitsu.me>
@l4l l4l added needs-review pr awaits review from maintainers query All that relates to the iroha querying labels Jul 19, 2018
@l4l l4l requested review from vdrobnyi and kamilsa July 19, 2018 20:51
} // namespace shared_model
#include "interfaces/queries/blocks_query.hpp"
#include "interfaces/queries/query.hpp"
#include "interfaces/query_responses/query_response.hpp"
Copy link
Contributor

Choose a reason for hiding this comment

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

Forward declaration?


std::shared_ptr<ametsuchi::WsvQuery> wsvQuery_;
std::shared_ptr<ametsuchi::BlockQuery> blockQuery_;
std::mutex m;
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the reasons to introduce connection pool and SOCI was to remove locks. Please remove the mutex and use a unique connection for each query from the client.

l4l added 3 commits July 20, 2018 16:31
Signed-off-by: Kitsu <mail@kitsu.me>
Signed-off-by: Kitsu <mail@kitsu.me>
Signed-off-by: Kitsu <mail@kitsu.me>
@sorabot
Copy link

sorabot commented Jul 20, 2018

SonarQube analysis reported 6 issues

  1. MINOR find_fuzz.cpp#L28: Variable 'storage_' is assigned in constructor body. Consider performing initialization in initialization list. rule
  2. MINOR find_fuzz.cpp#L29: Variable 'bq_' is assigned in constructor body. Consider performing initialization in initialization list. rule
  3. MINOR find_fuzz.cpp#L30: Variable 'wq_' is assigned in constructor body. Consider performing initialization in initialization list. rule
  4. MINOR find_fuzz.cpp#L40: The function 'LLVMFuzzerTestOneInput' is never used. rule
  5. MINOR status_fuzz.cpp#L36: Variable 'pcs_' is assigned in constructor body. Consider performing initialization in initialization list. rule
  6. MINOR torii_fuzz.cpp#L33: Variable 'pcs_' is assigned in constructor body. Consider performing initialization in initialization list. rule

@l4l l4l removed the needs-review pr awaits review from maintainers label Jul 23, 2018
@l4l l4l merged commit 41b66c9 into develop Jul 23, 2018
@l4l l4l deleted the feature/query_exec_module branch July 23, 2018 09:55
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
l4l added a commit that referenced this pull request Jul 25, 2018
Signed-off-by: Kitsu <mail@kitsu.me>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
query All that relates to the iroha querying
Development

Successfully merging this pull request may close these issues.

5 participants