Skip to content

Commit 8ac8433

Browse files
author
Michael Babker
committed
Refactor how the list of files in a pull request is processed
1 parent 542e5f2 commit 8ac8433

File tree

3 files changed

+84
-128
lines changed

3 files changed

+84
-128
lines changed

‎administrator/components/com_patchtester/PatchTester/GitHub/GitHub.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public function getFilesForPullRequest($user, $repo, $pullId)
152152
// Build the request path.
153153
$path = "/repos/$user/$repo/pulls/" . (int) $pullId . '/files';
154154

155-
$prepared = $this->prepareRequest($path, 0, 0, $headers);
155+
$prepared = $this->prepareRequest($path, 0, 0);
156156

157157
return $this->processResponse($this->client->get($prepared['url'], $prepared['headers']));
158158
}

‎administrator/components/com_patchtester/PatchTester/Model/PullModel.php‎

Lines changed: 82 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -47,101 +47,47 @@ class PullModel extends \JModelDatabase
4747
);
4848

4949
/**
50-
* Method to parse a patch and extract the affected files
50+
* Parse the list of modified files from a pull request
5151
*
52-
* @param string $patch Patch file to parse
52+
* @param object $files The modified files to parse
5353
*
54-
* @return array Array of files within a patch
54+
* @return array
5555
*
56-
* @since 2.0
56+
* @since __DEPLOY_VERSION__
5757
*/
58-
protected function parsePatch($patch)
58+
protected function parseFileList($files)
5959
{
60-
$state = 0;
61-
$files = array();
60+
$parsedFiles = array();
6261

63-
$lines = explode("\n", $patch);
64-
65-
foreach ($lines as $line)
62+
foreach ($files as $file)
6663
{
67-
switch ($state)
64+
/*
65+
* Check if the patch tester is running in a production environment
66+
* If so, do not patch certain files as errors will be thrown
67+
*/
68+
if (!file_exists(JPATH_ROOT . '/installation/index.php'))
6869
{
69-
case 0:
70-
if (strpos($line, 'diff --git') === 0)
71-
{
72-
$state = 1;
73-
}
74-
75-
$file = new \stdClass;
76-
$file->action = 'modified';
77-
78-
break;
79-
80-
case 1:
81-
if (strpos($line, 'index') === 0)
82-
{
83-
$file->index = substr($line, 6);
84-
}
85-
86-
if (strpos($line, '---') === 0)
87-
{
88-
$file->old = substr($line, 6);
89-
}
90-
91-
if (strpos($line, '+++') === 0)
92-
{
93-
$file->new = substr($line, 6);
94-
}
95-
96-
if (strpos($line, 'new file mode') === 0)
97-
{
98-
$file->action = 'added';
99-
}
100-
101-
if (strpos($line, 'deleted file mode') === 0)
102-
{
103-
$file->action = 'deleted';
104-
}
105-
106-
// Binary files are presently unsupported, use this to reset the parser in the meantime
107-
if (strpos($line, 'Binary files') === 0)
108-
{
109-
$state = 0;
110-
111-
$this->state->set('pull.has_binary', true);
112-
}
113-
114-
if (strpos($line, '@@') === 0)
115-
{
116-
$state = 0;
70+
$filePath = explode('/', $file->filename);
11771

118-
/*
119-
* Check if the patch tester is running in a production environment
120-
* If so, do not patch certain files as errors will be thrown
121-
*/
122-
if (!file_exists(JPATH_ROOT . '/installation/index.php'))
123-
{
124-
$filePath = explode('/', $file->new);
125-
126-
if (in_array($filePath[0], $this->nonProductionFiles))
127-
{
128-
continue;
129-
}
130-
131-
if (in_array($filePath[0], $this->nonProductionFolders))
132-
{
133-
continue;
134-
}
135-
}
136-
137-
$files[] = $file;
138-
}
72+
if (in_array($filePath[0], $this->nonProductionFiles))
73+
{
74+
continue;
75+
}
13976

140-
break;
77+
if (in_array($filePath[0], $this->nonProductionFolders))
78+
{
79+
continue;
80+
}
14181
}
82+
83+
$parsedFiles[] = (object) array(
84+
'action' => $file->status,
85+
'filename' => $file->filename,
86+
'fileurl' => $file->contents_url,
87+
);
14288
}
14389

144-
return $files;
90+
return $parsedFiles;
14591
}
14692

