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

New insert system #3

Merged
merged 14 commits into from Jul 29, 2019
@@ -18,6 +18,7 @@ You can then edit the `config.php` and `database.php` files in the `config/` fol
| QANB_DB_USERNAME | Database username |
| QANB_DB_PASSWORD | Database password |
| QANB_DB_NAME | Database name |
| QANB_TOKEN | Token to add JSON data through the Hook |


## Web server configuration
@@ -43,13 +44,12 @@ Set up a vhost that points to the `/public` folder:

## Inserting new data

Use `insert.php` to insert json files. This file uses the `database.php` config file in `application/config` so be sure it's set up correctly.

The first argument of the script is the path to the file you want to insert. The second argument is the version. Example:

```
php insert.php application/files/reports_2019-06-18_develop.json develop
```
Use the hook provided in the `Hook` controller. You need to call this URL: `BASE_URL/hook/add` with the following GET parameters:
- `token`: the token set in the environment variable `QANB_TOKEN` (e.g.: `IpBzOmwXQUrW5Hn`)
- `filename` : the complete filename to look for in the Google Cloud Storage (e.g.: `2019-07-22-develop.json`). The name must follow this pattern: `/[0-9]{4}-[0-9]{2}-[0-9]{2}-(.*)?\.json/`
Optional:
- `force`: a special parameter used to force insert when a similar entry is found (criterias are date and version)
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

I suggest adding 2 things:

  • a complete example like mysite.com/hook/add?token= IpBzOmwXQUrW5Hn&filename=2019-07-22-develop.json
  • indicate server must be configured properly in order to be able to download huge files in memory

This comment has been minimized.

Copy link
@matks

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Done thanks


## Containers

@@ -0,0 +1,254 @@
<?php
defined('BASEPATH') OR exit('No direct script access allowed');
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Jul 24, 2019

Collaborator

unneeded

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Welcome to CodeIgniter. It's used everywhere... why exactly it is unneeded ?

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Jul 24, 2019

Collaborator

Because no code is running is, it's a class declaration.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Makes sense

