Skip to content

Commit b12acc2

Browse files
nolanpronolanpro
committed
Update script runner to use the saved script object and fix tests
1 parent cb01dcc commit b12acc2

10 files changed

Lines changed: 66 additions & 35 deletions

File tree

‎ProcessMaker/Http/Controllers/Api/ScriptController.php‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,16 @@ public function index(Request $request)
9191
* Previews executing a script, with sample data/config data
9292
*
9393
* @OA\Get(
94-
* path="/scripts/ew",
95-
* summary="Returns all scripts that the user has access to",
94+
* path="/scripts/{script_id}/preview",
95+
* summary="Test script code without saving it",
9696
* operationId="getScriptsPreview",
9797
* tags={"Scripts"},
9898
* @OA\Parameter(
99+
* name="script_id",
100+
* in="path",
101+
* @OA\Schema(type="integer"),
102+
* ),
103+
* @OA\Parameter(
99104
* name="data",
100105
* in="query",
101106
* @OA\Schema(type="string"),
@@ -110,11 +115,6 @@ public function index(Request $request)
110115
* in="query",
111116
* @OA\Schema(type="string"),
112117
* ),
113-
* @OA\Parameter(
114-
* name="language",
115-
* in="query",
116-
* @OA\Schema(type="string"),
117-
* ),
118118
*
119119
* @OA\Response(
120120
* response=200,
@@ -128,11 +128,9 @@ public function preview(Request $request, Script $script)
128128
{
129129
$data = json_decode($request->get('data'), true) ?: [];
130130
$config = json_decode($request->get('config'), true) ?: [];
131+
$code = $request->get('code');
131132

132-
# set attribute but do not save (for preview only)
133-
$script->code = $request->get('code');
134-
135-
TestScript::dispatch($script, $data, $config);
133+
TestScript::dispatch($script, $code, $data, $config);
136134
return ['status' => 'success'];
137135
}
138136

‎ProcessMaker/Jobs/RunScriptTask.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public function action(TokenInterface $token, ScriptTaskInterface $element)
5656
if (empty($scriptRef)) {
5757
$script = new Script([
5858
'code' => $element->getScript(),
59-
'language' => Script::scriptFormat2Language($element->getProperty('scriptFormat', 'application/x-php'))
59+
'language' => Script::scriptFormat2Language($element->getProperty('scriptFormat', 'application/x-php')),
60+
'run_as_user_id' => Script::defaultRunAsUser()->id,
6061
]);
6162
} else {
6263
$script = Script::find($scriptRef);

‎ProcessMaker/Jobs/TestScript.php‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,28 +19,23 @@ class TestScript implements ShouldQueue
1919
Queueable,
2020
SerializesModels;
2121

22+
protected $script;
2223
protected $code;
23-
protected $language;
24-
protected $timeout;
25-
protected $user;
2624
protected $data;
2725
protected $configuration;
2826

2927
/**
3028
* Create a new job instance to execute a script.
3129
*
30+
* @param ProcessMaker\Models\Script $script
3231
* @param string $code
33-
* @param string $language
34-
* @param string $timeout
3532
* @param array $data
3633
* @param array $configuration
3734
*/
38-
public function __construct($code, $language, $timeout, User $user, array $data, array $configuration)
35+
public function __construct($script, $code, array $data, array $configuration)
3936
{
37+
$this->script = $script;
4038
$this->code = $code;
41-
$this->language = $language;
42-
$this->timeout = $timeout;
43-
$this->user = $user;
4439
$this->data = $data;
4540
$this->configuration = $configuration;
4641
}
@@ -53,12 +48,9 @@ public function __construct($code, $language, $timeout, User $user, array $data,
5348
public function handle()
5449
{
5550
try {
56-
$script = new Script([
57-
'code' => $this->code,
58-
'language' => $this->language,
59-
'timeout' => $this->timeout,
60-
]);
61-
$response = $script->runScript($this->data, $this->configuration);
51+
# Just set the code but do not save the object (preview only)
52+
$this->script->code = $this->code;
53+
$response = $this->script->runScript($this->data, $this->configuration);
6254
$this->sendResponse(200, $response);
6355
} catch (Throwable $exception) {
6456
$this->sendResponse(500, [
@@ -76,6 +68,6 @@ public function handle()
7668
*/
7769
private function sendResponse($status, array $response)
7870
{
79-
$this->user->notify(new ScriptResponseNotification($status, $response));
71+
$this->script->runAsUser->notify(new ScriptResponseNotification($status, $response));
8072
}
8173
}

‎ProcessMaker/Models/Script.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,23 @@ public function versions()
195195
{
196196
return $this->hasMany(ScriptVersion::class);
197197
}
198+
199+
/**
200+
* Get the associated run_as_user
201+
*/
202+
public function runAsUser()
203+
{
204+
return $this->belongsTo(User::class, 'run_as_user_id');
205+
}
206+
207+
/**
208+
* Return the a user for service tasks
209+
*
210+
* @return ProcessMaker\Models\User
211+
*/
212+
public static function defaultRunAsUser()
213+
{
214+
# return the default admin user
215+
return User::where('is_administrator', true)->firstOrFail();
216+
}
198217
}

‎ProcessMaker/ScriptRunners/Base.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ abstract public function config($code, array $dockerConfig);
4646
*/
4747
public function run($code, array $data, array $config, $timeout, ?User $user)
4848
{
49-
// Prepate the docker parameters
49+
// Prepare the docker parameters
5050
$environmentVariables = $this->getEnvironmentVariables();
5151

5252
// Create tokens for the SDK if a user is set

‎ProcessMaker/ScriptRunners/ScriptRunner.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ public function __construct($language)
3232
*/
3333
public function run($code, array $data, array $config, $timeout = 60, $user)
3434
{
35-
// throw new \Exception("HERE " . $user->id);
3635
return $this->runner->run($code, $data, $config, $timeout, $user);
3736
}
3837

‎storage/api-docs/api-docs.json‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎tests/Feature/Api/ScriptsTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ public function testPreviewScript()
331331
$url = route('api.script.preview', $this->getScript('php')->id);
332332
$response = $this->apiCall('POST', $url, ['data' => '{}', 'code' => '<?php return ["response"=>1];']);
333333
$response->assertStatus(200);
334-
334+
335335
// Assertion: The script output is sent to usr through broadcast channel
336336
Notification::assertSentTo(
337337
[$this->user],
@@ -349,8 +349,8 @@ function ($notification, $channels) {
349349
public function testPreviewScriptFail()
350350
{
351351
Notification::fake();
352-
$url = self::API_TEST_SCRIPT . '/preview/?data=adkasdlasj&config=&code=adkasdlasj&language=JAVA';
353-
$response = $this->apiCall('POST', $url, []);
352+
$url = route('api.script.preview', $this->getScript('foo')->id);
353+
$response = $this->apiCall('POST', $url, ['data' => 'foo', 'config' => 'foo', 'code' => 'foo']);
354354

355355
// Assertion: An exception is notified to usr through broadcast channel
356356
Notification::assertSentTo(

‎tests/Feature/Api/ServiceTaskExecutionTest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,19 @@ protected function withUserSetUp()
4747
'key' => 'EchoConnector',
4848
'language' => 'php',
4949
'code' => '<?php return ["pong" => $data["ping"]];',
50+
'run_as_user_id' => $this->user->id,
5051
]);
5152
$this->process = $this->createTestProcess();
5253
}
54+
55+
/**
56+
* Make sure we have a personal access client set up
57+
*
58+
*/
59+
public function setUpWithPersonalAccessClient()
60+
{
61+
$this->withPersonalAccessClient();
62+
}
5363

5464
/**
5565
* Create a single task process assigned to $this->user

‎tests/Feature/Docker/TimeoutsTest.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,15 @@ private function skipWithoutDocker()
3333
return $this->markTestSkipped('This test requires docker');
3434
}
3535
}
36+
37+
/**
38+
* Make sure we have a personal access client set up
39+
*
40+
*/
41+
public function setUpWithPersonalAccessClient()
42+
{
43+
$this->withPersonalAccessClient();
44+
}
3645

3746
/**
3847
* Run a test script and assert that the specified timeout is exceeded
@@ -48,7 +57,10 @@ private function assertTimeoutExceeded($data)
4857
);
4958

5059
$this->benchmarkStart();
51-
$response = $this->apiCall('POST', $url, $data);
60+
$response = $this->apiCall('POST', $url, [
61+
'code' => $data['code'],
62+
'data' => $data['data'],
63+
]);
5264
$this->benchmarkEnd();
5365

5466
$this->assertLogMessageExists('Script timed out');

0 commit comments

Comments
 (0)