From 0c5a215ab2f76e2b38f18f626a8186b6a45bc395 Mon Sep 17 00:00:00 2001 From: Jarek Jarcec Cecho Date: Sun, 2 Nov 2014 18:37:41 -0800 Subject: [PATCH] SQOOP-1653: Link and connector handler minor fixes (Veena Basavaraj via Jarek Jarcec Cecho) --- .../org/apache/sqoop/json/ConnectorsBean.java | 1 - .../java/org/apache/sqoop/json/LinkBean.java | 15 +++---- .../java/org/apache/sqoop/json/LinksBean.java | 6 +-- .../java/org/apache/sqoop/model/MLink.java | 6 +-- .../org/apache/sqoop/json/TestLinkBean.java | 8 ++-- .../apache/sqoop/repository/Repository.java | 10 ++--- .../handler/ConnectorRequestHandler.java | 1 + .../sqoop/handler/LinkRequestHandler.java | 39 ++++++++++--------- .../apache/sqoop/server/RequestHandler.java | 3 ++ .../apache/sqoop/server/v1/LinkServlet.java | 20 +++++----- .../sqoop/shell/UpdateLinkFunction.java | 4 +- 11 files changed, 57 insertions(+), 56 deletions(-) diff --git a/common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java b/common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java index 4cd3698f..b04594eb 100644 --- a/common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java +++ b/common/src/main/java/org/apache/sqoop/json/ConnectorsBean.java @@ -46,7 +46,6 @@ public ConnectorsBean() { @SuppressWarnings("unchecked") @Override public JSONObject extract(boolean skipSensitive) { - JSONArray connectorArray = extractConnectors(skipSensitive); JSONObject connectors = new JSONObject(); connectors.put(CONNECTORS, connectorArray); diff --git a/common/src/main/java/org/apache/sqoop/json/LinkBean.java b/common/src/main/java/org/apache/sqoop/json/LinkBean.java index bc59352e..ae3a4ff4 100644 --- a/common/src/main/java/org/apache/sqoop/json/LinkBean.java +++ b/common/src/main/java/org/apache/sqoop/json/LinkBean.java @@ -89,15 +89,15 @@ public ResourceBundle getConnectorConfigBundle(Long id) { @SuppressWarnings("unchecked") @Override public JSONObject extract(boolean skipSensitive) { - JSONArray linkArray = new JSONArray(); - extractLinks(skipSensitive, linkArray); - JSONObject all = new JSONObject(); - all.put(LINK, linkArray); - return all; + JSONArray linkArray = extractLinks(skipSensitive); + JSONObject link = new JSONObject(); + link.put(LINK, linkArray); + return link; } - @SuppressWarnings("unchecked") - protected void extractLinks(boolean skipSensitive, JSONArray linkArray) { + protected JSONArray extractLinks(boolean skipSensitive) { + JSONArray linkArray = new JSONArray(); + for(MLink link : links) { JSONObject linkJsonObject = new JSONObject(); linkJsonObject.put(ID, link.getPersistenceId()); @@ -112,6 +112,7 @@ protected void extractLinks(boolean skipSensitive, JSONArray linkArray) { extractConfigList(link.getConnectorLinkConfig().getConfigs(), link.getConnectorLinkConfig().getType(), skipSensitive)); linkArray.add(linkJsonObject); } + return linkArray; } @Override diff --git a/common/src/main/java/org/apache/sqoop/json/LinksBean.java b/common/src/main/java/org/apache/sqoop/json/LinksBean.java index 5858a184..58cacb9c 100644 --- a/common/src/main/java/org/apache/sqoop/json/LinksBean.java +++ b/common/src/main/java/org/apache/sqoop/json/LinksBean.java @@ -43,8 +43,7 @@ public LinksBean() { @SuppressWarnings("unchecked") @Override public JSONObject extract(boolean skipSensitive) { - JSONArray linkArray = new JSONArray(); - super.extractLinks(skipSensitive, linkArray); + JSONArray linkArray = extractLinks(skipSensitive); JSONObject links = new JSONObject(); links.put(LINKS, linkArray); return links; @@ -52,8 +51,7 @@ public JSONObject extract(boolean skipSensitive) { @Override public void restore(JSONObject jsonObject) { - JSONArray array = (JSONArray) jsonObject.get(LINKS); super.restoreLinks(array); } -} +} \ No newline at end of file diff --git a/common/src/main/java/org/apache/sqoop/model/MLink.java b/common/src/main/java/org/apache/sqoop/model/MLink.java index 7a9f5380..8e318613 100644 --- a/common/src/main/java/org/apache/sqoop/model/MLink.java +++ b/common/src/main/java/org/apache/sqoop/model/MLink.java @@ -73,10 +73,6 @@ public long getConnectorId() { return connectorId; } - public void setConnectorId(long connectorId) { - this.connectorId = connectorId; - } - public MLinkConfig getConnectorLinkConfig() { return connectorLinkConfig; } @@ -108,4 +104,4 @@ public boolean equals(Object object) { && (mLink.getPersistenceId() == this.getPersistenceId()) && (mLink.connectorLinkConfig.equals(this.connectorLinkConfig)); } -} +} \ No newline at end of file diff --git a/common/src/test/java/org/apache/sqoop/json/TestLinkBean.java b/common/src/test/java/org/apache/sqoop/json/TestLinkBean.java index 811cbf02..7c3c921d 100644 --- a/common/src/test/java/org/apache/sqoop/json/TestLinkBean.java +++ b/common/src/test/java/org/apache/sqoop/json/TestLinkBean.java @@ -59,9 +59,9 @@ public void testSerialization() { JSONObject json = linkBean.extract(false); // Check for sensitivity - JSONArray all = (JSONArray)json.get(LinkBean.LINK); - JSONObject allItem = (JSONObject)all.get(0); - JSONArray connectors = (JSONArray)allItem.get(LinkBean.LINK_CONFIG); + JSONArray linkArray = (JSONArray)json.get(LinkBean.LINK); + JSONObject linkItem = (JSONObject)linkArray.get(0); + JSONArray connectors = (JSONArray)linkItem.get(LinkBean.LINK_CONFIG); JSONObject connector = (JSONObject)connectors.get(0); JSONArray inputs = (JSONArray)connector.get(ConfigInputConstants.CONFIG_INPUTS); for (Object input1 : inputs) { @@ -137,4 +137,4 @@ public void testSensitivityFilter() { password = (JSONObject)inputs.get(2); assertFalse(password.containsKey(ConfigInputConstants.CONFIG_INPUT_VALUE)); } -} +} \ No newline at end of file diff --git a/core/src/main/java/org/apache/sqoop/repository/Repository.java b/core/src/main/java/org/apache/sqoop/repository/Repository.java index 79742b9d..be053832 100644 --- a/core/src/main/java/org/apache/sqoop/repository/Repository.java +++ b/core/src/main/java/org/apache/sqoop/repository/Repository.java @@ -403,8 +403,8 @@ private void deleteJobs(List jobs, RepositoryTransaction tx) { */ public final void upgradeConnector(MConnector oldConnector, MConnector newConnector) { LOG.info("Upgrading connector: " + oldConnector.getUniqueName()); - long connectorID = oldConnector.getPersistenceId(); - newConnector.setPersistenceId(connectorID); + long connectorId = oldConnector.getPersistenceId(); + newConnector.setPersistenceId(connectorId); RepositoryTransaction tx = null; try { @@ -415,9 +415,9 @@ public final void upgradeConnector(MConnector oldConnector, MConnector newConnec // 1. Get an upgrader for the connector ConnectorConfigurableUpgrader upgrader = connector.getConfigurableUpgrader(); // 2. Get all links associated with the connector. - List existingLinksByConnector = findLinksForConnector(connectorID); + List existingLinksByConnector = findLinksForConnector(connectorId); // 3. Get all jobs associated with the connector. - List existingJobsByConnector = findJobsForConnector(connectorID); + List existingJobsByConnector = findJobsForConnector(connectorId); // -- BEGIN TXN -- tx = getTransaction(); tx.begin(); @@ -602,4 +602,4 @@ private void logInvalidModelObject(String objectType, MPersistableEntity entity, LOG.error("\t" + entry.getKey() + ": " + StringUtils.join(entry.getValue(), ",")); } } -} \ No newline at end of file +} diff --git a/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java b/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java index 5694ea58..7903713b 100644 --- a/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java +++ b/server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java @@ -71,6 +71,7 @@ public JsonBean handleEvent(RequestContext ctx) { } else { // NOTE: we now support using unique name as well as the connector id + // NOTE: connectorId is a fallback for older sqoop clients if any, since we want to primarily use unique conenctorNames boolean cIdNameIdentfierUsed = true; Long cId = ConnectorManager.getInstance().getConnectorId(cIdentifier); if (cId == null) { diff --git a/server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java b/server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java index 35a96352..9a81832d 100644 --- a/server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java +++ b/server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java @@ -34,6 +34,7 @@ import org.apache.sqoop.model.ConfigUtils; import org.apache.sqoop.model.MLink; import org.apache.sqoop.model.MLinkConfig; +import org.apache.sqoop.model.MPersistableEntity; import org.apache.sqoop.repository.Repository; import org.apache.sqoop.repository.RepositoryManager; import org.apache.sqoop.server.RequestContext; @@ -51,7 +52,6 @@ public class LinkRequestHandler implements RequestHandler { static final String DISABLE = "disable"; static final String LINKS_PATH = "links"; static final String LINK_PATH = "link"; - static final String CONNECTOR_NAME_QUERY_PARAM = "cname"; public LinkRequestHandler() { LOG.info("LinkRequestHandler initialized"); @@ -124,10 +124,10 @@ private JsonBean createUpdateLink(RequestContext ctx, boolean create) { "Expected one link while parsing JSON request but got " + links.size()); } - MLink link = links.get(0); + MLink postedLink = links.get(0); MLinkConfig linkConfig = ConnectorManager.getInstance() - .getConnectorConfigurable(link.getConnectorId()).getLinkConfig(); - if (!linkConfig.equals(link.getConnectorLinkConfig())) { + .getConnectorConfigurable(postedLink.getConnectorId()).getLinkConfig(); + if (!linkConfig.equals(postedLink.getConnectorLinkConfig())) { throw new SqoopException(ServerError.SERVER_0003, "Detected incorrect link config structure"); } // if update get the link id from the request URI @@ -135,15 +135,17 @@ private JsonBean createUpdateLink(RequestContext ctx, boolean create) { String linkIdentifier = ctx.getLastURLElement(); // support linkName or linkId for the api long linkId = getLinkIdFromIdentifier(linkIdentifier, repository); - MLink existingLink = repository.findLink(linkId); - link.setConnectorId(existingLink.getConnectorId()); + if (postedLink.getPersistenceId() == MPersistableEntity.PERSISTANCE_ID_DEFAULT) { + MLink existingLink = repository.findLink(linkId); + postedLink.setPersistenceId(existingLink.getPersistenceId()); + } } // Associated connector for this link SqoopConnector connector = ConnectorManager.getInstance().getSqoopConnector( - link.getConnectorId()); + postedLink.getConnectorId()); // Validate user supplied config data - ConfigValidationResult connectorLinkConfigValidation = ConfigUtils.validateConfigs(link + ConfigValidationResult connectorLinkConfigValidation = ConfigUtils.validateConfigs(postedLink .getConnectorLinkConfig().getConfigs(), connector.getLinkConfigurationClass()); // Return back link validation result bean ValidationResultBean linkValidationBean = new ValidationResultBean( @@ -154,17 +156,17 @@ private JsonBean createUpdateLink(RequestContext ctx, boolean create) { if (create) { AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(), ctx.getRequest().getRemoteAddr(), "create", "link", - String.valueOf(link.getPersistenceId())); - link.setCreationUser(username); - link.setLastUpdateUser(username); - repository.createLink(link); - linkValidationBean.setId(link.getPersistenceId()); + String.valueOf(postedLink.getPersistenceId())); + postedLink.setCreationUser(username); + postedLink.setLastUpdateUser(username); + repository.createLink(postedLink); + linkValidationBean.setId(postedLink.getPersistenceId()); } else { AuditLoggerManager.getInstance().logAuditEvent(ctx.getUserName(), ctx.getRequest().getRemoteAddr(), "update", "link", - String.valueOf(link.getPersistenceId())); - link.setLastUpdateUser(username); - repository.updateLink(link); + String.valueOf(postedLink.getPersistenceId())); + postedLink.setLastUpdateUser(username); + repository.updateLink(postedLink); } } @@ -214,6 +216,7 @@ private JsonBean getLinks(RequestContext ctx) { private long getLinkIdFromIdentifier(String identifier, Repository repository) { // support linkName or linkId for the api + // NOTE: linkId is a fallback for older sqoop clients if any, since we want to primarily use unique linkNames long linkId; if (repository.findLink(identifier) != null) { linkId = repository.findLink(identifier).getPersistenceId(); @@ -221,7 +224,7 @@ private long getLinkIdFromIdentifier(String identifier, Repository repository) { try { linkId = Long.valueOf(identifier); } catch (NumberFormatException ex) { - // this means name nor Id existed + // this means name nor Id existed and we want to throw a user friendly message than a number format exception throw new SqoopException(ServerError.SERVER_0005, "Invalid link: " + identifier + " requested"); } @@ -260,4 +263,4 @@ private JsonBean enableLink(RequestContext ctx, boolean enabled) { repository.enableLink(linkId, enabled); return JsonBean.EMPTY_BEAN; } -} +} \ No newline at end of file diff --git a/server/src/main/java/org/apache/sqoop/server/RequestHandler.java b/server/src/main/java/org/apache/sqoop/server/RequestHandler.java index 508edd21..bd62f31e 100644 --- a/server/src/main/java/org/apache/sqoop/server/RequestHandler.java +++ b/server/src/main/java/org/apache/sqoop/server/RequestHandler.java @@ -22,5 +22,8 @@ public interface RequestHandler { + static final String CONNECTOR_NAME_QUERY_PARAM = "cname"; + static final String JOB_NAME_QUERY_PARAM = "jname"; + JsonBean handleEvent(RequestContext ctx); } diff --git a/server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java b/server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java index 127903a2..0e2e1763 100644 --- a/server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java +++ b/server/src/main/java/org/apache/sqoop/server/v1/LinkServlet.java @@ -27,30 +27,30 @@ * Provides operations for link resource * * GET /v1/link/{lid} - * Return details about one particular link with id :lid + * Return details about one particular link with id lid * GET /v1/link/{lname} - * Return details about one particular link with name :lname + * Return details about one particular link with name lname * * POST /v1/link/ with {connector-id} and {link-config-id} in the post data * Create link for connector with id connector-id * PUT /v1/link/ with {connector-id} and {link-config-id} in the post data * Edit/Update link for connector with id connector-id - + * * PUT /v1/link/{lid} - * Edit/Update details about one particular link with id :lid + * Edit/Update details about one particular link with id lid * PUT /v1/link/{lname} - * Edit/Update details about one particular link with name :lname + * Edit/Update details about one particular link with name lname * * DELETE /v1/link/{lid} - * Delete/Remove one particular link with id :lid + * Delete/Remove one particular link with id lid * DELETE /v1/link/{lname} - * Delete/Remove one particular link with name :lname + * Delete/Remove one particular link with name lname * * PUT /v1/link/{lname}/enable - * Enable link with id :lname + * Enable link with name lname * * PUT /v1/link/{lname}/disable - * Disable link with id :lname + * Disable link with name lname * */ @SuppressWarnings("serial") @@ -81,4 +81,4 @@ protected JsonBean handlePutRequest(RequestContext ctx) throws Exception { protected JsonBean handleDeleteRequest(RequestContext ctx) throws Exception { return linkRequestHandler.handleEvent(ctx); } -} +} \ No newline at end of file diff --git a/shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java b/shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java index 176833aa..60f95006 100644 --- a/shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java +++ b/shell/src/main/java/org/apache/sqoop/shell/UpdateLinkFunction.java @@ -60,7 +60,7 @@ private Status updateLink(Long linkId, List args, boolean isInteractive) ConsoleReader reader = new ConsoleReader(); - // TODO: why do we need 2 http calls for update? + // TODO(SQOOP-1634): using link config id, this call can be avoided MLink link = client.getLink(linkId); ResourceBundle connectorLinkConfigBundle = client.getConnectorConfigBundle(link.getConnectorId()); @@ -104,4 +104,4 @@ private Status updateLink(Long linkId, List args, boolean isInteractive) return status; } -} +} \ No newline at end of file