Mercurial > hg > release > thermostat-2.0
changeset 1574:967ff19b0416
Re-register categories on WebStorageEndPoint reload/redeploy.
Reviewed-by: vanaltj
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-November/011717.html
line wrap: on
line diff
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/RetryableDescriptorParsingException.java Fri Nov 28 12:04:09 2014 +0100 @@ -0,0 +1,52 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.storage.core; + +/** + * Exception thrown if parsing of a prepared statement descriptor + * failed but retrying to prepare the same descriptor may succeed. + * + */ +@SuppressWarnings("serial") +public class RetryableDescriptorParsingException extends + DescriptorParsingException { + + public RetryableDescriptorParsingException(String msg) { + super(msg); + } + +}
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Fri Nov 28 12:04:09 2014 +0100 @@ -96,6 +96,7 @@ import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.shared.config.SSLConfiguration; import com.redhat.thermostat.storage.core.AuthToken; +import com.redhat.thermostat.storage.core.Categories; import com.redhat.thermostat.storage.core.Category; import com.redhat.thermostat.storage.core.Connection; import com.redhat.thermostat.storage.core.Cursor; @@ -103,6 +104,7 @@ import com.redhat.thermostat.storage.core.IllegalDescriptorException; import com.redhat.thermostat.storage.core.IllegalPatchException; import com.redhat.thermostat.storage.core.PreparedStatement; +import com.redhat.thermostat.storage.core.RetryableDescriptorParsingException; import com.redhat.thermostat.storage.core.RetryableStatementExecutionException; import com.redhat.thermostat.storage.core.SecureStorage; import com.redhat.thermostat.storage.core.StatementDescriptor; @@ -110,6 +112,7 @@ import com.redhat.thermostat.storage.core.Storage; import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; +import com.redhat.thermostat.storage.model.AggregateResult; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.web.common.PreparedStatementResponseCode; import com.redhat.thermostat.web.common.SharedStateId; @@ -348,7 +351,7 @@ private String endpoint; - private Map<Category<?>, Integer> categoryIds; + private Map<Category<?>, SharedStateId> categoryIds; private Gson gson; // The shared http client we use for execution (uses the context below) private HttpClient httpClient; @@ -548,7 +551,7 @@ try (CloseableHttpEntity entity = post(endpoint + "/register-category", formparams)) { Reader reader = getContentAsReader(entity); - Integer id = gson.fromJson(reader, Integer.class); + SharedStateId id = gson.fromJson(reader, SharedStateId.class); categoryIds.put(category, id); } } @@ -766,7 +769,7 @@ // Nothing to do here. } - int getCategoryId(Category<?> category) { + SharedStateId getCategoryId(Category<?> category) { return categoryIds.get(category); } @@ -808,11 +811,11 @@ <T extends Pojo> WebPreparedStatementHolder sendPrepareStmtRequest(StatementDescriptor<T> desc) throws DescriptorParsingException { String strDesc = desc.getDescriptor(); - int categoryId = getCategoryId(desc.getCategory()); + SharedStateId categoryId = getCategoryId(desc.getCategory()); NameValuePair nameParam = new BasicNameValuePair("query-descriptor", strDesc); NameValuePair categoryParam = new BasicNameValuePair("category-id", - gson.toJson(categoryId, Integer.class)); + gson.toJson(categoryId, SharedStateId.class)); List<NameValuePair> formparams = Arrays .asList(nameParam, categoryParam); try (CloseableHttpEntity entity = post(endpoint + "/prepare-statement", @@ -821,26 +824,58 @@ WebPreparedStatementResponse result = gson.fromJson(reader, WebPreparedStatementResponse.class); int numParams = result.getNumFreeVariables(); SharedStateId statementId = result.getStatementId(); - if (statementId.getId() == WebPreparedStatementResponse.ILLEGAL_STATEMENT) { - // we've got a descriptor the endpoint doesn't know about or - // refuses to accept for security reasons. - String msg = "Unknown query descriptor which endpoint of " + WebStorage.class.getName() + " refused to accept!"; - throw new IllegalDescriptorException(msg, desc.getDescriptor()); - } else if (statementId.getId() == WebPreparedStatementResponse.DESCRIPTOR_PARSE_FAILED) { - String msg = "Statement descriptor failed to parse. " + - "Please check server logs for details!"; - throw new DescriptorParsingException(msg); - } else { - // We need this ugly trick in order for WebQueryResponse - // deserialization to work properly. I.e. GSON needs this type - // info hint. - Class<T> dataClass = desc.getCategory().getDataClass(); - Type typeToken = new WebQueryResponse<T>().getRuntimeParametrizedType(dataClass); - return new WebPreparedStatementHolder(typeToken, numParams, statementId); + int stmtId = statementId.getId(); + switch (stmtId) { + case WebPreparedStatementResponse.ILLEGAL_STATEMENT: { + // we've got a descriptor the endpoint doesn't know about or + // refuses to accept for security reasons. + String msg = "Unknown query descriptor which endpoint of " + WebStorage.class.getName() + " refused to accept!"; + throw new IllegalDescriptorException(msg, desc.getDescriptor()); + } + case WebPreparedStatementResponse.DESCRIPTOR_PARSE_FAILED: { + String msg = "Statement descriptor failed to parse. " + + "Please check server logs for details!"; + throw new DescriptorParsingException(msg); + } + case WebPreparedStatementResponse.CATEGORY_OUT_OF_SYNC: { + // We tried to prepare a statement and the server's + // representation of category IDs changed. Thus, be sure to + // clear the category state and get their new IDs. + String msg = "Preparing statement failed. Server changed category state. Clearing category ID for statement: " + + desc.getDescriptor(); + logger.log(Level.WARNING, msg); + sendCategoryReRegistrationRequest(desc.getCategory()); + throw new RetryableDescriptorParsingException(msg); + } + default: { + // Common case where stmtId is the actual ID of the statement + // and not an error code. + assert(stmtId >= 0); // negative values are error codes + // We need this ugly trick in order for WebQueryResponse + // deserialization to work properly. I.e. GSON needs this type + // info hint. + Class<T> dataClass = desc.getCategory().getDataClass(); + Type typeToken = new WebQueryResponse<T>().getRuntimeParametrizedType(dataClass); + return new WebPreparedStatementHolder(typeToken, numParams, statementId); + } } } } + private synchronized <T extends Pojo> void sendCategoryReRegistrationRequest(Category<T> category) { + // There are two possible cases. Category is an aggregate category or + // it is not. For aggregate categories we need to re-register the + // original first and then the aggregate category. + Class<T> dataClass = category.getDataClass(); + if (AggregateResult.class.isAssignableFrom(dataClass)) { + Category<?> nonAggregateCategory = Categories.getByName(category.getName()); + categoryIds.remove(nonAggregateCategory); + registerCategory(nonAggregateCategory); + } + categoryIds.remove(category); + registerCategory(category); + } + // Container used for parameter caching in order to avoid unneccessary // network overhead. static class WebPreparedStatementHolder {
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Fri Nov 28 12:04:09 2014 +0100 @@ -134,6 +134,7 @@ private Map<String,String> headers; private String method; private String requestURI; + private UUID serverNonce; private static Category<TestObj> category; private static Key<String> key1; @@ -156,7 +157,7 @@ @Before public void setUp() throws Exception { - + serverNonce = UUID.randomUUID(); port = FreePortFinder.findFreePort(new TryPort() { @Override public void tryPort(int port) throws Exception { @@ -251,8 +252,8 @@ private void registerCategory() { // Return 42 for categoryId. - Gson gson = new Gson(); - responseBody = gson.toJson(42); + Gson gson = new GsonBuilder().registerTypeAdapterFactory(new SharedStateIdTypeAdapterFactory()).create(); + responseBody = gson.toJson(new SharedStateId(42, serverNonce)); storage.registerCategory(category); }
--- a/web/common/src/main/java/com/redhat/thermostat/web/common/SharedStateId.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/common/src/main/java/com/redhat/thermostat/web/common/SharedStateId.java Fri Nov 28 12:04:09 2014 +0100 @@ -77,4 +77,9 @@ public int hashCode() { return Objects.hash(id, serverToken); } + + @Override + public String toString() { + return serverToken + ":" + id; + } }
--- a/web/common/src/main/java/com/redhat/thermostat/web/common/WebPreparedStatementResponse.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/common/src/main/java/com/redhat/thermostat/web/common/WebPreparedStatementResponse.java Fri Nov 28 12:04:09 2014 +0100 @@ -51,6 +51,13 @@ */ public static final int DESCRIPTOR_PARSE_FAILED = -2; + /** + * Response code indicating that the server token + * of the client and the token the server is using + * did not match. + */ + public static final int CATEGORY_OUT_OF_SYNC = -3; + public WebPreparedStatementResponse() { // Should always be set using the setter before it // is retrieved. Since 0 is a bad default for this,
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/CategoryManager.java Fri Nov 28 12:04:09 2014 +0100 @@ -0,0 +1,118 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.web.server; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; + +import com.redhat.thermostat.storage.core.Category; +import com.redhat.thermostat.storage.model.Pojo; +import com.redhat.thermostat.web.common.SharedStateId; + +class CategoryManager { + + private final Map<CategoryIdentifier, SharedStateId> categoryIds; + private final Map<SharedStateId, Category<?>> categories; + private int categoryIdCounter = 0; + + CategoryManager() { + categories = new HashMap<>(); + categoryIds = new HashMap<>(); + } + + // Testing only + CategoryManager(int initialValue) { + this(); + categoryIdCounter = initialValue; + } + + synchronized <T extends Pojo> SharedStateId putCategory(UUID serverNonce, Category<T> category, CategoryIdentifier catId) { + if (categoryIds.containsKey(Objects.requireNonNull(catId))) { + return categoryIds.get(catId); + } else { + // add new category + Objects.requireNonNull(category); + Objects.requireNonNull(serverNonce); + SharedStateId newId = new SharedStateId(categoryIdCounter, serverNonce); + categoryIdCounter++; + // This really should not happen, but if it does fail early. + if (categoryIdCounter == Integer.MAX_VALUE) { + throw new IllegalStateException("Too many categories!"); + } + categoryIds.put(catId, newId); + categories.put(newId, category); + return newId; + } + } + + synchronized SharedStateId getCategoryId(CategoryIdentifier key) { + return categoryIds.get(Objects.requireNonNull(key)); + } + + @SuppressWarnings("unchecked") + synchronized <T extends Pojo> Category<T> getCategory(SharedStateId id) { + return (Category<T>)categories.get(Objects.requireNonNull(id)); + } + + static class CategoryIdentifier { + + private final String categoryName; + private final String dataClassName; + + CategoryIdentifier(String categoryName, String dataClassName) { + this.categoryName = categoryName; + this.dataClassName = dataClassName; + } + + @Override + public boolean equals(Object other) { + if (other == null || CategoryIdentifier.class != other.getClass()) { + return false; + } + CategoryIdentifier o = (CategoryIdentifier)other; + return Objects.equals(categoryName, o.categoryName) && + Objects.equals(dataClassName, o.dataClassName); + } + + @Override + public int hashCode() { + return Objects.hash(categoryName, dataClassName); + } + } +}
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Fri Nov 28 12:04:09 2014 +0100 @@ -41,15 +41,12 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.PrintWriter; -import java.io.Writer; import java.lang.reflect.Array; import java.net.URLDecoder; import java.security.Principal; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.UUID; @@ -113,6 +110,7 @@ import com.redhat.thermostat.web.common.typeadapters.WebPreparedStatementResponseTypeAdapterFactory; import com.redhat.thermostat.web.common.typeadapters.WebPreparedStatementTypeAdapterFactory; import com.redhat.thermostat.web.common.typeadapters.WebQueryResponseTypeAdapterFactory; +import com.redhat.thermostat.web.server.CategoryManager.CategoryIdentifier; import com.redhat.thermostat.web.server.auth.FilterResult; import com.redhat.thermostat.web.server.auth.PrincipalCallback; import com.redhat.thermostat.web.server.auth.PrincipalCallbackFactory; @@ -135,9 +133,9 @@ private static final String TOKEN_MANAGER_KEY = "token-manager"; private static final String USER_PRINCIPAL_CALLBACK_KEY = "user-principal-callback"; private static final String CURSOR_MANAGER_KEY = "cursor-manager"; - private static final String PREPARED_STMT_MANAGER_KEY = "prepared-stmt-manager"; + static final String CATEGORY_MANAGER_KEY = "category-manager"; + static final String PREPARED_STMT_MANAGER_KEY = "prepared-stmt-manager"; private static final int UNKNOWN_CURSOR_ID = -0xdeadbeef; - private static final String CATEGORY_KEY_FORMAT = "%s|%s"; // our strings can contain non-ASCII characters. Use UTF-8 // see also PR 1344 @@ -154,12 +152,6 @@ public static final String STORAGE_PASSWORD = "storage.password"; public static final String STORAGE_CLASS = "storage.class"; - private int currentCategoryId; - - private Map<String, Integer> categoryIds; - private Map<Integer, Category<?>> categories; - // A unique server token, which gets reset on every Servlet.init() call. - // I.e. every reload/redeployment. private UUID serverToken; // read-only set of all known statement descriptors we trust and allow @@ -185,8 +177,6 @@ .registerTypeAdapterFactory(new WebPreparedStatementTypeAdapterFactory()) .registerTypeAdapterFactory(new PreparedParametersTypeAdapterFactory()) .create(); - categoryIds = new HashMap<>(); - categories = new HashMap<>(); TokenManager tokenManager = new TokenManager(); String timeoutParam = getInitParameter(TOKEN_MANAGER_TIMEOUT_PARAM); if (timeoutParam != null) { @@ -210,6 +200,7 @@ PrincipalCallback callback = Objects.requireNonNull(cbFactory.getCallback()); servletContext.setAttribute(USER_PRINCIPAL_CALLBACK_KEY, callback); synchronized(servletContext) { + servletContext.setAttribute(CATEGORY_MANAGER_KEY, new CategoryManager()); servletContext.setAttribute(PREPARED_STMT_MANAGER_KEY, new PreparedStatementManager()); } serverToken = UUID.randomUUID(); @@ -349,63 +340,82 @@ } String queryDescrParam = req.getParameter("query-descriptor"); String categoryIdParam = req.getParameter("category-id"); - Integer catId = gson.fromJson(categoryIdParam, Integer.class); - Category<T> cat = (Category<T>)getCategoryFromId(catId); - WebPreparedStatementResponse response = new WebPreparedStatementResponse(); - if (cat == null) { - // bad category? we refuse to accept this - logger.log(Level.WARNING, "Attepted to prepare a statement with an illegal category id"); - SharedStateId id = new SharedStateId(WebPreparedStatementResponse.ILLEGAL_STATEMENT, serverToken); - response.setStatementId(id); - writeResponse(resp, response, WebPreparedStatementResponse.class); - return; - } - StatementDescriptor<T> desc = new StatementDescriptor<>(cat, queryDescrParam); - // Check if descriptor is trusted (i.e. known) - if (!knownStatementDescriptors.contains(desc.getDescriptor())) { - String msg = "Attempted to prepare a statement descriptor which we " + - "don't trust! Descriptor was: ->" + desc.getDescriptor() + "<-"; - logger.log(Level.WARNING, msg); - SharedStateId id = new SharedStateId(WebPreparedStatementResponse.ILLEGAL_STATEMENT, serverToken); - response.setStatementId(id); - writeResponse(resp, response, WebPreparedStatementResponse.class); - return; - } - - PreparedStatementManager prepStmtManager = getPreparedStmtManager(); - // see if we've prepared this query already - PreparedStatementHolder<T> holder = prepStmtManager.getStatementHolder(desc); - if (holder != null) { - ParsedStatement<T> parsed = holder.getStmt().getParsedStatement(); - int freeVars = parsed.getNumParams(); - response.setNumFreeVariables(freeVars); - SharedStateId id = new SharedStateId(holder.getId().getId(), serverToken); + SharedStateId catId = gson.fromJson(URLDecoder.decode(categoryIdParam, "UTF-8"), SharedStateId.class); + // Check if server token of the given category id is still valid. If it is + // different it means that the server has been reloaded/redeployed + // while the client remained up. Of course, it does not rule out a + // malicious client which sends a bad token on purpose. In either case + // it should be OK to solely send back a distinct error code indicating + // this situation. + if (!serverToken.equals(catId.getServerToken())) { + logger.log(Level.INFO, "Server token: '" + serverToken + + "' and client token '" + catId.getServerToken() + + "' out of sync."); + WebPreparedStatementResponse response = new WebPreparedStatementResponse(); + SharedStateId id = new SharedStateId(WebPreparedStatementResponse.CATEGORY_OUT_OF_SYNC, serverToken); response.setStatementId(id); writeResponse(resp, response, WebPreparedStatementResponse.class); return; } else { - // Prepare the target statement and track it via - // PreparedStatementManager - PreparedStatement<T> targetPreparedStatement; - try { - targetPreparedStatement = (PreparedStatement<T>) storage - .prepareStatement(desc); - } catch (DescriptorParsingException e) { - logger.log(Level.WARNING, "Descriptor parse error!", e); - SharedStateId id = new SharedStateId(WebPreparedStatementResponse.DESCRIPTOR_PARSE_FAILED, serverToken); + CategoryManager catManager = getCategoryManager(); + Category<T> cat = (Category<T>)catManager.getCategory(catId); + WebPreparedStatementResponse response = new WebPreparedStatementResponse(); + if (cat == null) { + // bad category? we refuse to accept this + logger.log(Level.WARNING, "Attepted to prepare a statement with an illegal category id: '" + + catId + "'. server token was: '" + serverToken + "'"); + SharedStateId id = new SharedStateId(WebPreparedStatementResponse.ILLEGAL_STATEMENT, serverToken); response.setStatementId(id); - writeResponse(resp, response, - WebPreparedStatementResponse.class); + writeResponse(resp, response, WebPreparedStatementResponse.class); + return; + } + StatementDescriptor<T> desc = new StatementDescriptor<>(cat, queryDescrParam); + // Check if descriptor is trusted (i.e. known) + if (!knownStatementDescriptors.contains(desc.getDescriptor())) { + String msg = "Attempted to prepare a statement descriptor which we " + + "don't trust! Descriptor was: ->" + desc.getDescriptor() + "<-"; + logger.log(Level.WARNING, msg); + SharedStateId id = new SharedStateId(WebPreparedStatementResponse.ILLEGAL_STATEMENT, serverToken); + response.setStatementId(id); + writeResponse(resp, response, WebPreparedStatementResponse.class); return; } - SharedStateId stmtId = prepStmtManager.createAndPutHolder( - serverToken, targetPreparedStatement, cat.getDataClass(), - desc); - ParsedStatement<?> parsed = targetPreparedStatement - .getParsedStatement(); - response.setNumFreeVariables(parsed.getNumParams()); - response.setStatementId(stmtId); - writeResponse(resp, response, WebPreparedStatementResponse.class); + + PreparedStatementManager prepStmtManager = getPreparedStmtManager(); + // see if we've prepared this query already + PreparedStatementHolder<T> holder = prepStmtManager.getStatementHolder(desc); + if (holder != null) { + ParsedStatement<T> parsed = holder.getStmt().getParsedStatement(); + int freeVars = parsed.getNumParams(); + response.setNumFreeVariables(freeVars); + SharedStateId id = new SharedStateId(holder.getId().getId(), serverToken); + response.setStatementId(id); + writeResponse(resp, response, WebPreparedStatementResponse.class); + return; + } else { + // Prepare the target statement and track it via + // PreparedStatementManager + PreparedStatement<T> targetPreparedStatement; + try { + targetPreparedStatement = (PreparedStatement<T>) storage + .prepareStatement(desc); + } catch (DescriptorParsingException e) { + logger.log(Level.WARNING, "Descriptor parse error!", e); + SharedStateId id = new SharedStateId(WebPreparedStatementResponse.DESCRIPTOR_PARSE_FAILED, serverToken); + response.setStatementId(id); + writeResponse(resp, response, + WebPreparedStatementResponse.class); + return; + } + SharedStateId stmtId = prepStmtManager.createAndPutHolder( + serverToken, targetPreparedStatement, cat.getDataClass(), + desc); + ParsedStatement<?> parsed = targetPreparedStatement + .getParsedStatement(); + response.setNumFreeVariables(parsed.getNumParams()); + response.setStatementId(stmtId); + writeResponse(resp, response, WebPreparedStatementResponse.class); + } } } @@ -518,26 +528,22 @@ } } - @SuppressWarnings("unchecked") // need to adapt categories + @SuppressWarnings("unchecked") // we adapt categories in an unchecked fashion @WebStoragePathHandler( path = "register-category" ) private synchronized void registerCategory(HttpServletRequest req, HttpServletResponse resp) throws IOException { if (! isAuthorized(req, resp, Roles.REGISTER_CATEGORY)) { return; } - String categoryName = req.getParameter("name"); String dataClassName = req.getParameter("data-class"); - // We need to index into the category map using name + data class since - // we have a different category for aggregate queries. For them the - // category name will be the same, but the data class will be different. - String categoryKey = String.format(CATEGORY_KEY_FORMAT, categoryName, dataClassName); String categoryParam = req.getParameter("category"); - int id; - if (categoryIds.containsKey(categoryKey)) { - id = categoryIds.get(categoryKey); - } else { + CategoryIdentifier catIdentifier = new CategoryIdentifier(categoryName, dataClassName); + CategoryManager catManager = getCategoryManager(); + SharedStateId id = catManager.getCategoryId(catIdentifier); + if (id == null) { Class<?> dataClass = getDataClassFromName(dataClassName); Category<?> category = null; + boolean isAggregateCat = false; if ((AggregateResult.class.isAssignableFrom(dataClass))) { // Aggregate category case Category<?> original = Categories.getByName(categoryName); @@ -552,7 +558,7 @@ @SuppressWarnings({ "rawtypes" }) CategoryAdapter adapter = new CategoryAdapter(original); category = adapter.getAdapted(dataClass); - logger.log(Level.FINEST, "(id: " + currentCategoryId + ") not registering aggregate category " + category ); + isAggregateCat = true; } else { // Regular, non-aggregate category. Those categories we actually // need to register with backing storage. @@ -569,20 +575,19 @@ // deserialized Category in the Categories class. category = gson.fromJson(categoryParam, Category.class); storage.registerCategory(category); - logger.log(Level.FINEST, "(id: " + currentCategoryId + ") registered non-aggreate category: " + category); } - id = currentCategoryId; - categoryIds.put(categoryKey, id); - categories.put(id, category); - currentCategoryId++; + id = catManager.putCategory(serverToken, category, catIdentifier); + if (isAggregateCat) { + logger.log(Level.FINEST, "(id: " + id.getId() + ") did not register aggregate category " + category ); + } else { + logger.log(Level.FINEST, "(id: " + id.getId() + ") registered non-aggreate category: " + category); + } } resp.setStatus(HttpServletResponse.SC_OK); resp.setContentType(RESPONSE_JSON_CONTENT_TYPE); - Writer writer = resp.getWriter(); - gson.toJson(id, writer); - writer.flush(); + writeResponse(resp, id, SharedStateId.class); } - + private Class<?> getDataClassFromName(String dataClassName) { try { Class<?> clazz = Class.forName(dataClassName); @@ -680,15 +685,25 @@ } writeQueryResponse(resp, response, resultsList, targetStmtHolder); } + + // package-private for testing + @SuppressWarnings("unchecked") + <T> T getServletContextAttribute(final String attributeName) { + ServletContext servletContext = getServletContext(); + T attributeVal = null; + synchronized(servletContext) { + attributeVal = (T)servletContext.getAttribute(attributeName); + } + // If this throws a NPE this is certainly a bug. + return Objects.requireNonNull(attributeVal); + } + + private CategoryManager getCategoryManager() { + return getServletContextAttribute(CATEGORY_MANAGER_KEY); + } private PreparedStatementManager getPreparedStmtManager() { - ServletContext servletContext = getServletContext(); - PreparedStatementManager prepStmtManager = null; - synchronized(servletContext) { - prepStmtManager = (PreparedStatementManager)servletContext.getAttribute(PREPARED_STMT_MANAGER_KEY); - } - // If this throws a NPE this is certainly a bug. - return Objects.requireNonNull(prepStmtManager); + return getServletContextAttribute(PREPARED_STMT_MANAGER_KEY); } private <T extends Pojo> void writeQueryResponse(HttpServletResponse resp, WebQueryResponse<T> response, List<T> resultsList, PreparedStatementHolder<T> targetStmtHolder) throws IOException { @@ -898,11 +913,6 @@ return patchedQuery; } - private Category<?> getCategoryFromId(int categoryId) { - Category<?> category = categories.get(categoryId); - return category; - } - private void writeResponse(HttpServletResponse resp, Object responseObj, Class<?> typeOfResponseObj) throws IOException { String json = null;
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/CategoryManagerTest.java Fri Nov 28 12:04:09 2014 +0100 @@ -0,0 +1,220 @@ +/* + * Copyright 2012-2014 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * <http://www.gnu.org/licenses/>. + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.web.server; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; + +import java.util.UUID; + +import org.junit.Test; + +import com.redhat.thermostat.storage.core.Category; +import com.redhat.thermostat.web.common.SharedStateId; +import com.redhat.thermostat.web.server.CategoryManager.CategoryIdentifier; + +public class CategoryManagerTest { + + // CategoryIdentifier tests + + @Test + public void testCategoryIdentifierEquals() { + String catName = "foo-collection"; + String dataClassName = "com.redhat.thermostat.storage.core.model.Foo"; + CategoryIdentifier id = new CategoryIdentifier(catName, dataClassName); + assertTrue(id.equals(id)); + assertFalse(id.equals(null)); + String dataClass2 = "com.redhat.thermostat.storage.core.model.Bar"; + CategoryIdentifier id2 = new CategoryIdentifier(catName, dataClass2); + assertFalse("different data classes", id.equals(id2)); + String otherCatName = "bar-collection"; + CategoryIdentifier id3 = new CategoryIdentifier(otherCatName, dataClassName); + assertFalse("different category names", id.equals(id3)); + CategoryIdentifier id4 = new CategoryIdentifier(catName, dataClassName); + assertNotSame(id, id4); + assertTrue("category name and data class name match up", id.equals(id4)); + } + + @Test + public void testCategoryIdentifierHashCode() { + String catName = "foo-collection"; + String dataClassName = "com.redhat.thermostat.storage.core.model.Foo"; + CategoryIdentifier id = new CategoryIdentifier(catName, dataClassName); + assertTrue(id.hashCode() == id.hashCode()); + String dataClass2 = "com.redhat.thermostat.storage.core.model.Bar"; + CategoryIdentifier id2 = new CategoryIdentifier(catName, dataClass2); + assertTrue("different data classes", id.hashCode() != id2.hashCode()); + String otherCatName = "bar-collection"; + CategoryIdentifier id3 = new CategoryIdentifier(otherCatName, dataClassName); + assertTrue("different category names", id.hashCode() != id3.hashCode()); + CategoryIdentifier id4 = new CategoryIdentifier(catName, dataClassName); + assertNotSame(id, id4); + assertTrue("category name and data class name match up", id.hashCode() == id4.hashCode()); + } + + // CategoryManager tests + + /** + * Verifies that only categories can be added to a manager correctly. It + * should add a category which has been added already (same cat-identifier) + * only once. + */ + @Test + public void testPutCategory() { + UUID serverToken = UUID.randomUUID(); + String catName = "foo-collection"; + String dataClassName = "com.redhat.thermostat.storage.core.model.Foo"; + CategoryIdentifier identifier = new CategoryIdentifier(catName, dataClassName); + Category<?> mockCategory = mock(Category.class); + CategoryManager manager = new CategoryManager(); + SharedStateId id = manager.putCategory(serverToken, mockCategory, identifier); + assertNotNull(id); + assertEquals(serverToken, id.getServerToken()); + assertEquals(0, id.getId()); + CategoryIdentifier identifier2 = new CategoryIdentifier("bar", dataClassName); + Category<?> otherCat = mock(Category.class); + SharedStateId otherId = manager.putCategory(serverToken, otherCat, identifier2); + assertNotNull(otherId); + assertNotSame(id, otherId); + assertFalse(otherId.equals(id)); + + // This should give back the same id since it's the very same identifier + SharedStateId thirdId = manager.putCategory(serverToken, mockCategory, identifier); + assertSame(id, thirdId); + } + + /** + * Verifies that NPEs get thrown when essential params are null. + */ + @Test + public void testPutCategoryNullParams() { + UUID serverToken = UUID.randomUUID(); + String catName = "foo-collection"; + String dataClassName = "com.redhat.thermostat.storage.core.model.Foo"; + CategoryIdentifier identifier = new CategoryIdentifier(catName, dataClassName); + Category<?> mockCategory = mock(Category.class); + CategoryManager manager = new CategoryManager(); + try { + manager.putCategory(null, mockCategory, identifier); + fail("Expected NPE due to null server token"); + } catch (NullPointerException e) { + // pass + } + try { + manager.putCategory(serverToken, null, identifier); + fail("Expected NPE due to null category"); + } catch (NullPointerException e) { + // pass + } + try { + manager.putCategory(serverToken, mockCategory, null); + fail("Expected NPE due to null category identifier"); + } catch (NullPointerException e) { + // pass + } + } + + @Test + public void testGetCategoryIdNullParams() { + CategoryManager manager = new CategoryManager(); + try { + manager.getCategoryId(null); + fail("Expected NPE due to null identifier"); + } catch (NullPointerException e) { + // pass + } + } + + @Test + public void canGetCategoryId() { + UUID serverNonce = UUID.randomUUID(); + CategoryManager manager = new CategoryManager(); + CategoryIdentifier catIdentifier = mock(CategoryIdentifier.class); + Category<?> mockCat = mock(Category.class); + SharedStateId id = manager.putCategory(serverNonce, mockCat, catIdentifier); + SharedStateId getId = manager.getCategoryId(catIdentifier); + assertEquals(id, getId); + assertSame(id, getId); + + // unknown cat identifier should return null + CategoryIdentifier otherIdentifier = mock(CategoryIdentifier.class); + assertNull(manager.getCategoryId(otherIdentifier)); + } + + @Test + public void testGetCategoryNullParams() { + CategoryManager manager = new CategoryManager(); + try { + manager.getCategory(null); + fail("Expected NPE due to null shared state id"); + } catch (NullPointerException e) { + // pass + } + } + + /** + * Check CategoryManager.getCategory() and CategoryManager.getCategoryId() + * in tandem. + */ + @Test + public void canGetCategoryCategoryId() { + UUID serverNonce = UUID.randomUUID(); + CategoryManager manager = new CategoryManager(); + assertNull(manager.getCategory(mock(SharedStateId.class))); + assertNull(manager.getCategoryId(mock(CategoryIdentifier.class))); + + // add an element + Category<?> category = mock(Category.class); + CategoryIdentifier catId = new CategoryIdentifier("foo", "bar"); + SharedStateId id = manager.putCategory(serverNonce, category, catId); + + // getting it should never be null now + Category<?> cat = manager.getCategory(id); + assertNotNull(cat); + SharedStateId otherId = manager.getCategoryId(catId); + assertNotNull(otherId); + + assertSame(id, otherId); + } +}
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java Fri Nov 28 12:04:09 2014 +0100 @@ -38,12 +38,16 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; +import static org.mockito.Matchers.eq; import java.io.File; +import java.io.IOException; import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; @@ -52,10 +56,12 @@ import java.util.Map; import javax.servlet.ServletConfig; +import javax.servlet.ServletContext; import javax.servlet.ServletException; import org.junit.After; import org.junit.Test; +import org.mockito.ArgumentCaptor; import com.redhat.thermostat.web.server.auth.WebStoragePathHandler; @@ -67,9 +73,10 @@ */ public class WebStorageEndPointUnitTest { + private static final String TH_HOME_PROP_NAME = "THERMOSTAT_HOME"; @After public void tearDown() { - System.clearProperty("THERMOSTAT_HOME"); + System.clearProperty(TH_HOME_PROP_NAME); } /* @@ -84,9 +91,9 @@ */ @Test public void testCheckThermostatHome() { - System.setProperty("THERMOSTAT_HOME", "/root"); + System.setProperty(TH_HOME_PROP_NAME, "/root"); WebStorageEndPoint endpoint = new WebStorageEndPoint(); - assertTrue("THERMOSTAT_HOME clearly set, do we create paths where we shouldn't?", + assertTrue(TH_HOME_PROP_NAME + " clearly set, do we create paths where we shouldn't?", endpoint.isThermostatHomeSet()); } @@ -154,18 +161,18 @@ fail("Thermostat home was not set in config, should not get here!"); } catch (RuntimeException e) { // pass - assertTrue(e.getMessage().contains("THERMOSTAT_HOME")); + assertTrue(e.getMessage().contains(TH_HOME_PROP_NAME)); } catch (ServletException e) { fail(e.getMessage()); } // set config with non-existing dir - when(config.getInitParameter("THERMOSTAT_HOME")).thenReturn("not-existing"); + when(config.getInitParameter(TH_HOME_PROP_NAME)).thenReturn("not-existing"); try { endpoint.init(config); fail("Thermostat home was set in config but file does not exist, should have died!"); } catch (RuntimeException e) { // pass - assertTrue(e.getMessage().contains("THERMOSTAT_HOME")); + assertTrue(e.getMessage().contains(TH_HOME_PROP_NAME)); } catch (ServletException e) { fail(e.getMessage()); } @@ -173,25 +180,15 @@ @Test public void initThrowsRuntimeExceptionIfSSLPropertiesNotReadable() throws Exception { - Path testThermostatHome = null; - File etcDir = null; + ThCreatorResult result = null; try { - testThermostatHome = Files.createTempDirectory( - "foo-thermostat-home-", new FileAttribute[] {}); - File thFile = testThermostatHome.toFile(); - etcDir = new File(thFile, "etc"); - etcDir.mkdir(); - assertTrue(etcDir.exists()); - assertTrue(etcDir.canWrite()); - File sslProperties = new File(etcDir, "ssl.properties"); - sslProperties.createNewFile(); - assertTrue(sslProperties.canRead()); + result = creatWorkingThermostatHome(); // explicitly remove read perms from etc directory - etcDir.setExecutable(false); - assertFalse(sslProperties.canRead()); + result.etcDir.setExecutable(false); + assertFalse(result.sslProperties.canRead()); WebStorageEndPoint endpoint = new WebStorageEndPoint(); - System.setProperty("THERMOSTAT_HOME", thFile.getAbsolutePath()); + System.setProperty(TH_HOME_PROP_NAME, result.thermostatHome.toFile().getAbsolutePath()); try { endpoint.init(mock(ServletConfig.class)); fail("should have failed to initialize! can't read ssl.properties"); @@ -199,9 +196,9 @@ assertTrue(e.getMessage().contains("ssl.properties")); } } finally { - etcDir.setExecutable(true); - if (testThermostatHome != null) { - WebstorageEndpointTestUtils.deleteDirectoryRecursive(testThermostatHome); + result.etcDir.setExecutable(true); + if (result.thermostatHome != null) { + WebstorageEndpointTestUtils.deleteDirectoryRecursive(result.thermostatHome); } } } @@ -214,7 +211,7 @@ "bar-thermostat-home-", new FileAttribute[] {}); File thFile = testThermostatHome.toFile(); WebStorageEndPoint endpoint = new WebStorageEndPoint(); - System.setProperty("THERMOSTAT_HOME", thFile.getAbsolutePath()); + System.setProperty(TH_HOME_PROP_NAME, thFile.getAbsolutePath()); try { endpoint.init(mock(ServletConfig.class)); fail("should have failed to initialize, ssl.properties not existing!"); @@ -228,4 +225,55 @@ } } + /** + * Verifies that Servlet.init() sets servlet context attributes correctly. + * @throws ServletException + * @throws IOException + */ + @Test + public void testSetServletAttribute() throws ServletException, IOException { + final ServletContext mockContext = mock(ServletContext.class); + when(mockContext.getServerInfo()).thenReturn("jetty/9.1.0.v20131115"); + @SuppressWarnings("serial") + WebStorageEndPoint endpoint = new WebStorageEndPoint() { + @Override + public ServletContext getServletContext() { + return mockContext; + } + }; + ServletConfig config = mock(ServletConfig.class); + ThCreatorResult result = creatWorkingThermostatHome(); + System.setProperty(TH_HOME_PROP_NAME, result.thermostatHome.toFile().getAbsolutePath()); + endpoint.init(config); + ArgumentCaptor<CategoryManager> managerCaptor = ArgumentCaptor.forClass(CategoryManager.class); + verify(mockContext).setAttribute(eq("category-manager"), managerCaptor.capture()); + assertNotNull(managerCaptor.getValue()); + } + + private ThCreatorResult creatWorkingThermostatHome() throws IOException { + Path testThermostatHome = Files.createTempDirectory( + "foo-thermostat-home-", new FileAttribute[] {}); + File thFile = testThermostatHome.toFile(); + File etcDir = new File(thFile, "etc"); + etcDir.mkdir(); + assertTrue(etcDir.exists()); + assertTrue(etcDir.canWrite()); + File sslProperties = new File(etcDir, "ssl.properties"); + sslProperties.createNewFile(); + assertTrue(sslProperties.canRead()); + return new ThCreatorResult(testThermostatHome, etcDir, sslProperties); + } + + private static class ThCreatorResult { + private final Path thermostatHome; + private final File etcDir; + private final File sslProperties; + + ThCreatorResult(Path thermostatHome, File etcFile, File sslProperties) { + this.thermostatHome = thermostatHome; + this.etcDir = etcFile; + this.sslProperties = sslProperties; + } + } + }
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Tue Nov 11 16:02:59 2014 +0100 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Fri Nov 28 12:04:09 2014 +0100 @@ -57,6 +57,7 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Reader; +import java.io.UnsupportedEncodingException; import java.lang.reflect.Type; import java.net.HttpURLConnection; import java.net.MalformedURLException; @@ -187,7 +188,6 @@ private Server server; private int port; private BackingStorage mockStorage; - private Integer categoryId; private static Key<String> key1; private static Key<Integer> key2; @@ -325,7 +325,7 @@ // This makes register category work for the "test" category. // Undone via @After setupTrustedCategory(categoryName); - registerCategory(testuser, password); + SharedStateId catId = registerCategoryAndGetId(category, testuser, password); String endpoint = getEndpoint(); URL url = new URL(endpoint + "/prepare-statement"); @@ -344,7 +344,7 @@ .registerTypeAdapterFactory(new WebPreparedStatementTypeAdapterFactory()) .create(); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + categoryId; + String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + getURLEncodedCategoryIdJson(gson, catId); out.write(body + "\n"); out.flush(); @@ -368,10 +368,11 @@ public void authorizedPrepareQueryWithTrustedDescriptorSuccessfulGetMore() throws Exception { // Get the trusted descriptor String strDescriptor = setupPreparedQueryWithTrustedDescriptor(); + SharedStateId catId = registerCategoryAndGetId(category, "ignored1", "ignored2"); // Prepare the query boolean moreBatches = true; - TrustedPreparedQueryTestResult prepareQueryResult = prepareQuery(strDescriptor, moreBatches); + TrustedPreparedQueryTestResult prepareQueryResult = prepareQuery(catId, strDescriptor, moreBatches); Type typeToken = new TypeToken<WebQueryResponse<TestClass>>(){}.getType(); @@ -439,10 +440,11 @@ public void authorizedPrepareQueryWithTrustedDescriptorGetMoreFail() throws Exception { // Get the trusted descriptor String strDescriptor = setupPreparedQueryWithTrustedDescriptor(); + SharedStateId catId = registerCategoryAndGetId(category, "ignored1", "ignored2"); // Prepare the query boolean moreBatches = false; - TrustedPreparedQueryTestResult prepareQueryResult = prepareQuery(strDescriptor, moreBatches); + TrustedPreparedQueryTestResult prepareQueryResult = prepareQuery(catId, strDescriptor, moreBatches); Type typeToken = new TypeToken<WebQueryResponse<TestClass>>(){}.getType(); @@ -547,7 +549,7 @@ } @SuppressWarnings("unchecked") - private TrustedPreparedQueryTestResult prepareQuery(String strDescriptor, boolean moreBatches) throws Exception { + private TrustedPreparedQueryTestResult prepareQuery(SharedStateId catId, String strDescriptor, boolean moreBatches) throws Exception { TestClass expected1 = new TestClass(); expected1.setKey1("fluff1"); expected1.setKey2(42); @@ -598,7 +600,7 @@ .registerTypeAdapterFactory(new PreparedParametersTypeAdapterFactory()) .create(); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + categoryId; + String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + getURLEncodedCategoryIdJson(gson, catId); out.write(body + "\n"); out.flush(); @@ -637,7 +639,6 @@ // This makes register category work for the "test" category. // Undone via @After setupTrustedCategory(categoryName); - registerCategory("ignored1", "ignored2"); return strDescriptor; } @@ -681,7 +682,7 @@ // This makes register category work for the "test" category. // Undone via @After setupTrustedCategory(categoryName); - registerCategory("ignored1", "ignored2"); + SharedStateId catId = registerCategoryAndGetId(category, "ignored1", "ignored2"); TestClass expected1 = new TestClass(); expected1.setKey1("fluff1"); @@ -729,7 +730,7 @@ .registerTypeAdapterFactory(new PreparedParametersTypeAdapterFactory()) .create(); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + categoryId; + String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + getURLEncodedCategoryIdJson(gson, catId); out.write(body + "\n"); out.flush(); @@ -814,7 +815,7 @@ // prepare-statement does this under the hood AggregateQuery2<AggregateCount> mockMongoQuery = mock(AggregateQuery2.class); Category<AggregateCount> adapted = new CategoryAdapter(category).getAdapted(AggregateCount.class); - registerCategory(adapted, "no-matter", "no-matter"); + SharedStateId catId = registerCategoryAndGetId(adapted, "no-matter", "no-matter"); when(mockStorage.createAggregateQuery(eq(AggregateFunction.COUNT), eq(adapted))).thenReturn(mockMongoQuery); Cursor<AggregateCount> cursor = mock(Cursor.class); @@ -852,7 +853,7 @@ .registerTypeAdapterFactory(new PreparedParametersTypeAdapterFactory()) .create(); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + categoryId; + String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + getURLEncodedCategoryIdJson(gson, catId); out.write(body + "\n"); out.flush(); @@ -946,7 +947,7 @@ // This makes register category work for the "test" category. // Undone via @After setupTrustedCategory(categoryName); - registerCategory("ignored1", "ignored2"); + SharedStateId catId = registerCategoryAndGetId(category, "ignored1", "ignored2"); // prepare-statement does this under the hood Add<TestClass> mockMongoAdd = mock(Add.class); @@ -983,7 +984,7 @@ .registerTypeAdapterFactory(new PreparedParametersTypeAdapterFactory()) .create(); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + categoryId; + String body = "query-descriptor=" + URLEncoder.encode(strDescriptor, "UTF-8") + "&category-id=" + getURLEncodedCategoryIdJson(gson, catId); out.write(body + "\n"); out.flush(); @@ -1021,6 +1022,10 @@ } } + private String getURLEncodedCategoryIdJson(Gson gson, SharedStateId catId) throws UnsupportedEncodingException { + return URLEncoder.encode(gson.toJson(catId), "UTF-8"); + } + @Test public void cannotRegisterCategoryWithoutRegistrationOnInit() throws Exception { // need this in order to pass basic permissions. @@ -1100,7 +1105,7 @@ // This makes register category work for the "test" category. // Undone via @After setupTrustedCategory(categoryName); - registerCategory(testuser, password); + registerCategoryAndGetId(category, testuser, password); } String endpoint = getEndpoint(); @@ -1134,16 +1139,16 @@ // First the originating category has to be registered, then the adapted // one. - Integer realId = registerCategoryAndGetId(wantedCategory, "no-matter", "no-matter"); - Integer aggregateId = registerCategoryAndGetId(aggregate, "no-matter", "no-matter"); + SharedStateId realId = registerCategoryAndGetId(wantedCategory, "no-matter", "no-matter"); + SharedStateId aggregateId = registerCategoryAndGetId(aggregate, "no-matter", "no-matter"); - assertTrue("Aggregate categories need their own ID", aggregateId != realId); + assertTrue("Aggregate categories need their own ID", realId.getId() != aggregateId.getId()); verify(mockStorage).registerCategory(eq(wantedCategory)); verifyNoMoreInteractions(mockStorage); } - private Integer registerCategoryAndGetId(Category<?> cat, String username, String password) throws Exception { + private SharedStateId registerCategoryAndGetId(Category<?> cat, String username, String password) throws Exception { String endpoint = getEndpoint(); URL url = new URL(endpoint + "/register-category"); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); @@ -1154,12 +1159,12 @@ conn.setDoInput(true); conn.setRequestProperty("Content-Type", "application/x-www-form-urlencoded"); OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream()); - Gson gson = new Gson(); + Gson gson = new GsonBuilder().registerTypeAdapterFactory(new SharedStateIdTypeAdapterFactory()).create(); out.write("name=" + cat.getName() + "&data-class=" + cat.getDataClass().getName() + "&category=" + gson.toJson(cat)); out.flush(); assertEquals(200, conn.getResponseCode()); Reader reader = new InputStreamReader(conn.getInputStream()); - Integer id = gson.fromJson(reader, Integer.class); + SharedStateId id = gson.fromJson(reader, SharedStateId.class); return id; } @@ -1446,41 +1451,6 @@ doUnauthorizedTest("purge", failMsg, insufficientRoles, false); } - private void registerCategory(String username, String password) { - registerCategory(category, username, password); - } - - private void registerCategory(Category<?> category, String username, String password) { - try { - String endpoint = getEndpoint(); - URL url = new URL(endpoint + "/register-category"); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); - String enc = "UTF-8"; - conn.setRequestProperty("Content-Encoding", enc); - conn.setDoOutput(true); - conn.setDoInput(true); - conn.setRequestMethod("POST"); - sendAuthentication(conn, username, password); - OutputStream out = conn.getOutputStream(); - Gson gson = new Gson(); - OutputStreamWriter writer = new OutputStreamWriter(out); - writer.write("name="); - writer.write(URLEncoder.encode(category.getName(), enc)); - writer.write("&category="); - writer.write(URLEncoder.encode(gson.toJson(category), enc)); - writer.write("&data-class="); - writer.write(URLEncoder.encode(category.getDataClass().getName(), enc)); - writer.flush(); - - InputStream in = conn.getInputStream(); - Reader reader = new InputStreamReader(in); - Integer id = gson.fromJson(reader, Integer.class); - categoryId = id; - } catch (IOException ex) { - throw new RuntimeException(ex); - } - } - private String getEndpoint() { return "http://localhost:" + port + "/storage"; }