From cf77d5744f9c930269a3ca92bb0d5a95f770b1ae Mon Sep 17 00:00:00 2001
From: Aaron Bauman <aaron@messageagency.com>
Date: Mon, 11 Mar 2019 16:15:30 -0400
Subject: [PATCH] Issue #3038693: Reduce the copious number of exceptions
---
modules/salesforce_mapping/src/PushParams.php | 25 +++++++----
src/Rest/RestClient.php | 8 ++--
src/Rest/RestClientInterface.php | 17 +++----
src/SFID.php | 31 ++++++++++++-
src/SObject.php | 44 ++++++++++++++++---
src/SelectQueryResult.php | 18 +++-----
tests/src/Unit/RestClientTest.php | 4 +-
tests/src/Unit/SObjectTest.php | 4 +-
tests/src/Unit/SelectQueryResultTest.php | 6 +--
9 files changed, 105 insertions(+), 52 deletions(-)
diff --git a/modules/salesforce_mapping/src/PushParams.php b/modules/salesforce_mapping/src/PushParams.php
index 17c032d4..9ae49077 100644
--- a/modules/salesforce_mapping/src/PushParams.php
+++ b/modules/salesforce_mapping/src/PushParams.php
@@ -75,17 +75,26 @@ class PushParams {
* @param string $key
* A param key.
*
- * @return mixed
- * The given param value for $key
+ * @return mixed|null
+ * The given param value for $key, or NULL if $key is not set.
*
- * @throws \Exception
- * If the key doesn't exist.
+ * @see hasParam()
*/
public function getParam($key) {
- if (!array_key_exists($key, $this->params)) {
- throw new \Exception("Param key $key does not exist");
- }
- return $this->params[$key];
+ return static::hasParam($key) ? $this->params[$key] : NULL;
+ }
+
+ /**
+ * Return TRUE if the given $key is set.
+ *
+ * @param $key
+ * A key.
+ *
+ * @return bool
+ * TRUE if $key is set.
+ */
+ public function hasParam($key) {
+ return array_key_exists($key, $this->params);
}
/**
diff --git a/src/Rest/RestClient.php b/src/Rest/RestClient.php
index a5d60d62..7edaa4df 100644
--- a/src/Rest/RestClient.php
+++ b/src/Rest/RestClient.php
@@ -594,11 +594,11 @@ class RestClient implements RestClientInterface {
if ($name != NULL) {
if (!isset($record_types[$name])) {
- throw new \Exception("No record types for $name");
+ return FALSE;
}
return $record_types[$name];
}
- return $record_types;
+ return $record_types ?: FALSE;
}
/**
@@ -607,7 +607,7 @@ class RestClient implements RestClientInterface {
public function getRecordTypeIdByDeveloperName($name, $devname, $reset = FALSE) {
$record_types = $this->getRecordTypes($name, $reset);
if (empty($record_types[$devname])) {
- throw new \Exception("No record type $devname for $name");
+ return FALSE;
}
return $record_types[$devname]->id();
}
@@ -623,7 +623,7 @@ class RestClient implements RestClientInterface {
return $object['name'];
}
}
- throw new \Exception('No matching object type');
+ return FALSE;
}
/**
diff --git a/src/Rest/RestClientInterface.php b/src/Rest/RestClientInterface.php
index 77cd98cb..b7c18398 100644
--- a/src/Rest/RestClientInterface.php
+++ b/src/Rest/RestClientInterface.php
@@ -371,9 +371,10 @@ interface RestClientInterface {
* @param string $name
* Object type name, e.g. Contact, Account, etc.
*
- * @return array
+ * @return array|false
* If $name is given, a record type array indexed by developer name.
* Otherwise, an array of record type arrays, indexed by object type name.
+ * FALSE if no record types found.
*/
public function getRecordTypes($name = NULL);
@@ -390,11 +391,8 @@ interface RestClientInterface {
* @param bool $reset
* If true, clear the local cache and fetch record types from API.
*
- * @return \Drupal\salesforce\SFID
- * The Salesforce ID of the given Record Type, or null.
- *
- * @throws \Exception
- * If record type is not found.
+ * @return \Drupal\salesforce\SFID|false
+ * The Salesforce ID of the given Record Type, or FALSE if not found.
*/
public function getRecordTypeIdByDeveloperName($name, $devname, $reset = FALSE);
@@ -404,11 +402,8 @@ interface RestClientInterface {
* @param \Drupal\salesforce\SFID $id
* The SFID.
*
- * @return string
- * The object type name.
- *
- * @throws \Exception
- * If SFID doesn't match any object type.
+ * @return string|false
+ * The object type name, or FALSE if not found.
*/
public function getObjectTypeName(SFID $id);
diff --git a/src/SFID.php b/src/SFID.php
index 6ddd8724..573b9df7 100644
--- a/src/SFID.php
+++ b/src/SFID.php
@@ -26,10 +26,39 @@ class SFID {
}
$this->id = $id;
if (strlen($this->id) == 15) {
- $this->id = self::convertId($id);
+ $this->id = static::convertId($id);
}
}
+ /**
+ * Given a potential SFID, return a new SFID object if it's valid.
+ *
+ * @param $id
+ * A potential sfid.
+ *
+ * @return \Drupal\salesforce\SFID|false
+ * A new SFID if $id is valid, otherwise FALSE.
+ */
+ public static function createIfValid($id) {
+ if (static::isValid($id)) {
+ return new static($id);
+ }
+ return FALSE;
+ }
+
+ /**
+ * Returns TRUE if $id is a valid length.
+ *
+ * @param $id
+ * An sfid.
+ *
+ * @return bool
+ * Returns TRUE if $id is a valid length.
+ */
+ public static function isValid($id) {
+ return strlen($id) == 15 || strlen($id) == self::MAX_LENGTH;
+ }
+
/**
* Magic method wrapping the SFID string.
*
diff --git a/src/SObject.php b/src/SObject.php
index 7067675b..10e9d248 100644
--- a/src/SObject.php
+++ b/src/SObject.php
@@ -17,8 +17,6 @@ class SObject {
*
* @param array $data
* The SObject field data.
- *
- * @throws \Exception
*/
public function __construct(array $data = []) {
if (!isset($data['id']) && !isset($data['Id'])) {
@@ -45,6 +43,31 @@ class SObject {
$this->fields['Id'] = (string) $this->id;
}
+ /**
+ * Given SObject data, instantiate a new SObject if data is valid.
+ *
+ * @param array $data
+ * SOBject data.
+ *
+ * @return \Drupal\salesforce\SObject|false
+ * SObject, or FALSE if data is not valid.
+ */
+ public static function createIfValid(array $data = []) {
+ if (!isset($data['id']) && !isset($data['Id'])) {
+ return FALSE;
+ }
+ if (isset($data['id'])) {
+ $data['Id'] = $data['id'];
+ }
+ if (!SFID::isValid($data['Id'])) {
+ return FALSE;
+ }
+ if (empty($data['attributes']) || !isset($data['attributes']['type'])) {
+ return FALSE;
+ }
+ return new static($data);
+ }
+
/**
* SFID Getter.
*
@@ -78,15 +101,24 @@ class SObject {
/**
* Given $key, return corresponding field value.
*
- * @return mixed
+ * @return mixed|false
* The value.
+ */
+ public function hasField($key) {
+ return array_key_exists($key, $this->fields);
+ }
+
+ /**
+ * Given $key, return corresponding field value.
+ *
+ * @return mixed|NULL
+ * The value, or NULL if given $key is not set.
*
- * @throws \Exception
- * If $key is not found.
+ * @see hasField()
*/
public function field($key) {
if (!array_key_exists($key, $this->fields)) {
- throw new \Exception('Index not found');
+ return NULL;
}
return $this->fields[$key];
}
diff --git a/src/SelectQueryResult.php b/src/SelectQueryResult.php
index 5634a400..fd3e0897 100644
--- a/src/SelectQueryResult.php
+++ b/src/SelectQueryResult.php
@@ -19,8 +19,6 @@ class SelectQueryResult {
*
* @param array $results
* The query results.
- *
- * @throws \Exception
*/
public function __construct(array $results) {
$this->totalSize = $results['totalSize'];
@@ -30,7 +28,9 @@ class SelectQueryResult {
}
$this->records = [];
foreach ($results['records'] as $record) {
- $this->records[$record['Id']] = new SObject($record);
+ if ($sobj = SObject::createIfValid($record)) {
+ $this->records[$record['Id']] = $sobj;
+ }
}
}
@@ -80,17 +80,11 @@ class SelectQueryResult {
* @param \Drupal\salesforce\SFID $id
* The SFID.
*
- * @return \Drupal\salesforce\SObject
- * The record.
- *
- * @throws \Exception
- * If the given SFID doesn't exist in these results.
+ * @return \Drupal\salesforce\SObject|false
+ * The record, or FALSE if no record exists for given id.
*/
public function record(SFID $id) {
- if (!isset($this->records[(string) $id])) {
- throw new \Exception('No record found');
- }
- return $this->records[(string) $id];
+ return isset($this->records[(string) $id]) ? $this->records[(string) $id] : FALSE;
}
}
diff --git a/tests/src/Unit/RestClientTest.php b/tests/src/Unit/RestClientTest.php
index b7e88cf3..3094f8e9 100644
--- a/tests/src/Unit/RestClientTest.php
+++ b/tests/src/Unit/RestClientTest.php
@@ -447,8 +447,6 @@ class RestClientTest extends UnitTestCase {
/**
* @covers ::getRecordTypes
- *
- * @expectedException Exception
*/
public function testGetRecordTypes() {
$this->initClient(array_merge($this->methods, ['query']));
@@ -498,7 +496,7 @@ class RestClientTest extends UnitTestCase {
$this->assertEquals($recordTypes[$sObjectType], $this->client->getRecordTypes($sObjectType));
- $this->client->getRecordTypes('fail');
+ $this->assertFalse($this->client->getRecordTypes('fail'));
}
}
diff --git a/tests/src/Unit/SObjectTest.php b/tests/src/Unit/SObjectTest.php
index ee2b4097..5b4ac10f 100644
--- a/tests/src/Unit/SObjectTest.php
+++ b/tests/src/Unit/SObjectTest.php
@@ -51,12 +51,10 @@ class SObjectTest extends UnitTestCase {
/**
* Test invalid field call.
- *
- * @expectedException \Exception
*/
public function testFieldNotExists() {
$sobject = new SObject(['id' => '1234567890abcde', 'attributes' => ['type' => 'dummy']]);
- $sobject->field('key');
+ $this->assertNull($sobject->field('key'));
}
/**
diff --git a/tests/src/Unit/SelectQueryResultTest.php b/tests/src/Unit/SelectQueryResultTest.php
index 10603d62..8e6713bc 100644
--- a/tests/src/Unit/SelectQueryResultTest.php
+++ b/tests/src/Unit/SelectQueryResultTest.php
@@ -46,13 +46,11 @@ class SelectQueryResultTest extends UnitTestCase {
}
/**
- * Test object instantiation with no ID.
- *
- * @expectedException \Exception
+ * Test object instantiation with non-existent ID.
*/
public function testNoId() {
$sfid = new SFID('1234567890abcdg');
- $this->sqr->record($sfid);
+ $this->assertFalse($this->sqr->record($sfid));
}
}
--
GitLab