class Hook extends MY_Base {
private $execution_id = null;
private $pattern = '/[0-9]{4}-[0-9]{2}-[0-9]{2}-(.*)?\.json/';
private $force = false;
public function add()
{
$this->load->model('Execution');
$this->load->model('Suite');
$this->load->model('Test');
log_message('info', "verifying data");
if (!$this->input->get('token') || !$this->input->get('filename')) {
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("no enough parameters");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
exit("no enough parameters");
exit(json_encode(['error' => "no enough parameters"]));

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

That's a real question here: do I need to json_encode all my error messages or just the ones after retrieving the data ?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Soooo I encoded them all.

}
log_message('info', "verifying name of file (".$this->input->get('filename').")");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Please do not directly write the content of $this->input->get('filename') in the logs, else you are introducing a potential Log Injection vulnerability.

Please check first filename is valid according to your regexp pattern, then log it

preg_match($this->pattern, $this->input->get('filename'), $matches);
if (!isset($matches[1])) {
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("filename '".$this->input->get('filename')."' is invalid");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Same here, dont output the filename, just say "given filename is invalid"

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Done thanks

}
log_message('info', "verifying token is here");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
log_message('info', "verifying token is here");
log_message('info', "verifying token is valid");
if (!$this->checkToken($this->input->get('token'))) {
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("invalid token");
}
if ($this->input->get('force') !== NULL) {
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
if ($this->input->get('force') !== NULL) {
if ($this->input->get('force') && ($this->input->get('force') !== 'false')) {

Else if I call mysite.com/hook/add?token= IpBzOmwXQUrW5Hn&filename=2019-07-22-develop.json&force=false it's going to force it although I asked not to force

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

That's perfectly correct

$this->force = true;
}
//get the file from the GCP API
$filename = $this->input->get('filename');
log_message('info', "receiving filename $filename");
//create URL
$url = sprintf("https://storage.googleapis.com/prestashop-core-nightly/reports/%s", $filename);
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@djodjo3

djodjo3 Jul 24, 2019

Contributor

Would it be wise to variablize this URL/Storage ?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

That's a good suggestion. I thought about it but i'm not sure to know where is the best place to put it ? It's a little too big to be in an environment variable IMHO

This comment has been minimized.

Copy link
@djodjo3

djodjo3 Jul 24, 2019

Contributor

It's really not that big :)
And, if possible, I'd rather have it as a configuration parameter.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

OK, I put it as a private property of that class, is that good enough ? Or would you prefer it as an environment variable so that we can change it through the Docker script

This comment has been minimized.

Copy link
@djodjo3

djodjo3 Jul 24, 2019

Contributor

Environment variable is always nice 👍

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

You can also provide a default value and allow it to be override to make config simpler 😄

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

I don't think it's something that will change anytime soon, so I prefer to keep it as a private property for now.

log_message('info', "creating url $url");
//retrieve content
log_message('info', "retrieving content...");

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

@PierreRambaud does not like double quotes " : use single quote '

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

So I need to use ugly concatenation or sprintf to make him happy ? 👎

try {
$contents = file_get_contents($url);
} catch(Exception $e) {
log_message('error', "Could not retrieve content from $url");
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("unable to decode JSON data");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
exit("unable to decode JSON data");
exit("unable to retrieve content from GCP URL");
}
log_message('info', "decoding JSON...");
try {
$file_contents = json_decode($contents);
} catch(Exception $e) {

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

If you want json_decode() to throw exception on bad parsing, you need to set JSON_THROW_ON_ERROR flag to true (https://www.php.net/manual/en/function.json-decode.php)

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

I see it's only available in PHP 7.3 though. What would be the correct way to handle it ?

log_message('error', "Could not decode JSON data from the file");
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("unable to decode JSON data");
}
//retrieving version number
preg_match($this->pattern, $filename, $matches);
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

You can do this earlier, so you avoid the download if the filename is bad

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Correct, moved higher

if (!isset($matches[1])) {
exit("could not retrieve version from filename '$filename'");
}
$version = $matches[1];
if (strlen($matches[1]) < 1) {
header($_SERVER['SERVER_PROTOCOL'] . ' 400 Bad Request', true, 400);
exit("version found not correct ('$version') from filename '$filename'");
}
//starting real stuff
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks
//create the execution
$stats = $file_contents->stats;
$execution_data = [
'ref' => date('YmdHis'),
'start_date' => date('Y-m-d H:i:s', strtotime($stats->start)),
'end_date' => date('Y-m-d H:i:s', strtotime($stats->end)),
'duration' => $stats->duration,
'version' => $version
];
//let's check if there's not a similar entry...
$entry_date = date('Y-m-d', strtotime($stats->start));
$similar = $this->Execution->findSimilarEntries($entry_date, $version);
if ($similar !== NULL) {
if (!$this->force) {
log_message('error', "A similar entry was found (criteria: version $version and date $entry_date)");
header($_SERVER['SERVER_PROTOCOL'] . ' 409 Conflict', true, 409);
exit("A similar entry was found (criteria: version '$version' and date '$entry_date'). Use the 'force' parameter to force insert");
} else {
log_message('warning', "A similar entry was found (criteria: version $version and date $entry_date) but FORCING insert anyway");
}
}
try {
log_message('info', "Inserting Execution");
$this->execution_id = $this->Execution->insert($execution_data);
} catch(Exception $e) {
header($_SERVER['SERVER_PROTOCOL'] . ' 500 Internal Server Error', true, 500);
exit("could not insert execution");
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
exit("could not insert execution");
exit("failed insert execution");

could not = there was a check or a if and the execution was stopped
failed = I tried but got an error

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Semantic details are tight

}
//launching into orbit
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks
$this->loopThrough($file_contents->suites);
//get the data from the database itself
$updated_data = $this->Execution->getSummaryData($this->execution_id);
$update_data = [
'skipped' => $updated_data->skipped,
'suites' => $updated_data->suites,
'tests' => $updated_data->tests,
'passes' => $updated_data->passed,
'failures' => $updated_data->failed,
'insertion_end_date' => "NOW()"
];
//update the execution row with updated data
$this->Execution->update($update_data, $this->execution_id);
header($_SERVER['SERVER_PROTOCOL'] . ' 200 OK', true, 200);
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

You could create a reusable private function for this, dont you think ?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Thing is, how many different headers do I need ? I could use a function but idk if it's really worth it.
What do you think ? Is it worth it for 3 different headers ?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

I did it.

echo json_encode(['status' => 'ok']);
}
/**
* Check validity of token provided
* @param $token
* @return bool
*/
private function checkToken($token) {
return $token == getenv('QANB_TOKEN');
}
/**
* Loop through data, recursive function
* @param $suite
* @param null $parent_suite_id
*/
private function loopThrough($suite, $parent_suite_id = null) {
$data_suite = [
'execution_id' => $this->execution_id,
'uuid' => $suite->uuid,
'title' => $suite->title,
'campaign' => $this->extractNames($suite->file, 'campaign'),
'file' => $this->extractNames($suite->file, 'file'),
'duration' => $suite->duration,
'hasSkipped' => $suite->hasSkipped ? 1 :0,
'hasPasses' => $suite->hasPasses ? 1 :0,
'hasFailures' => $suite->hasFailures ? 1 :0,
'totalSkipped' => $suite->totalSkipped,
'totalPasses' => $suite->totalPasses,
'totalFailures' => $suite->totalFailures,
'hasSuites' => $suite->hasSuites ? 1 :0,
'hasTests' => $suite->hasTests ? 1 :0,
'parent_id' => $parent_suite_id,
];
//inserting current suite
$suite_id = $this->Suite->insert($data_suite);
if ($suite_id) {
//insert tests
if (count($suite->tests) > 0) {
foreach($suite->tests as $test) {
$data_test = [
'suite_id' => $suite_id,
'uuid' => $test->uuid,
'title' => $test->title,
'state' => $this->getTestState($test),
'duration' => $test->duration,
'error_message' => isset($test->err->message) ? $this->sanitize($test->err->message) : null,
'stack_trace' => isset($test->err->estack) ? $this->sanitize($test->err->estack) : null,
'diff' => isset($test->err->diff) ? $this->sanitize($test->err->diff) : null,
];
$this->Test->insert($data_test);
}
}
//insert children suites
if (count($suite->suites) > 0) {
foreach($suite->suites as $s) {
$this->loopThrough($s, $suite_id);
}
}
} else {
//we don't want to abort, just log this
log_message("error", "Error inserting suite into database.");
}
}
/**
* Sanitize text by removing weird characters
* @param $text
* @return string
*/
private function sanitize($text) {

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

I dont understand what this function does internally. Why if ($CharNo == 163) { $NewStr .= $Char; continue; } ? Can you explain ?

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

This method is a mess. It's the result of a lot of trial and error to sanitize weird characters in the JSON that makes the json_decode method fails. It works and I'm kinda afraid to touch it now.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

As I understand it, it removes characters that are not in a certain range and thus break the json_decode function.

$StrArr = str_split($text);
$NewStr = '';
foreach ($StrArr as $Char) {
$CharNo = ord($Char);
if ($CharNo == 163) { $NewStr .= $Char; continue; }
if ($CharNo > 31 && $CharNo < 127) {
$NewStr .= $Char;
}
}
return $NewStr;
}
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matthieu-rolland

matthieu-rolland Jul 24, 2019

Couldn't this be done with php's sanitization methods? ex:

filter_var($text, FILTER_SANITIZE_STRING);

I'm not sure what this sanitize function removes exactly but there are many filters to choose from:
https://www.php.net/manual/en/filter.filters.sanitize.php

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

TBH this method is a mess but I had a hard time making it work so I'm kinda afraid to touch it now

/**
* Extract campaign name and file name from json data
* @param $filename
* @param $type
* @return mixed|null
*/
private function extractNames($filename, $type)
{
if (strlen($filename) > 0) {
$pattern = '/\/full\/(.*?)\/(.*)/';
preg_match($pattern, $filename, $matches);
if ($type == 'campaign') {
return isset($matches[1]) ? $matches[1] : null;
}
if ($type == 'file') {
return isset($matches[2]) ? $matches[2] : null;
}
}
return null;
}
/**
* Get the test state
* @param $test
* @return string
*/
private function getTestState($test)
{
if (isset($test->state)) {
return $test->state;
}
if ($test->skipped == true) {
return 'skipped';
}
return 'unknown';
}
}
@@ -5,10 +5,16 @@ class MY_Base extends CI_Controller
{
function __construct()
{
parent::__construct();
$db_obj = $this->load->database('default', TRUE);
if(!$db_obj->conn_id) {
exit('Unable to connect with database with given db details.');
try {
parent::__construct();
$db_obj = $this->load->database('default', TRUE);
if(!$db_obj->conn_id) {
header($_SERVER['SERVER_PROTOCOL'] . ' 500 Internal Server Error', true, 500);
exit('Unable to connect with database with given db details.');
}
} catch(Exception $e) {
header($_SERVER['SERVER_PROTOCOL'] . ' 500 Internal Server Error', true, 500);
exit('Unable to connect with database with given db details : '.$e->getMessage());
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Suggested change
exit('Unable to connect with database with given db details : '.$e->getMessage());
exit('Failed to connect with database with given db details : '.$e->getMessage());

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Dont output the exception message in production environment.

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Done

}
}
}
@@ -23,6 +23,16 @@ function find($id)
return $this->db->query("SELECT * FROM $this->table WHERE id = ?;", [$id])->row();
}
public function insert($data)
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

phpDoc 🎶

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Actually doing it on every method... :'(

{
$this->db->insert($this->table, $data);
return $this->db->insert_id();
This conversation was marked as resolved by SimonGrn

This comment has been minimized.

Copy link
@matks

matks Jul 24, 2019

Please leave a blank line before the return (PSR-2 https://www.php-fig.org/psr/psr-2/)

This comment has been minimized.

Copy link
@SimonGrn

SimonGrn Jul 24, 2019

Author Collaborator

Done (damn PSR)

}
public function update($data, $id)
{
return $this->db->update($this->table, $data, "id = $id");
}
function getAllInformation()
{
@@ -105,4 +115,32 @@ function getExecutionPreciseStats($execution_id)
GROUP BY e.id";
return $this->db->query($req, [$execution_id])->row();
}
function getSummaryData($execution_id)
{
$req = "SELECT
COUNT(DISTINCT(s.id)) suites,
COUNT(t.id) tests,
SUM(IF(t.state='passed', 1, 0)) passed,
SUM(IF(t.state='failed', 1, 0)) failed,
SUM(IF(t.state='skipped', 1, 0)) skipped
FROM `execution` e
INNER JOIN `suite` s on s.execution_id = e.id
INNER JOIN `test` t on t.suite_id = s.id
WHERE e.id = ?;";
return $this->db->query($req, [$execution_id])->row();
}
function findSimilarEntries($date, $version)
{
$req = "SELECT id
FROM `execution` e
WHERE version = ?
AND DATE(start_date) = ?
;";
return $this->db->query($req, [$version, $date])->row();
}
}
@@ -30,6 +30,12 @@ function getCampaigns() {
return $this->db->query("SELECT DISTINCT(campaign) FROM $this->table WHERE campaign IS NOT NULL AND campaign != '' ORDER BY campaign;");
}
public function insert($data)
{
$this->db->insert($this->table, $data);
return $this->db->insert_id();
}
function getAllSuitesByFile($execution_id, $campaign, $file)
{
return $this->db->query("SELECT * FROM suite WHERE campaign=? AND file=? AND execution_id=? ORDER BY id", [$campaign, $file, $execution_id]);
@@ -21,6 +21,12 @@ function getBySuiteId($suite_id) {
return $this->db->query("SELECT * FROM $this->table WHERE suite_id = :suite_id ORDER BY id ASC", [':suite_id' => $suite_id]);
}
public function insert($data)
{
$this->db->insert($this->table, $data);
return $this->db->insert_id();
}
function getAllTestsByFile($execution_id, $campaign, $file)
{
return $this->db->query("SELECT t.* FROM test t INNER JOIN suite s ON s.id=t.suite_id WHERE s.campaign=? AND s.file=? AND s.execution_id=?;", [$campaign, $file, $execution_id]);
@@ -1,6 +1,6 @@
{
"description": "CIPSJsonParser description",
"name": "SimonGrn/CIPSJsonParser",
"description": "QA Nightly board - displays useful information from the nightly test execution",
"name": "Prestashop/QANightlyBoard",
"type": "project",
"require": {
"php": ">=5.3.7"
@@ -9,4 +9,4 @@
"mikey179/vfsStream": "1.1.*",
"phpunit/phpunit": "4. *|| 5.*"
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.