14793
/**
@@ -194,49 +140,62 @@ public function apply($id)
194140

195141
try
196142
{
197-
$patchResponse = $github->getDiffForPullRequest($this->getState()->get('github_user'), $this->getState()->get('github_repo'), $id);
198-
$patch = json_decode($patchResponse->body);
143+
$filesResponse = $github->getFilesForPullRequest($this->getState()->get('github_user'), $this->getState()->get('github_repo'), $id);
144+
$files = json_decode($filesResponse->body);
199145
}
200146
catch (UnexpectedResponse $e)
201147
{
202148
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_COULD_NOT_CONNECT_TO_GITHUB', $e->getMessage()), $e->getCode(), $e);
203149
}
204150

205-
$files = $this->parsePatch($patch);
206-
207-
if (!$files)
151+
if (!count($files))
208152
{
209153
return false;
210154
}
211155

212-
foreach ($files as $file)
156+
$parsedFiles = $this->parseFileList($files);
157+
158+
foreach ($parsedFiles as $file)
213159
{
214-
if ($file->action == 'deleted' && !file_exists(JPATH_ROOT . '/' . $file->old))
160+
if ($file->action == 'deleted' && !file_exists(JPATH_ROOT . '/' . $file->filename))
215161
{
216162
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_FILE_DELETED_DOES_NOT_EXIST_S', $file->old));
217163
}
218164

219165
if ($file->action == 'added' || $file->action == 'modified')
220166
{
221167
// If the backup file already exists, we can't apply the patch
222-
if (file_exists(JPATH_COMPONENT . '/backups/' . md5($file->new) . '.txt'))
168+
if (file_exists(JPATH_COMPONENT . '/backups/' . md5($file->filename) . '.txt'))
223169
{
224-
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_CONFLICT_S', $file->new));
170+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_CONFLICT_S', $file->filename));
225171
}
226172

227-
if ($file->action == 'modified' && !file_exists(JPATH_ROOT . '/' . $file->old))
173+
if ($file->action == 'modified' && !file_exists(JPATH_ROOT . '/' . $file->filename))
228174
{
229-
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_FILE_MODIFIED_DOES_NOT_EXIST_S', $file->old));
175+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_FILE_MODIFIED_DOES_NOT_EXIST_S', $file->filename));
230176
}
231177

232-
$url = 'https://raw.github.com/' . urlencode($pull->head->user->login) . '/' . urlencode($pull->head->repo->name)
233-
. '/' . urlencode($pull->head->ref) . '/' . $file->new;
234-
235178
try
236179
{
237-
$file->body = $github->getClient()->get($url)->body;
180+
$contentsResponse = $github->getFileContents(
181+
$this->getState()->get('github_user'), $this->getState()->get('github_repo'), $file->filename, urlencode($pull->head->ref)
182+
);
183+
184+
$contents = json_decode($contentsResponse->body);
185+
186+
// In case encoding type ever changes
187+
switch ($contents->encoding)
188+
{
189+
case 'base64':
190+
$file->body = base64_decode($contents->content);
191+
192+
break;
193+
194+
default:
195+
throw new \RuntimeException(\JText::_('COM_PATCHTESTER_ERROR_UNSUPPORTED_ENCODING'));
196+
}
238197
}
239-
catch (\Exception $e)
198+
catch (UnexpectedResponse $e)
240199
{
241200
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_COULD_NOT_CONNECT_TO_GITHUB', $e->getMessage()), $e->getCode(), $e);
242201
}
@@ -247,38 +206,35 @@ public function apply($id)
247206
jimport('joomla.filesystem.path');
248207

249208
// At this point, we have ensured that we have all the new files and there are no conflicts
250-
foreach ($files as $file)
209+
foreach ($parsedFiles as $file)
251210
{
252211
// We only create a backup if the file already exists
253-
if ($file->action == 'deleted' || (file_exists(JPATH_ROOT . '/' . $file->new) && $file->action == 'modified'))
212+
if ($file->action == 'deleted' || (file_exists(JPATH_ROOT . '/' . $file->filename) && $file->action == 'modified'))
254213
{
255-
if (!\JFile::copy(\JPath::clean(JPATH_ROOT . '/' . $file->old), JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'))
214+
$src = JPATH_ROOT . '/' . $file->filename;
215+
$dest = JPATH_COMPONENT . '/backups/' . md5($file->filename) . '.txt';
216+
217+
if (!\JFile::copy(\JPath::clean($src), $dest))
256218
{
257-
throw new \RuntimeException(
258-
\JText::sprintf(
259-
'COM_PATCHTESTER_ERROR_CANNOT_COPY_FILE',
260-
JPATH_ROOT . '/' . $file->old,
261-
JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'
262-
)
263-
);
219+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_COPY_FILE', $src, $dest));
264220
}
265221
}
266222

267223
switch ($file->action)
268224
{
269225
case 'modified':
270226
case 'added':
271-
if (!\JFile::write(\JPath::clean(JPATH_ROOT . '/' . $file->new), $file->body))
227+
if (!\JFile::write(\JPath::clean(JPATH_ROOT . '/' . $file->filename), $file->body))
272228
{
273-
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_WRITE_FILE', JPATH_ROOT . '/' . $file->new));
229+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_WRITE_FILE', JPATH_ROOT . '/' . $file->filename));
274230
}
275231

276232
break;
277233

278234
case 'deleted':
279-
if (!\JFile::delete(\JPath::clean(JPATH_ROOT . '/' . $file->old)))
235+
if (!\JFile::delete(\JPath::clean(JPATH_ROOT . '/' . $file->filename)))
280236
{
281-
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', JPATH_ROOT . '/' . $file->old));
237+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', JPATH_ROOT . '/' . $file->filename));
282238
}
283239

284240
break;
@@ -287,7 +243,7 @@ public function apply($id)
287243

288244
$record = (object) array(
289245
'pull_id' => $pull->number,
290-
'data' => json_encode($files),
246+
'data' => json_encode($parsedFiles),
291247
'patched_by' => \JFactory::getUser()->id,
292248
'applied' => 1,
293249
'applied_version' => JVERSION,
@@ -350,36 +306,35 @@ public function revert($id)
350306
{
351307
case 'deleted':
352308
case 'modified':
353-
if (!\JFile::copy(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt', JPATH_ROOT . '/' . $file->old))
309+
$src = JPATH_COMPONENT . '/backups/' . md5($file->filename) . '.txt';
310+
$dest = JPATH_ROOT . '/' . $file->filename;
311+
312+
if (!\JFile::copy($src, $dest))
354313
{
355-
throw new \RuntimeException(
356-
\JText::sprintf(
357-
'COM_PATCHTESTER_ERROR_CANNOT_COPY_FILE',
358-
JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt',
359-
JPATH_ROOT . '/' . $file->old
360-
)
361-
);
314+
throw new \RuntimeException(\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_COPY_FILE', $src, $dest));
362315
}
363316

364-
if (file_exists(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'))
317+
if (file_exists($src))
365318
{
366-
if (!\JFile::delete(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'))
319+
if (!\JFile::delete($src))
367320
{
368321
throw new \RuntimeException(
369-
\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt')
322+
\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', $src)
370323
);
371324
}
372325
}
373326

374327
break;
375328

376329
case 'added':
377-
if (file_exists(JPATH_ROOT . '/' . $file->new))
330+
$src = JPATH_ROOT . '/' . $file->filename;
331+
332+
if (file_exists($src))
378333
{
379-
if (!\JFile::delete(JPATH_ROOT . '/' . $file->new))
334+
if (!\JFile::delete($src))
380335
{
381336
throw new \RuntimeException(
382-
\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', JPATH_ROOT . '/' . $file->new)
337+
\JText::sprintf('COM_PATCHTESTER_ERROR_CANNOT_DELETE_FILE', $src)
383338
);
384339
}
385340
}

‎administrator/components/com_patchtester/language/en-GB/en-GB.com_patchtester.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ COM_PATCHTESTER_ERROR_MODEL_NOT_FOUND="Model class %s not found."
2424
COM_PATCHTESTER_ERROR_READING_DATABASE_TABLE="%1$s - Error retrieving table data (%2$s)"
2525
COM_PATCHTESTER_ERROR_TRUNCATING_PULLS_TABLE="Error truncating the pulls table: %s"
2626
COM_PATCHTESTER_ERROR_TRUNCATING_TESTS_TABLE="Error truncating the tests table: %s"
27+
COM_PATCHTESTER_ERROR_UNSUPPORTED_ENCODING="The patch's files are encoded in an unsupported format."
2728
COM_PATCHTESTER_ERROR_VIEW_NOT_FOUND="View not found [name, format]: %1$s, %2$s"
2829
COM_PATCHTESTER_FETCH_AN_ERROR_HAS_OCCURRED="An error has occurred while fetching the data from GitHub."
2930
COM_PATCHTESTER_FETCH_COMPLETE_CLOSE_WINDOW="All data has been retrieved. Please close this modal window to refresh the page."

0 commit comments

Comments
 (0)