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.

Sign up for GitHub

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

Allow a Run to be imported over the network #230

Merged
merged 2 commits into from
Mar 11, 2018
Merged

Allow a Run to be imported over the network #230

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac0848a
Allow a Run to be imported over the network
Jan 23, 2018
9f99698
PSR-2 fixes
Feb 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/Xhgui/Controller/Run.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,21 @@

class Xhgui_Controller_Run extends Xhgui_Controller
{
/**
* @var \Slim\Slim
*/
protected $_app;
Copy link
Contributor

Choose a reason for hiding this comment

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

Property name "$_app" should not be prefixed with an underscore to indicate visibility

Copy link
Author

Choose a reason for hiding this comment

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

I have used the same style as used elsewhere in the application.

Copy link
Member

Choose a reason for hiding this comment

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

There is some work to be done in order to get the code up to a PSR2 level.

Copy link
Author

Choose a reason for hiding this comment

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

Is that required for the passing of this PR? I kept as close to the original coding style as possible. I'm perfectly willing to bring the code that I've touched in line with PSR2.

However, adding "Upgrade the whole project to PSR-2" in the critical path would completely destroy the cost-benefit ratio of pursuing this PR.

Copy link

Choose a reason for hiding this comment

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

You don't need to upgrade the entire project to PSR-2 but you should make sure your code changes pass the build and follow the chosen coding standards.

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough. As I said: I'm perfectly willing to fix the code I touched.

Already did so, and also a few small other CS fixes. See the last commit.


/**
* @var Xhgui_Profiles
*/
private $_profiles;
Copy link
Contributor

Choose a reason for hiding this comment

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

Property name "$_profiles" should not be prefixed with an underscore to indicate visibility

Copy link
Author

Choose a reason for hiding this comment

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

I have used the same style as used elsewhere in the application.


/**
* @var Xhgui_WatchFunctions
*/
private $_watches;
Copy link
Contributor

Choose a reason for hiding this comment

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

Property name "$_watches" should not be prefixed with an underscore to indicate visibility

Copy link
Author

Choose a reason for hiding this comment

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

I have used the same style as used elsewhere in the application.


public function __construct($app, $profiles, $watches)
{
$this->_app = $app;
Expand Down Expand Up @@ -309,4 +324,17 @@ public function callgraphDataDot()
$response['Content-Type'] = 'application/json';
return $response->body(json_encode($callgraph));
}

public function import()
{
$request = $this->_app->request();

$data = json_decode($request->getBody(), true);
Copy link
Member

Choose a reason for hiding this comment

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

There is no authentication, authorization or checksums on this data right now. Should there be?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I realize that. It was a conscious choice, since there were none of these in the rest of the project as well.

Furthermore, I assumed that XHGui, being a tool that is primarily used during development (imho), would be deployed in a somewhat trusted environment, like a staging server, or a developers' workstation.


$container = Xhgui_ServiceContainer::instance();
/* @var Xhgui_Saver_Mongo $saver */
$saver = $container['saverMongo'];

$saver->save($data);
}
}
5 changes: 5 additions & 0 deletions src/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
$app->controller->view();
})->name('run.view');

$app->post('/run/import', function () use ($di, $app) {
$app->controller = $di['runController'];
$app->controller->import();
})->name('run.import');

$app->get('/url/view', function () use ($di, $app) {
$app->controller = $di['runController'];
$app->controller->url();
Expand Down