From 2814b24915c121edaef144c1422f3989a835ce03 Mon Sep 17 00:00:00 2001 From: Ryan Morgan Date: Thu, 16 Jul 2009 11:38:02 -0700 Subject: [PATCH] [HHQ-3244] Add better error handling to Resource create and updates. --- ChangeLog | 2 + hqu/hqapi1/app/ResourceController.groovy | 81 +++++++++++-------- .../test/ResourceCreatePlatform_test.java | 45 +++++++++++ .../test/ResourceCreateServer_test.java | 39 +++++++++ .../test/ResourceCreateService_test.java | 42 ++++++++++ .../hq/hqapi1/test/ResourceUpdate_test.java | 30 +++++++ 6 files changed, 204 insertions(+), 35 deletions(-) diff --git a/ChangeLog b/ChangeLog index dbf671b7..c11a2baa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,8 @@ Changes in HQApi 2.2 + *) [HHQ-3244] Add better error handling to Resource create and updates. + *) [HHQ-3230] Add --conditionTypeExclude to allow definitions to be excluded based on conditions. As part of this change --conditionType has been renamed to --conditionTypeInclude diff --git a/hqu/hqapi1/app/ResourceController.groovy b/hqu/hqapi1/app/ResourceController.groovy index 10b6b658..8883580d 100644 --- a/hqu/hqapi1/app/ResourceController.groovy +++ b/hqu/hqapi1/app/ResourceController.groovy @@ -108,7 +108,14 @@ class ResourceController extends ApiController { } } } - + + private String getCause(Throwable t) { + while (t.getCause()) { + t = t.getCause() + } + return t.getMessage() + } + def getResourcePrototypes(params) { def existing = params.getOne('existing')?.toBoolean() @@ -232,14 +239,16 @@ class ResourceController extends ApiController { try { resource = prototype.createInstance(parent, resourceXml.'@name', user, cfg, agent, ips) - } catch (Exception e) { - // TODO: Fix this + } catch (Throwable t) { + String cause = getCause(t) renderXml() { ResourceResponse() { - out << getFailureXML(ErrorCode.OBJECT_EXISTS); + out << getFailureXML(ErrorCode.INVALID_PARAMETERS, + "Error creating '" + + resourceXml.'@name' + "': " + cause) } } - log.warn("Error creating resource", e) + log.warn("Error creating resource", t) return } @@ -307,22 +316,17 @@ class ResourceController extends ApiController { try { resource = prototype.createInstance(parent, resourceXml.'@name', user, cfg) - } catch (IllegalArgumentException e) { + } catch (Throwable t) { + String cause = getCause(t) renderXml() { ResourceResponse() { out << getFailureXML(ErrorCode.INVALID_PARAMETERS, - e.getMessage()) - } - } - return - } catch (Exception e) { - // TODO: Fix this - renderXml() { - ResourceResponse() { - out << getFailureXML(ErrorCode.OBJECT_EXISTS); + "Error creating '" + + resourceXml.'@name' + "': " + + cause); } } - log.warn("Error creating resource", e) + log.warn("Error creating resource", t) return } @@ -561,7 +565,7 @@ class ResourceController extends ApiController { "Resource name not given") } - def resource + def resource = null if (id) { resource = getResource(id) } @@ -632,7 +636,14 @@ class ResourceController extends ApiController { // Update if (!configsEqual(resource.getConfig(), config)) { - resource.setConfig(config, user) + try { + resource.setConfig(config, user) + } catch (Throwable t) { + String cause = getCause(t) + return getFailureXML(ErrorCode.INVALID_PARAMETERS, + "Error updating '" + name + "': " + + cause) + } } } else { // Create @@ -667,12 +678,12 @@ class ResourceController extends ApiController { try { resource = prototype.createInstance(parent, name, user, config, agent, ips) - } catch (IllegalArgumentException e) { + } catch (Throwable t) { + String cause = getCause(t) + log.warn("Error creating resource", t) return getFailureXML(ErrorCode.INVALID_PARAMETERS, - e.getMessage()) - } catch (Exception e) { - log.warn("Error creating resource", e) - return getFailureXML(ErrorCode.OBJECT_EXISTS); + "Error creating '" + name + "':" + + cause); } } else if (prototype.isServerPrototype()) { @@ -694,12 +705,12 @@ class ResourceController extends ApiController { resource = prototype.createInstance(parent, name, user, config) - } catch (IllegalArgumentException e) { + } catch (Throwable t) { + String cause = getCause(t) + log.warn("Error creating resource", t) return getFailureXML(ErrorCode.INVALID_PARAMETERS, - e.getMessage()) - } catch (Exception e) { - log.warn("Error creating resource", e) - return getFailureXML(ErrorCode.OBJECT_EXISTS); + "Error creating '" + name + "':" + + cause); } } else if (prototype.isServicePrototype()) { @@ -711,14 +722,14 @@ class ResourceController extends ApiController { config.put(PROP_AIIDENIFIER, aiid.'@value') } resource = prototype.createInstance(parent, name, - user, config) - } catch (IllegalArgumentException e) { + user, config) + } catch (Throwable t) { + String cause = getCause(t) + log.warn("Error creating resource", t) return getFailureXML(ErrorCode.INVALID_PARAMETERS, - e.getMessage()) - } catch (Exception e) { - log.warn("Error creating resource", e) - return getFailureXML(ErrorCode.OBJECT_EXISTS); - } + "Error creating '" + name + "':" + + cause); + } } else { return getFailureXML(ErrorCode.INVALID_PARAMETERS, "Invalid prototype=" + prototype.name) diff --git a/src/org/hyperic/hq/hqapi1/test/ResourceCreatePlatform_test.java b/src/org/hyperic/hq/hqapi1/test/ResourceCreatePlatform_test.java index 79acee71..fcd2cc74 100644 --- a/src/org/hyperic/hq/hqapi1/test/ResourceCreatePlatform_test.java +++ b/src/org/hyperic/hq/hqapi1/test/ResourceCreatePlatform_test.java @@ -85,4 +85,49 @@ public void testCreatePlatform() throws Exception { StatusResponse deleteResponse = api.deleteResource(resp.getResource().getId()); hqAssertSuccess(deleteResponse); } + + public void testCreatePlatformLongDescription() throws Exception { + + Agent a = getRunningAgent(); + ResourceApi api = getApi().getResourceApi(); + + ResourcePrototypeResponse protoResponse = + api.getResourcePrototype("Network Device"); + + Random r = new Random(); + final String name = "Test Network Device" + r.nextInt(); + final String fqdn = "apitest.hyperic.com"; + + Ip ip = new Ip(); + ip.setAddress("10.0.0.1"); + + List ips = new ArrayList(); + ips.add(ip); + + String longDescription = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + Map config = new HashMap(); + config.put("interface.index", "ifDescr"); + config.put("snmpIp", "10.0.0.1"); + config.put("snmpPort", "161"); + config.put("snmpCommunity", "public"); + config.put("snmpVersion", "v2c"); + config.put("description", longDescription); + + ResourceResponse resp = + api.createPlatform(a, protoResponse.getResourcePrototype(), + name, fqdn, ips, config); + hqAssertFailureInvalidParameters(resp); + } } diff --git a/src/org/hyperic/hq/hqapi1/test/ResourceCreateServer_test.java b/src/org/hyperic/hq/hqapi1/test/ResourceCreateServer_test.java index 9db4db09..a8e6b271 100644 --- a/src/org/hyperic/hq/hqapi1/test/ResourceCreateServer_test.java +++ b/src/org/hyperic/hq/hqapi1/test/ResourceCreateServer_test.java @@ -132,4 +132,43 @@ public void testCreateServerInvalidPrototype() throws Exception { parent, name, config); hqAssertFailureInvalidParameters(resp); } + + public void testCreateServerLongDescription() throws Exception { + + ResourceApi api = getApi().getResourceApi(); + + ResourcePrototypeResponse protoResponse = + api.getResourcePrototype("Apache httpd"); + hqAssertSuccess(protoResponse); + + Resource parent = getLocalPlatformResource(false, false); + + String longDescription = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + Random r = new Random(); + final String name = "Test Apache Server" + r.nextInt(); + + Map config = new HashMap(); + config.put("hostname", "localhost"); + config.put("port", "80"); + config.put("path", "/server-status"); + config.put("sotimeout", "10"); + config.put("description", longDescription); + + ResourceResponse resp = + api.createServer(protoResponse.getResourcePrototype(), + parent, name, config); + hqAssertFailureInvalidParameters(resp); + } } diff --git a/src/org/hyperic/hq/hqapi1/test/ResourceCreateService_test.java b/src/org/hyperic/hq/hqapi1/test/ResourceCreateService_test.java index 7b3e5d5f..0e4158bb 100644 --- a/src/org/hyperic/hq/hqapi1/test/ResourceCreateService_test.java +++ b/src/org/hyperic/hq/hqapi1/test/ResourceCreateService_test.java @@ -123,4 +123,46 @@ public void testServiceCreateInvalidPrototype() throws Exception { ResourceResponse resp = api.createService(pt, agent, name, params); hqAssertFailureInvalidParameters(resp); } + + public void testServiceCreateInvalidDescription() throws Exception { + + Agent a = getRunningAgent(); + + ResourceApi api = getApi().getResourceApi(); + + // Find HTTP resource type + ResourcePrototypeResponse protoResponse = api.getResourcePrototype("HTTP"); + hqAssertSuccess(protoResponse); + ResourcePrototype pt = protoResponse.getResourcePrototype(); + + // Find local platform + ResourcesResponse resourcesResponse = api.getResources(a, false, false); + hqAssertSuccess(resourcesResponse); + assertTrue("Did not find a single platform for " + a.getAddress() + ":" + + a.getPort(), resourcesResponse.getResource().size() == 1); + Resource platform = resourcesResponse.getResource().get(0); + + String longDescription = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + // Configure service + Map params = new HashMap(); + params.put("hostname", "www.hyperic.com"); + params.put("port", "80"); + params.put("sotimeout", "10"); + params.put("path", "/"); + params.put("method", "GET"); + params.put("description", longDescription); + + Random r = new Random(); + String name = "My HTTP Check " + r.nextInt(); + + ResourceResponse resp = api.createService(pt, platform, name, params); + hqAssertFailureInvalidParameters(resp); + } } diff --git a/src/org/hyperic/hq/hqapi1/test/ResourceUpdate_test.java b/src/org/hyperic/hq/hqapi1/test/ResourceUpdate_test.java index 2614690f..f043e716 100644 --- a/src/org/hyperic/hq/hqapi1/test/ResourceUpdate_test.java +++ b/src/org/hyperic/hq/hqapi1/test/ResourceUpdate_test.java @@ -184,5 +184,35 @@ public void testUpdateInvalidChildResource() throws Exception { StatusResponse updateResponse = api.updateResource(platform); hqAssertFailureObjectNotFound(updateResponse); } + + public void testUpdateInvalidDescription() throws Exception { + + ResourceApi api = getApi().getResourceApi(); + Resource createdResource = createTestHTTPService(); + + String longDescription = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + + createdResource.setDescription(longDescription); + + StatusResponse updateResponse = api.updateResource(createdResource); + hqAssertFailureInvalidParameters(updateResponse); + + // Cannot delete resources soon after modifying them.. + try { + Thread.sleep(2000); + } catch (InterruptedException e) { + // Ignore + } + + // Cleanup + StatusResponse deleteResponse = api.deleteResource(createdResource.getId()); + hqAssertSuccess(deleteResponse); + } }