Repository: aurora Updated Branches: refs/heads/master 87eb8914b -> 9e646ae53
Remove endpoint.thrift, ServiceInstance is never serialized to thrift This enables removal of some unnecessary complexity in the build (commons no longer needs thrift) and the unused Codec abstraction (we always encode in JSON). Reviewed at https://reviews.apache.org/r/63418/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/9e646ae5 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/9e646ae5 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/9e646ae5 Branch: refs/heads/master Commit: 9e646ae53bca5bc4ace6686328f8790842d2c375 Parents: 87eb891 Author: Bill Farner <[email protected]> Authored: Mon Oct 30 21:58:13 2017 -0700 Committer: Bill Farner <[email protected]> Committed: Mon Oct 30 21:58:13 2017 -0700 ---------------------------------------------------------------------- build.gradle | 11 -- .../java/org/apache/aurora/common/io/Codec.java | 53 ------- .../org/apache/aurora/common/thrift/Util.java | 156 ------------------- .../aurora/common/zookeeper/Encoding.java | 87 ----------- .../aurora/common/zookeeper/JsonCodec.java | 139 ----------------- .../aurora/common/net/http/handlers/thrift.st | 64 -------- .../apache/aurora/common/thrift/endpoint.thrift | 90 ----------- .../aurora/common/zookeeper/EncodingTest.java | 44 ------ .../aurora/common/zookeeper/JsonCodecTest.java | 151 ------------------ .../scheduler/app/ServiceGroupMonitor.java | 2 +- .../CuratorServiceDiscoveryModule.java | 15 +- .../discovery/CuratorServiceGroupMonitor.java | 18 +-- .../discovery/CuratorSingletonService.java | 31 +--- .../aurora/scheduler/discovery/Encoding.java | 71 +++++++++ .../scheduler/discovery/ServiceInstance.java | 125 +++++++++++++++ .../aurora/scheduler/http/LeaderRedirect.java | 15 +- .../aurora/scheduler/http/StructDump.java | 9 +- .../discovery/BaseCuratorDiscoveryTest.java | 22 +-- .../CuratorServiceGroupMonitorTest.java | 7 +- .../discovery/CuratorSingletonServiceTest.java | 2 +- .../scheduler/discovery/EncodingTest.java | 98 ++++++++++++ .../scheduler/http/AbstractJettyTest.java | 9 +- .../scheduler/http/LeaderRedirectTest.java | 20 +-- 23 files changed, 337 insertions(+), 902 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/build.gradle ---------------------------------------------------------------------- diff --git a/build.gradle b/build.gradle index 1c1d381..26051ac 100644 --- a/build.gradle +++ b/build.gradle @@ -180,11 +180,6 @@ project(':ui') { tasks.processResources.dependsOn(':ui:webpack') project(':commons') { - apply plugin: org.apache.aurora.build.ThriftPlugin - thrift { - version = thriftRev - } - apply plugin: 'license' license { header rootProject.file('config/checkstyle/apache.header') @@ -192,17 +187,13 @@ project(':commons') { } dependencies { - compile project(':commons-args') - compile "com.google.code.findbugs:jsr305:${jsrRev}" compile "com.google.code.gson:gson:${gsonRev}" compile "com.google.guava:guava:${guavaRev}" - compile "com.google.inject.extensions:guice-multibindings:${guiceRev}" compile "com.google.inject:guice:${guiceRev}" compile "com.sun.jersey:jersey-core:${jerseyRev}" compile "commons-lang:commons-lang:${commonsLangRev}" compile "javax.servlet:javax.servlet-api:${servletRev}" - compile "joda-time:joda-time:2.9.1" compile "org.antlr:stringtemplate:${stringTemplateRev}" compile "org.apache.zookeeper:zookeeper:${zookeeperRev}" compile "org.easymock:easymock:3.4" @@ -213,8 +204,6 @@ project(':commons') { compile "junit:junit:${junitRev}" testCompile "junit:junit:${junitRev}" - testCompile "org.powermock:powermock-module-junit4:1.6.4" - testCompile "org.powermock:powermock-api-easymock:1.6.4" } } http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/java/org/apache/aurora/common/io/Codec.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/io/Codec.java b/commons/src/main/java/org/apache/aurora/common/io/Codec.java deleted file mode 100644 index 94d1e36..0000000 --- a/commons/src/main/java/org/apache/aurora/common/io/Codec.java +++ /dev/null @@ -1,53 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.io; - -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; - -/** - * A Codec represents a reversible encoding for a given type. Codecs are able to both - * {@link #deserialize(java.io.InputStream) read} items from streams and - * {@link #serialize(Object, java.io.OutputStream) write} items to streams. - * - * <p> TODO(John Sirois): consider whether this interface should optionally support null items to be - * read and written. - * - * @param <T> The type of object the Codec can handle. - * - * @author John Sirois - */ -public interface Codec<T> { - - /** - * Writes a representation of {@code item} to the {@code sink} that can be read back by - * {@link #deserialize(java.io.InputStream)}. - * - * @param item the item to serialize - * @param sink the stream to write the item out to - * @throws IOException if there is a problem serializing the item - */ - void serialize(T item, OutputStream sink) throws IOException; - - /** - * Reads an item from the {@code source} stream that was written by - * {@link #serialize(Object, java.io.OutputStream)}. - * - * @param source the stream to read an item from - * @return the deserialized item - * @throws IOException if there is a problem reading an item - */ - T deserialize(InputStream source) throws IOException; -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/java/org/apache/aurora/common/thrift/Util.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/thrift/Util.java b/commons/src/main/java/org/apache/aurora/common/thrift/Util.java deleted file mode 100644 index 4ef7f49..0000000 --- a/commons/src/main/java/org/apache/aurora/common/thrift/Util.java +++ /dev/null @@ -1,156 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.thrift; - -import java.util.List; -import java.util.Map; -import java.util.Set; - -import com.google.common.base.Joiner; -import com.google.common.base.Strings; -import com.google.common.collect.Lists; - -import org.apache.thrift.TBase; -import org.apache.thrift.TFieldIdEnum; -import org.apache.thrift.meta_data.FieldMetaData; - -/** - * Utility functions for thrift. - * - * @author William Farner - */ -public class Util { - /** - * Pretty-prints a thrift object contents. - * - * @param t The thrift object to print. - * @return The pretty-printed version of the thrift object. - */ - public static String prettyPrint(TBase t) { - return t == null ? "null" : printTbase(t, 0); - } - - /** - * Prints an object contained in a thrift message. - * - * @param o The object to print. - * @param depth The print nesting level. - * @return The pretty-printed version of the thrift field. - */ - private static String printValue(Object o, int depth) { - if (o == null) { - return "null"; - } else if (TBase.class.isAssignableFrom(o.getClass())) { - return "\n" + printTbase((TBase) o, depth + 1); - } else if (Map.class.isAssignableFrom(o.getClass())) { - return printMap((Map) o, depth + 1); - } else if (List.class.isAssignableFrom(o.getClass())) { - return printList((List) o, depth + 1); - } else if (Set.class.isAssignableFrom(o.getClass())) { - return printSet((Set) o, depth + 1); - } else if (String.class == o.getClass()) { - return '"' + o.toString() + '"'; - } else { - return o.toString(); - } - } - - /** - * Prints a TBase. - * - * @param t The object to print. - * @param depth The print nesting level. - * @return The pretty-printed version of the TBase. - */ - private static String printTbase(TBase t, int depth) { - List<String> fields = Lists.newArrayList(); - for (Map.Entry<? extends TFieldIdEnum, FieldMetaData> entry : - FieldMetaData.getStructMetaDataMap(t.getClass()).entrySet()) { - @SuppressWarnings("unchecked") - boolean fieldSet = t.isSet(entry.getKey()); - String strValue; - if (fieldSet) { - @SuppressWarnings("unchecked") - Object value = t.getFieldValue(entry.getKey()); - strValue = printValue(value, depth); - } else { - strValue = "not set"; - } - fields.add(tabs(depth) + entry.getValue().fieldName + ": " + strValue); - } - - return Joiner.on("\n").join(fields); - } - - /** - * Prints a map in a style that is consistent with TBase pretty printing. - * - * @param map The map to print - * @param depth The print nesting level. - * @return The pretty-printed version of the map. - */ - private static String printMap(Map<?, ?> map, int depth) { - List<String> entries = Lists.newArrayList(); - for (Map.Entry entry : map.entrySet()) { - entries.add(tabs(depth) + printValue(entry.getKey(), depth) - + " = " + printValue(entry.getValue(), depth)); - } - - return entries.isEmpty() ? "{}" - : String.format("{\n%s\n%s}", Joiner.on(",\n").join(entries), tabs(depth - 1)); - } - - /** - * Prints a list in a style that is consistent with TBase pretty printing. - * - * @param list The list to print - * @param depth The print nesting level. - * @return The pretty-printed version of the list - */ - private static String printList(List<?> list, int depth) { - List<String> entries = Lists.newArrayList(); - for (int i = 0; i < list.size(); i++) { - entries.add( - String.format("%sItem[%d] = %s", tabs(depth), i, printValue(list.get(i), depth))); - } - - return entries.isEmpty() ? "[]" - : String.format("[\n%s\n%s]", Joiner.on(",\n").join(entries), tabs(depth - 1)); - } - /** - * Prints a set in a style that is consistent with TBase pretty printing. - * - * @param set The set to print - * @param depth The print nesting level. - * @return The pretty-printed version of the set - */ - private static String printSet(Set<?> set, int depth) { - List<String> entries = Lists.newArrayList(); - for (Object item : set) { - entries.add( - String.format("%sItem = %s", tabs(depth), printValue(item, depth))); - } - - return entries.isEmpty() ? "{}" - : String.format("{\n%s\n%s}", Joiner.on(",\n").join(entries), tabs(depth - 1)); - } - - private static String tabs(int n) { - return Strings.repeat(" ", n); - } - - private Util() { - // Utility class. - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/java/org/apache/aurora/common/zookeeper/Encoding.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/Encoding.java b/commons/src/main/java/org/apache/aurora/common/zookeeper/Encoding.java deleted file mode 100644 index 204f5c4..0000000 --- a/commons/src/main/java/org/apache/aurora/common/zookeeper/Encoding.java +++ /dev/null @@ -1,87 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.zookeeper; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.util.Map; - -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; - -/** - * Utility class for encoding and decoding data stored in ZooKeeper nodes. - */ -public class Encoding { - /** - * Encodes a {@link ServiceInstance} as a JSON object. - * - * This is the default encoding for service instance data in ZooKeeper. - */ - public static final Codec<ServiceInstance> JSON_CODEC = new JsonCodec(); - - private Encoding() { - // Utility class. - } - - /** - * Returns a serialized Thrift service instance object, with given endpoints and codec. - * - * @param serviceInstance the Thrift service instance object to be serialized - * @param codec the codec to use to serialize a Thrift service instance object - * @return byte array that contains a serialized Thrift service instance - */ - static byte[] serializeServiceInstance( - ServiceInstance serviceInstance, Codec<ServiceInstance> codec) throws IOException { - - ByteArrayOutputStream output = new ByteArrayOutputStream(); - codec.serialize(serviceInstance, output); - return output.toByteArray(); - } - - /** - * Serializes a service instance based on endpoints. - * @see #serializeServiceInstance(ServiceInstance, Codec) - * - * @param address the target address of the service instance - * @param additionalEndpoints additional endpoints of the service instance - * @param status service status - */ - static byte[] serializeServiceInstance( - InetSocketAddress address, - Map<String, Endpoint> additionalEndpoints, - Status status, - Codec<ServiceInstance> codec) throws IOException { - - ServiceInstance serviceInstance = new ServiceInstance( - new Endpoint(address.getHostName(), address.getPort()), additionalEndpoints, status); - return serializeServiceInstance(serviceInstance, codec); - } - - /** - * Creates a service instance object deserialized from byte array. - * - * @param data the byte array contains a serialized Thrift service instance - * @param codec the codec to use to deserialize the byte array - */ - static ServiceInstance deserializeServiceInstance( - byte[] data, Codec<ServiceInstance> codec) throws IOException { - - return codec.deserialize(new ByteArrayInputStream(data)); - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/java/org/apache/aurora/common/zookeeper/JsonCodec.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/JsonCodec.java b/commons/src/main/java/org/apache/aurora/common/zookeeper/JsonCodec.java deleted file mode 100644 index 9d31608..0000000 --- a/commons/src/main/java/org/apache/aurora/common/zookeeper/JsonCodec.java +++ /dev/null @@ -1,139 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.zookeeper; - -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.Writer; -import java.nio.charset.Charset; -import java.util.Map; - -import javax.annotation.Nullable; - -import com.google.common.base.Charsets; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Maps; -import com.google.gson.Gson; -import com.google.gson.JsonIOException; -import com.google.gson.JsonParseException; - -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; - -import static java.util.Objects.requireNonNull; - -class JsonCodec implements Codec<ServiceInstance> { - - private static void assertRequiredField(String fieldName, Object fieldValue) { - if (fieldValue == null) { - throw new JsonParseException(String.format("Field %s is required", fieldName)); - } - } - - private static class EndpointSchema { - private final String host; - private final Integer port; - - EndpointSchema(Endpoint endpoint) { - host = endpoint.getHost(); - port = endpoint.getPort(); - } - - Endpoint asEndpoint() { - assertRequiredField("host", host); - assertRequiredField("port", port); - - return new Endpoint(host, port); - } - } - - private static class ServiceInstanceSchema { - private final EndpointSchema serviceEndpoint; - private final Map<String, EndpointSchema> additionalEndpoints; - private final Status status; - private final @Nullable Integer shard; - - ServiceInstanceSchema(ServiceInstance instance) { - serviceEndpoint = new EndpointSchema(instance.getServiceEndpoint()); - if (instance.isSetAdditionalEndpoints()) { - additionalEndpoints = - Maps.transformValues(instance.getAdditionalEndpoints(), EndpointSchema::new); - } else { - additionalEndpoints = ImmutableMap.of(); - } - status = instance.getStatus(); - shard = instance.isSetShard() ? instance.getShard() : null; - } - - ServiceInstance asServiceInstance() { - assertRequiredField("serviceEndpoint", serviceEndpoint); - assertRequiredField("status", status); - - Map<String, EndpointSchema> extraEndpoints = - additionalEndpoints == null ? ImmutableMap.of() : additionalEndpoints; - - ServiceInstance instance = - new ServiceInstance( - serviceEndpoint.asEndpoint(), - Maps.transformValues(extraEndpoints, EndpointSchema::asEndpoint), - status); - if (shard != null) { - instance.setShard(shard); - } - return instance; - } - } - - private static final Charset ENCODING = Charsets.UTF_8; - - private final Gson gson; - - JsonCodec() { - this(new Gson()); - } - - JsonCodec(Gson gson) { - this.gson = requireNonNull(gson); - } - - @Override - public void serialize(ServiceInstance instance, OutputStream sink) throws IOException { - Writer writer = new OutputStreamWriter(sink, ENCODING); - try { - gson.toJson(new ServiceInstanceSchema(instance), writer); - } catch (JsonIOException e) { - throw new IOException(String.format("Problem serializing %s to JSON", instance), e); - } - writer.flush(); - } - - @Override - public ServiceInstance deserialize(InputStream source) throws IOException { - InputStreamReader reader = new InputStreamReader(source, ENCODING); - try { - @Nullable ServiceInstanceSchema schema = gson.fromJson(reader, ServiceInstanceSchema.class); - if (schema == null) { - throw new IOException("JSON did not include a ServiceInstance object"); - } - return schema.asServiceInstance(); - } catch (JsonParseException e) { - throw new IOException("Problem parsing JSON ServiceInstance.", e); - } - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/resources/org/apache/aurora/common/net/http/handlers/thrift.st ---------------------------------------------------------------------- diff --git a/commons/src/main/resources/org/apache/aurora/common/net/http/handlers/thrift.st b/commons/src/main/resources/org/apache/aurora/common/net/http/handlers/thrift.st deleted file mode 100644 index 0586653..0000000 --- a/commons/src/main/resources/org/apache/aurora/common/net/http/handlers/thrift.st +++ /dev/null @@ -1,64 +0,0 @@ -<html> - <body> - <h2>Clients</h2> - $if(clientMonitors)$ - $clientMonitors:{ monitor | - <div> - <h3>$monitor.serviceName$</h3> - Lifetime Requests: $monitor.lifetimeRequestCount$ - <br /> - <table border=0> - <tr> - <th>Server</th> - <th>Leased Connections</th> - <th>Successful Requests</th> - <th>Failed Requests</th> - </tr> - $monitor.trafficInfo:{ trafficInfo | - <tr> - <td align='center'>$trafficInfo.key$</td> - <td align='center'>$trafficInfo.connectionCount$</td> - <td align='center'>$trafficInfo.requestSuccessCount$</td> - <td align='center'>$trafficInfo.requestFailureCount$</td> - </tr> - }$ - </table> - </div> - }$ - $else$ - No thrift clients registered. - $endif$ - - <br /> - <br /> - - <h2>Servers</h2> - $if(serverMonitors)$ - $serverMonitors:{ monitor | - <div> - <h3>$monitor.serviceName$</h3> - Lifetime Requests: $monitor.lifetimeRequestCount$ - <br /> - <table border=0> - <tr> - <th>Client</th> - <th>Active Connections</th> - <th>Successful Requests</th> - <th>Failed Requests</th> - </tr> - $monitor.trafficInfo:{ trafficInfo | - <tr> - <td align='center'>$trafficInfo.key$</td> - <td align='center'>$trafficInfo.connectionCount$</td> - <td align='center'>$trafficInfo.requestSuccessCount$</td> - <td align='center'>$trafficInfo.requestFailureCount$</td> - </tr> - }$ - </table> - </div> - }$ - $else$ - No thrift servers registered. - $endif$ - </body> -</html> http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/main/thrift/org/apache/aurora/common/thrift/endpoint.thrift ---------------------------------------------------------------------- diff --git a/commons/src/main/thrift/org/apache/aurora/common/thrift/endpoint.thrift b/commons/src/main/thrift/org/apache/aurora/common/thrift/endpoint.thrift deleted file mode 100644 index 87da2e1..0000000 --- a/commons/src/main/thrift/org/apache/aurora/common/thrift/endpoint.thrift +++ /dev/null @@ -1,90 +0,0 @@ -namespace java org.apache.aurora.common.thrift - -/* - * Represents the status of a service. - */ -enum Status { - - /* - * The service is dead and can no longer be contacted. - */ - DEAD = 0, - - /* - * The service is in the process of starting up for the first time or from a STOPPED state. - */ - STARTING = 1, - - /* - * The service is alive and ready to receive requests. - */ - ALIVE = 2, - - /* - * The service is in the process of stopping and should no longer be contacted. In this state - * well behaved services will typically finish existing requests but accept no new rtequests. - */ - STOPPING = 3, - - /* - * The service is stopped and cannot be contacted unless started again. - */ - STOPPED = 4, - - /* - * The service is alive but in a potentially bad state. - */ - WARNING = 5, -} - -/* - * Represents a TCP service network endpoint. - */ -struct Endpoint { - - /* - * The remote hostname or ip address of the endpoint. - */ - 1: string host - - /* - * The TCP port the endpoint listens on. - */ - 2: i32 port -} - -/* - * Represents information about the state of a service instance. - */ -struct ServiceInstance { - - /* - * Represents the primary service interface endpoint. This is typically a thrift service - * endpoint. - */ - 1: Endpoint serviceEndpoint - - /* - * A mapping of any additional interfaces the service exports. The mapping is from logical - * interface names to endpoints. The map may be empty, but a typical additional endpoint mapping - * would provide the endoint got the "http-admin" debug interface for example. - * - * TODO(John Sirois): consider promoting string -> Enum or adding thrift string constants for common - * service names to help identify common beasts like ostrich-http-admin, ostrich-telnet and - * process-http-admin but still allow for new experimental interfaces as well without having to - * change this thift file. - */ - 2: map<string, Endpoint> additionalEndpoints - - /* - * The status of this service instance. - * NOTE: Only status ALIVE should be used. This field is pending removal. - * TODO(Sathya Hariesh): Remove the status field. - */ - 3: Status status; - - /* - * The shard identifier for this instance. - */ - 4: optional i32 shard; -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/test/java/org/apache/aurora/common/zookeeper/EncodingTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/zookeeper/EncodingTest.java b/commons/src/test/java/org/apache/aurora/common/zookeeper/EncodingTest.java deleted file mode 100644 index 16c0171..0000000 --- a/commons/src/test/java/org/apache/aurora/common/zookeeper/EncodingTest.java +++ /dev/null @@ -1,44 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.zookeeper; - -import java.net.InetSocketAddress; -import java.util.Map; - -import com.google.common.collect.ImmutableMap; - -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; -import org.junit.Test; - -import static org.junit.Assert.assertEquals; - -public class EncodingTest { - @Test - public void testSimpleSerialization() throws Exception { - InetSocketAddress endpoint = new InetSocketAddress(12345); - Map<String, Endpoint > additionalEndpoints = ImmutableMap.of(); - Status status = Status.ALIVE; - - byte[] data = Encoding.serializeServiceInstance( - endpoint, additionalEndpoints, status, Encoding.JSON_CODEC); - - ServiceInstance instance = Encoding.deserializeServiceInstance(data, Encoding.JSON_CODEC); - - assertEquals(endpoint.getPort(), instance.getServiceEndpoint().getPort()); - assertEquals(additionalEndpoints, instance.getAdditionalEndpoints()); - assertEquals(Status.ALIVE, instance.getStatus()); - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java b/commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java deleted file mode 100644 index 6cf335d..0000000 --- a/commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java +++ /dev/null @@ -1,151 +0,0 @@ -/** - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.aurora.common.zookeeper; - -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; - -import com.google.common.base.Charsets; -import com.google.common.collect.ImmutableMap; -import com.google.gson.Gson; -import com.google.gson.JsonIOException; - -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; -import org.easymock.EasyMock; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.api.easymock.PowerMock; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - -@RunWith(PowerMockRunner.class) -@PrepareForTest(Gson.class) -public class JsonCodecTest { - - private static final Codec<ServiceInstance> STANDARD_JSON_CODEC = new JsonCodec(); - - @Test - public void testJsonCodecRoundtrip() throws Exception { - Codec<ServiceInstance> codec = STANDARD_JSON_CODEC; - ServiceInstance instance1 = new ServiceInstance( - new Endpoint("foo", 1000), - ImmutableMap.of("http", new Endpoint("foo", 8080)), - Status.ALIVE) - .setShard(0); - byte[] data = Encoding.serializeServiceInstance(instance1, codec); - assertTrue(Encoding.deserializeServiceInstance(data, codec).getServiceEndpoint().isSetPort()); - assertTrue(Encoding.deserializeServiceInstance(data, codec).isSetShard()); - - ServiceInstance instance2 = new ServiceInstance( - new Endpoint("foo", 1000), - ImmutableMap.of("http-admin1", new Endpoint("foo", 8080)), - Status.ALIVE); - data = Encoding.serializeServiceInstance(instance2, codec); - assertTrue(Encoding.deserializeServiceInstance(data, codec).getServiceEndpoint().isSetPort()); - assertFalse(Encoding.deserializeServiceInstance(data, codec).isSetShard()); - - ServiceInstance instance3 = new ServiceInstance( - new Endpoint("foo", 1000), - ImmutableMap.<String, Endpoint>of(), - Status.ALIVE); - data = Encoding.serializeServiceInstance(instance3, codec); - assertTrue(Encoding.deserializeServiceInstance(data, codec).getServiceEndpoint().isSetPort()); - assertFalse(Encoding.deserializeServiceInstance(data, codec).isSetShard()); - } - - @Test - public void testJsonCompatibility() throws IOException { - ServiceInstance instance = new ServiceInstance( - new Endpoint("foo", 1000), - ImmutableMap.of("http", new Endpoint("foo", 8080)), - Status.ALIVE).setShard(42); - - ByteArrayOutputStream results = new ByteArrayOutputStream(); - STANDARD_JSON_CODEC.serialize(instance, results); - assertEquals( - "{\"serviceEndpoint\":{\"host\":\"foo\",\"port\":1000}," - + "\"additionalEndpoints\":{\"http\":{\"host\":\"foo\",\"port\":8080}}," - + "\"status\":\"ALIVE\"," - + "\"shard\":42}", - results.toString()); - } - - @Test - public void testInvalidSerialize() { - // Gson is final so we need to call on PowerMock here. - Gson gson = PowerMock.createMock(Gson.class); - gson.toJson(EasyMock.isA(Object.class), EasyMock.isA(Appendable.class)); - EasyMock.expectLastCall().andThrow(new JsonIOException("error")); - PowerMock.replay(gson); - - ServiceInstance instance = - new ServiceInstance(new Endpoint("foo", 1000), ImmutableMap.of(), Status.ALIVE); - - try { - new JsonCodec(gson).serialize(instance, new ByteArrayOutputStream()); - fail(); - } catch (IOException e) { - // Expected. - } - - PowerMock.verify(gson); - } - - @Test - public void testDeserializeMinimal() throws IOException { - String minimal = "{\"serviceEndpoint\":{\"host\":\"foo\",\"port\":1000},\"status\":\"ALIVE\"}"; - ByteArrayInputStream source = new ByteArrayInputStream(minimal.getBytes(Charsets.UTF_8)); - ServiceInstance actual = STANDARD_JSON_CODEC.deserialize(source); - ServiceInstance expected = - new ServiceInstance(new Endpoint("foo", 1000), ImmutableMap.of(), Status.ALIVE); - assertEquals(expected, actual); - } - - @Test - public void testInvalidDeserialize() { - // Not JSON. - assertInvalidDeserialize(new byte[] {0xC, 0xA, 0xF, 0xE}); - - // No JSON object. - assertInvalidDeserialize(""); - assertInvalidDeserialize("[]"); - - // Missing required fields. - assertInvalidDeserialize("{}"); - assertInvalidDeserialize("{\"serviceEndpoint\":{\"host\":\"foo\",\"port\":1000}}"); - assertInvalidDeserialize("{\"status\":\"ALIVE\"}"); - } - - private void assertInvalidDeserialize(String data) { - assertInvalidDeserialize(data.getBytes(Charsets.UTF_8)); - } - - private void assertInvalidDeserialize(byte[] data) { - try { - STANDARD_JSON_CODEC.deserialize(new ByteArrayInputStream(data)); - fail(); - } catch (IOException e) { - // Expected. - } - } -} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/app/ServiceGroupMonitor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/app/ServiceGroupMonitor.java b/src/main/java/org/apache/aurora/scheduler/app/ServiceGroupMonitor.java index a1329fd..71171a6 100644 --- a/src/main/java/org/apache/aurora/scheduler/app/ServiceGroupMonitor.java +++ b/src/main/java/org/apache/aurora/scheduler/app/ServiceGroupMonitor.java @@ -18,7 +18,7 @@ import java.util.function.Supplier; import com.google.common.collect.ImmutableSet; -import org.apache.aurora.common.thrift.ServiceInstance; +import org.apache.aurora.scheduler.discovery.ServiceInstance; /** * Monitors a service group's membership and supplies a live view of the most recent set. http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java index 77f90ee..361f7cc 100644 --- a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java +++ b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java @@ -26,18 +26,14 @@ import com.google.common.collect.FluentIterable; import com.google.inject.Exposed; import com.google.inject.PrivateModule; import com.google.inject.Provides; -import com.google.inject.TypeLiteral; import org.apache.aurora.common.application.ShutdownRegistry; import org.apache.aurora.common.base.MorePreconditions; -import org.apache.aurora.common.io.Codec; import org.apache.aurora.common.net.InetSocketAddressHelper; import org.apache.aurora.common.quantity.Amount; import org.apache.aurora.common.quantity.Time; import org.apache.aurora.common.stats.StatsProvider; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.aurora.common.zookeeper.Credentials; -import org.apache.aurora.common.zookeeper.Encoding; import org.apache.aurora.common.zookeeper.SingletonService; import org.apache.aurora.scheduler.app.ServiceGroupMonitor; import org.apache.curator.RetryPolicy; @@ -75,8 +71,6 @@ class CuratorServiceDiscoveryModule extends PrivateModule { protected void configure() { requireBinding(ServiceDiscoveryBindings.ZOO_KEEPER_CLUSTER_KEY); requireBinding(ServiceDiscoveryBindings.ZOO_KEEPER_ACL_KEY); - - bind(new TypeLiteral<Codec<ServiceInstance>>() { }).toInstance(Encoding.JSON_CODEC); } @Provides @@ -216,8 +210,7 @@ class CuratorServiceDiscoveryModule extends PrivateModule { @Exposed ServiceGroupMonitor provideServiceGroupMonitor( ShutdownRegistry shutdownRegistry, - CuratorFramework client, - Codec<ServiceInstance> codec) { + CuratorFramework client) { PathChildrenCache groupCache = new PathChildrenCache(client, discoveryPath, true /* cacheData */); @@ -230,7 +223,7 @@ class CuratorServiceDiscoveryModule extends PrivateModule { // when it is closed to avoid errors in those clients when attempting to use it. ServiceGroupMonitor serviceGroupMonitor = - new CuratorServiceGroupMonitor(groupCache, MEMBER_SELECTOR, codec); + new CuratorServiceGroupMonitor(groupCache, MEMBER_SELECTOR); shutdownRegistry.addAction(groupCache::close); return serviceGroupMonitor; @@ -239,8 +232,8 @@ class CuratorServiceDiscoveryModule extends PrivateModule { @Provides @Singleton @Exposed - SingletonService provideSingletonService(CuratorFramework client, Codec<ServiceInstance> codec) { - return new CuratorSingletonService(client, discoveryPath, MEMBER_TOKEN, codec); + SingletonService provideSingletonService(CuratorFramework client) { + return new CuratorSingletonService(client, discoveryPath, MEMBER_TOKEN); } private String zkConnectionStateCounterName(ConnectionState state) { http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java index eba56be..b4a373d 100644 --- a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java +++ b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java @@ -13,16 +13,14 @@ */ package org.apache.aurora.scheduler.discovery; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.util.Optional; import java.util.function.Predicate; import com.google.common.collect.ImmutableSet; +import com.google.gson.JsonSyntaxException; import org.apache.aurora.GuavaUtils; -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.aurora.scheduler.app.ServiceGroupMonitor; import org.apache.curator.framework.recipes.cache.ChildData; import org.apache.curator.framework.recipes.cache.PathChildrenCache; @@ -37,7 +35,6 @@ class CuratorServiceGroupMonitor implements ServiceGroupMonitor { private final PathChildrenCache groupCache; private final Predicate<String> memberSelector; - private final Codec<ServiceInstance> codec; /** * Creates a {@code ServiceGroupMonitor} backed by Curator. @@ -54,16 +51,10 @@ class CuratorServiceGroupMonitor implements ServiceGroupMonitor { * @param memberSelector A predicate that returns {@code true} for group node names that represent * group members. Here the name is just the `basename` of the node's full * ZooKeeper path. - * @param codec A codec that can be used to deserialize group member {@link ServiceInstance} data. */ - CuratorServiceGroupMonitor( - PathChildrenCache groupCache, - Predicate<String> memberSelector, - Codec<ServiceInstance> codec) { - + CuratorServiceGroupMonitor(PathChildrenCache groupCache, Predicate<String> memberSelector) { this.groupCache = requireNonNull(groupCache); this.memberSelector = requireNonNull(memberSelector); - this.codec = requireNonNull(codec); } @Override @@ -104,10 +95,9 @@ class CuratorServiceGroupMonitor implements ServiceGroupMonitor { } private Optional<ServiceInstance> extractServiceInstance(ChildData data) { - ByteArrayInputStream source = new ByteArrayInputStream(data.getData()); try { - return Optional.of(codec.deserialize(source)); - } catch (IOException e) { + return Optional.of(Encoding.decode(data.getData())); + } catch (JsonSyntaxException e) { LOG.error("Failed to deserialize ServiceInstance from " + data, e); return Optional.empty(); } http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java index 2847c41..eec07bf 100644 --- a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java +++ b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorSingletonService.java @@ -13,7 +13,6 @@ */ package org.apache.aurora.scheduler.discovery; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.net.InetSocketAddress; import java.util.Map; @@ -25,12 +24,9 @@ import com.google.common.collect.Maps; import com.google.common.io.Closer; import org.apache.aurora.common.base.MorePreconditions; -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; import org.apache.aurora.common.zookeeper.SingletonService; import org.apache.aurora.scheduler.base.AsyncUtil; +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.leader.CancelLeadershipException; import org.apache.curator.framework.recipes.leader.LeaderSelector; @@ -56,18 +52,11 @@ class CuratorSingletonService implements SingletonService { private final String groupPath; private final String memberToken; private final CuratorFramework client; - private final Codec<ServiceInstance> codec; - - Advertiser( - CuratorFramework client, - String groupPath, - String memberToken, - Codec<ServiceInstance> codec) { + Advertiser(CuratorFramework client, String groupPath, String memberToken) { this.client = requireNonNull(client); this.groupPath = PathUtils.validatePath(groupPath); this.memberToken = MorePreconditions.checkNotBlank(memberToken); - this.codec = requireNonNull(codec); } void advertise( @@ -109,17 +98,14 @@ class CuratorSingletonService implements SingletonService { ServiceInstance serviceInstance = new ServiceInstance( asEndpoint(endpoint), - Maps.transformValues(additionalEndpoints, Advertiser::asEndpoint), - Status.ALIVE); + Maps.transformValues(additionalEndpoints, Advertiser::asEndpoint)); - ByteArrayOutputStream sink = new ByteArrayOutputStream(); try { - codec.serialize(serviceInstance, sink); + return Encoding.encode(serviceInstance); } catch (IOException e) { throw new AdvertiseException( "Problem serializing service instance data for " + serviceInstance, e); } - return sink.toByteArray(); } private static Endpoint asEndpoint(InetSocketAddress endpoint) { @@ -137,14 +123,9 @@ class CuratorSingletonService implements SingletonService { * @param client A client to interact with a ZooKeeper ensemble. * @param groupPath The root ZooKeeper path service members advertise their presence under. * @param memberToken A token used to form service member node names. - * @param codec A codec that can be used to deserialize group member {@link ServiceInstance} data. */ - CuratorSingletonService( - CuratorFramework client, - String groupPath, - String memberToken, - Codec<ServiceInstance> codec) { - advertiser = new Advertiser(client, groupPath, memberToken, codec); + CuratorSingletonService(CuratorFramework client, String groupPath, String memberToken) { + advertiser = new Advertiser(client, groupPath, memberToken); this.client = client; this.groupPath = PathUtils.validatePath(groupPath); } http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/discovery/Encoding.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/Encoding.java b/src/main/java/org/apache/aurora/scheduler/discovery/Encoding.java new file mode 100644 index 0000000..17196da --- /dev/null +++ b/src/main/java/org/apache/aurora/scheduler/discovery/Encoding.java @@ -0,0 +1,71 @@ +/** + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.aurora.scheduler.discovery; + +import java.io.IOException; + +import com.google.common.base.Charsets; +import com.google.gson.Gson; +import com.google.gson.JsonParseException; +import com.google.gson.JsonSyntaxException; + +/** + * Utility class for encoding and decoding data stored in ZooKeeper nodes. + */ +public final class Encoding { + + private static final Gson GSON = new Gson(); + + private Encoding() { + // Utility class. + } + + /** + * Returns a serialized Thrift service instance object, with given endpoints and codec. + * + * @param serviceInstance the Thrift service instance object to be serialized + * @return byte array that contains a serialized Thrift service instance + */ + public static byte[] encode(ServiceInstance serviceInstance) throws IOException { + return GSON.toJson(serviceInstance).getBytes(Charsets.UTF_8); + } + + /** + * Creates a service instance object deserialized from byte array. + * + * @param data the byte array contains a serialized Thrift service instance + */ + public static ServiceInstance decode(byte[] data) throws JsonSyntaxException { + ServiceInstance instance = + GSON.fromJson(new String(data, Charsets.UTF_8), ServiceInstance.class); + assertRequiredField("serviceInstance", instance); + assertRequiredField("serviceEndpoint", instance.getServiceEndpoint()); + assertRequiredFields(instance.getServiceEndpoint()); + if (instance.getAdditionalEndpoints() != null) { + instance.getAdditionalEndpoints().values().forEach(Encoding::assertRequiredFields); + } + + return instance; + } + + private static void assertRequiredFields(ServiceInstance.Endpoint endpoint) { + assertRequiredField("host", endpoint.getHost()); + } + + private static void assertRequiredField(String fieldName, Object fieldValue) { + if (fieldValue == null) { + throw new JsonParseException(String.format("Field %s is required", fieldName)); + } + } +} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/discovery/ServiceInstance.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/ServiceInstance.java b/src/main/java/org/apache/aurora/scheduler/discovery/ServiceInstance.java new file mode 100644 index 0000000..41840af --- /dev/null +++ b/src/main/java/org/apache/aurora/scheduler/discovery/ServiceInstance.java @@ -0,0 +1,125 @@ +/** + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.aurora.scheduler.discovery; + +import java.util.Map; +import java.util.Objects; + +import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableMap; + +import static java.util.Objects.requireNonNull; + +public class ServiceInstance { + private final Endpoint serviceEndpoint; + private final Map<String, Endpoint> additionalEndpoints; + + // Legacy field. + private final String status; + + /** + * Default constructor for gson, needed to inject a default empty map. + */ + ServiceInstance() { + this.serviceEndpoint = null; + this.additionalEndpoints = ImmutableMap.of(); + this.status = "ALIVE"; + } + + public ServiceInstance(Endpoint serviceEndpoint, Map<String, Endpoint> additionalEndpoints) { + this.serviceEndpoint = requireNonNull(serviceEndpoint); + this.additionalEndpoints = requireNonNull(additionalEndpoints); + this.status = "ALIVE"; + } + + public Endpoint getServiceEndpoint() { + return serviceEndpoint; + } + + public Map<String, Endpoint> getAdditionalEndpoints() { + return additionalEndpoints; + } + + public String getStatus() { + return status; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof ServiceInstance)) { + return false; + } + + ServiceInstance other = (ServiceInstance) obj; + return Objects.equals(serviceEndpoint, other.serviceEndpoint) + && Objects.equals(additionalEndpoints, other.additionalEndpoints) + && status.equals(other.status); + } + + @Override + public int hashCode() { + return Objects.hash(serviceEndpoint, additionalEndpoints, status); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("serviceEndpoint", serviceEndpoint) + .add("additionalEndpoints", additionalEndpoints) + .add("status", status) + .toString(); + } + + public static class Endpoint { + private final String host; + private final int port; + + public Endpoint(String host, int port) { + this.host = requireNonNull(host); + this.port = port; + } + + public String getHost() { + return host; + } + + public int getPort() { + return port; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof Endpoint)) { + return false; + } + + Endpoint other = (Endpoint) obj; + return Objects.equals(host, other.host) + && port == other.port; + } + + @Override + public int hashCode() { + return Objects.hash(host, port); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("host", host) + .add("port", port) + .toString(); + } + } +} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java index bc0e2a8..41bed3e 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java +++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java @@ -25,10 +25,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.net.HostAndPort; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.aurora.scheduler.app.ServiceGroupMonitor; import org.apache.aurora.scheduler.app.ServiceGroupMonitor.MonitorException; +import org.apache.aurora.scheduler.discovery.ServiceInstance; +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,11 +85,9 @@ class LeaderRedirect implements Closeable { private Optional<HostAndPort> getLeaderHttp() { Optional<ServiceInstance> leadingScheduler = getLeader(); - if (leadingScheduler.isPresent() && leadingScheduler.get().isSetServiceEndpoint()) { + if (leadingScheduler.isPresent()) { Endpoint leaderHttp = leadingScheduler.get().getServiceEndpoint(); - if (leaderHttp != null && leaderHttp.isSetHost() && leaderHttp.isSetPort()) { - return Optional.of(HostAndPort.fromParts(leaderHttp.getHost(), leaderHttp.getPort())); - } + return Optional.of(HostAndPort.fromParts(leaderHttp.getHost(), leaderHttp.getPort())); } LOG.warn("Leader service instance seems to be incomplete: " + leadingScheduler); @@ -137,11 +135,6 @@ class LeaderRedirect implements Closeable { return LeaderStatus.NO_LEADER; } - if (!leadingScheduler.get().isSetServiceEndpoint()) { - LOG.warn("Leader service instance seems to be incomplete: " + leadingScheduler); - return LeaderStatus.NO_LEADER; - } - Optional<HostAndPort> leaderHttp = getLeaderHttp(); Optional<HostAndPort> localHttp = getLocalHttp(); http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/main/java/org/apache/aurora/scheduler/http/StructDump.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java index 3ed256b..8826d25 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java +++ b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java @@ -23,8 +23,9 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import com.google.common.base.Optional; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; -import org.apache.aurora.common.thrift.Util; import org.apache.aurora.scheduler.base.JobKeys; import org.apache.aurora.scheduler.storage.Storage; import org.apache.aurora.scheduler.storage.Storage.Work.Quiet; @@ -98,12 +99,14 @@ public class StructDump extends JerseyTemplateServlet { .transform(IJobConfiguration::newBuilder)); } - private Response dumpEntity(final String id, final Quiet<Optional<? extends TBase<?, ?>>> work) { + private static final Gson GSON = new GsonBuilder().setPrettyPrinting().create(); + + private Response dumpEntity(String id, Quiet<Optional<? extends TBase<?, ?>>> work) { return fillTemplate(template -> { template.setAttribute("id", id); Optional<? extends TBase<?, ?>> struct = storage.read(work); if (struct.isPresent()) { - template.setAttribute("structPretty", Util.prettyPrint(struct.get())); + template.setAttribute("structPretty", GSON.toJson(struct.get())); template.setAttribute("exception", null); } else { template.setAttribute("exception", "Entity not found"); http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java index 02c8183..51f13ef 100644 --- a/src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java +++ b/src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java @@ -13,21 +13,15 @@ */ package org.apache.aurora.scheduler.discovery; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.function.Predicate; import com.google.common.collect.ImmutableMap; -import org.apache.aurora.common.io.Codec; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; -import org.apache.aurora.common.thrift.Status; -import org.apache.aurora.common.zookeeper.Encoding; import org.apache.aurora.common.zookeeper.testing.BaseZooKeeperTest; import org.apache.aurora.scheduler.app.ServiceGroupMonitor; +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.CuratorFrameworkFactory; import org.apache.curator.framework.recipes.cache.PathChildrenCache; @@ -38,7 +32,6 @@ class BaseCuratorDiscoveryTest extends BaseZooKeeperTest { static final String GROUP_PATH = "/group/root"; static final String MEMBER_TOKEN = "member_"; - static final Codec<ServiceInstance> CODEC = Encoding.JSON_CODEC; static final int PRIMARY_PORT = 42; private CuratorFramework client; @@ -55,7 +48,7 @@ class BaseCuratorDiscoveryTest extends BaseZooKeeperTest { groupCache.getListenable().addListener((c, event) -> groupEvents.put(event)); Predicate<String> memberSelector = name -> name.contains(MEMBER_TOKEN); - groupMonitor = new CuratorServiceGroupMonitor(groupCache, memberSelector, Encoding.JSON_CODEC); + groupMonitor = new CuratorServiceGroupMonitor(groupCache, memberSelector); } final CuratorFramework startNewClient() { @@ -104,16 +97,7 @@ class BaseCuratorDiscoveryTest extends BaseZooKeeperTest { } } - final byte[] serialize(ServiceInstance serviceInstance) throws IOException { - ByteArrayOutputStream sink = new ByteArrayOutputStream(); - CODEC.serialize(serviceInstance, sink); - return sink.toByteArray(); - } - final ServiceInstance serviceInstance(String hostName) { - return new ServiceInstance( - new Endpoint(hostName, PRIMARY_PORT), - ImmutableMap.of(), - Status.ALIVE); + return new ServiceInstance(new Endpoint(hostName, PRIMARY_PORT), ImmutableMap.of()); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java index e6b57ee..c920d07 100644 --- a/src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java +++ b/src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java @@ -13,10 +13,9 @@ */ package org.apache.aurora.scheduler.discovery; +import com.google.common.base.Charsets; import com.google.common.collect.ImmutableSet; -import org.apache.aurora.codec.ThriftBinaryCodec; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; import org.apache.curator.utils.ZKPaths; import org.apache.zookeeper.CreateMode; @@ -98,7 +97,7 @@ public class CuratorServiceGroupMonitorTest extends BaseCuratorDiscoveryTest { public void testInvalidMemberNode() throws Exception { startGroupMonitor(); - createMember(ThriftBinaryCodec.encode(serviceInstance("invalid"))); + createMember("invalid".getBytes(Charsets.UTF_8)); ServiceInstance member = serviceInstance("member"); createMember(member); @@ -134,7 +133,7 @@ public class CuratorServiceGroupMonitorTest extends BaseCuratorDiscoveryTest { private String createMember(ServiceInstance serviceInstance, boolean waitForGroupEvent) throws Exception { - return createMember(serialize(serviceInstance), waitForGroupEvent); + return createMember(Encoding.encode(serviceInstance), waitForGroupEvent); } private String createMember(byte[] nodeData) throws Exception { http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java index 946a78e..9909151 100644 --- a/src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java @@ -67,7 +67,7 @@ public class CuratorSingletonServiceTest extends BaseCuratorDiscoveryTest { throws Exception { CuratorSingletonService singletonService = - new CuratorSingletonService(client, GROUP_PATH, MEMBER_TOKEN, CODEC); + new CuratorSingletonService(client, GROUP_PATH, MEMBER_TOKEN); InetSocketAddress leaderEndpoint = InetSocketAddress.createUnresolved(hostName, PRIMARY_PORT); singletonService.lead(leaderEndpoint, ImmutableMap.of(), listener); } http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/discovery/EncodingTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/EncodingTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/EncodingTest.java new file mode 100644 index 0000000..dcda346 --- /dev/null +++ b/src/test/java/org/apache/aurora/scheduler/discovery/EncodingTest.java @@ -0,0 +1,98 @@ +/** + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.aurora.scheduler.discovery; + +import java.io.IOException; + +import com.google.common.base.Charsets; +import com.google.common.collect.ImmutableMap; +import com.google.gson.JsonParseException; + +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class EncodingTest { + + @Test + public void testEncodingRoundTrip() throws Exception { + ServiceInstance instance1 = new ServiceInstance( + new Endpoint("foo", 1000), + ImmutableMap.of("http", new Endpoint("foo", 8080))); + byte[] data = Encoding.encode(instance1); + assertEquals(instance1, Encoding.decode(data)); + + ServiceInstance instance2 = new ServiceInstance( + new Endpoint("foo", 1000), + ImmutableMap.of("http-admin1", new Endpoint("foo", 8080))); + data = Encoding.encode(instance2); + assertEquals(instance2, Encoding.decode(data)); + + ServiceInstance instance3 = new ServiceInstance(new Endpoint("foo", 1000), ImmutableMap.of()); + data = Encoding.encode(instance3); + assertEquals(instance3, Encoding.decode(data)); + } + + @Test + public void testJsonCompatibility() throws IOException { + ServiceInstance instance = new ServiceInstance( + new Endpoint("foo", 1000), + ImmutableMap.of("http", new Endpoint("foo", 8080))); + + byte[] encoded = Encoding.encode(instance); + assertEquals( + "{\"serviceEndpoint\":{\"host\":\"foo\",\"port\":1000}," + + "\"additionalEndpoints\":{\"http\":{\"host\":\"foo\",\"port\":8080}}," + + "\"status\":\"ALIVE\"}", + new String(encoded, Charsets.UTF_8)); + } + + @Test + public void testDeserializeMinimal() throws IOException { + String minimal = "{\"serviceEndpoint\":{\"host\":\"foo\",\"port\":1000},\"status\":\"ALIVE\"}"; + ServiceInstance actual = Encoding.decode(minimal.getBytes(Charsets.UTF_8)); + ServiceInstance expected = + new ServiceInstance(new Endpoint("foo", 1000), ImmutableMap.of()); + assertEquals(expected, actual); + } + + @Test + public void testInvalidDeserialize() { + // Not JSON. + assertInvalidDeserialize(new byte[] {0xC, 0xA, 0xF, 0xE}); + + // No JSON object. + assertInvalidDeserialize(""); + assertInvalidDeserialize("[]"); + + // Missing required fields. + assertInvalidDeserialize("{}"); + assertInvalidDeserialize("{\"status\":\"ALIVE\"}"); + } + + private void assertInvalidDeserialize(String data) { + assertInvalidDeserialize(data.getBytes(Charsets.UTF_8)); + } + + private void assertInvalidDeserialize(byte[] data) { + try { + Encoding.decode(data); + fail(); + } catch (JsonParseException e) { + // Expected. + } + } +} http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java b/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java index f3ae5a5..8301b19 100644 --- a/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java +++ b/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java @@ -20,6 +20,7 @@ import javax.servlet.ServletContextListener; import javax.ws.rs.core.MediaType; import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.net.HostAndPort; import com.google.common.util.concurrent.RateLimiter; @@ -40,8 +41,6 @@ import org.apache.aurora.common.quantity.Amount; import org.apache.aurora.common.quantity.Time; import org.apache.aurora.common.stats.StatsProvider; import org.apache.aurora.common.testing.easymock.EasyMockTest; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.aurora.common.util.BackoffStrategy; import org.apache.aurora.gen.ServerInfo; import org.apache.aurora.scheduler.AppStartup; @@ -52,6 +51,8 @@ import org.apache.aurora.scheduler.app.ServiceGroupMonitor; import org.apache.aurora.scheduler.async.AsyncModule; import org.apache.aurora.scheduler.config.CliOptions; import org.apache.aurora.scheduler.cron.CronJobManager; +import org.apache.aurora.scheduler.discovery.ServiceInstance; +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; import org.apache.aurora.scheduler.http.api.GsonMessageBodyHandler; import org.apache.aurora.scheduler.offers.OfferManager; import org.apache.aurora.scheduler.scheduling.RescheduleCalculator; @@ -151,8 +152,8 @@ public abstract class AbstractJettyTest extends EasyMockTest { } protected void setLeadingScheduler(String host, int port) { - schedulers.set( - ImmutableSet.of(new ServiceInstance().setServiceEndpoint(new Endpoint(host, port)))); + schedulers.set(ImmutableSet.of( + new ServiceInstance(new Endpoint(host, port), ImmutableMap.of()))); } protected void unsetLeadingSchduler() { http://git-wip-us.apache.org/repos/asf/aurora/blob/9e646ae5/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java b/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java index a7cc046..1bd4440 100644 --- a/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java +++ b/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java @@ -20,15 +20,16 @@ import javax.servlet.http.HttpServletRequest; import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.net.HostAndPort; import org.apache.aurora.common.testing.easymock.EasyMockTest; -import org.apache.aurora.common.thrift.Endpoint; -import org.apache.aurora.common.thrift.ServiceInstance; import org.apache.aurora.scheduler.app.ServiceGroupMonitor; import org.apache.aurora.scheduler.app.ServiceGroupMonitor.MonitorException; +import org.apache.aurora.scheduler.discovery.ServiceInstance; +import org.apache.aurora.scheduler.discovery.ServiceInstance.Endpoint; import org.junit.Before; import org.junit.Test; @@ -42,8 +43,9 @@ public class LeaderRedirectTest extends EasyMockTest { private static final int HTTP_PORT = 500; private static final Function<HostAndPort, ServiceInstance> CREATE_INSTANCE = - endpoint -> new ServiceInstance() - .setServiceEndpoint(new Endpoint(endpoint.getHost(), endpoint.getPort())); + endpoint -> new ServiceInstance( + new Endpoint(endpoint.getHost(), endpoint.getPort()), + ImmutableMap.of()); private AtomicReference<ImmutableSet<ServiceInstance>> schedulers; private ServiceGroupMonitor serviceGroupMonitor; @@ -128,16 +130,6 @@ public class LeaderRedirectTest extends EasyMockTest { assertEquals(LeaderStatus.NO_LEADER, leaderRedirector.getLeaderStatus()); } - @Test - public void testBadServiceInstance() throws Exception { - replayAndMonitor(2); - - publishSchedulers(ImmutableSet.of(new ServiceInstance())); - - assertEquals(Optional.absent(), leaderRedirector.getRedirect()); - assertEquals(LeaderStatus.NO_LEADER, leaderRedirector.getLeaderStatus()); - } - private HttpServletRequest mockRequest(String attributeValue, String queryString) { HttpServletRequest mockRequest = createMock(HttpServletRequest.class); expect(mockRequest.getScheme()).andReturn("http");
