Try to simplify server method definition
diff --git a/core/src/main/java/io/grpc/ServerServiceDefinition.java b/core/src/main/java/io/grpc/ServerServiceDefinition.java
index 099f829..a37a4f1 100644
--- a/core/src/main/java/io/grpc/ServerServiceDefinition.java
+++ b/core/src/main/java/io/grpc/ServerServiceDefinition.java
@@ -31,10 +31,13 @@
package io.grpc;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
import com.google.common.collect.ImmutableMap;
+import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
@@ -45,14 +48,12 @@
}
private final String name;
- private final ImmutableList<ServerMethodDefinition<?, ?>> methods;
- private final ImmutableMap<String, ServerMethodDefinition<?, ?>> methodLookup;
+ private final ImmutableMap<String, ServerMethodDefinition<?, ?>> methods;
- private ServerServiceDefinition(String name, ImmutableList<ServerMethodDefinition<?, ?>> methods,
- Map<String, ServerMethodDefinition<?, ?>> methodLookup) {
- this.name = name;
- this.methods = methods;
- this.methodLookup = ImmutableMap.copyOf(methodLookup);
+ private ServerServiceDefinition(
+ String name, Map<String, ServerMethodDefinition<?, ?>> methods) {
+ this.name = checkNotNull(name);
+ this.methods = ImmutableMap.copyOf(methods);
}
/** Simple name of the service. It is not an absolute path. */
@@ -60,8 +61,8 @@
return name;
}
- public ImmutableList<ServerMethodDefinition<?, ?>> getMethods() {
- return methods;
+ public Collection<ServerMethodDefinition<?, ?>> getMethods() {
+ return methods.values();
}
/**
@@ -70,16 +71,14 @@
* @param name the fully qualified name without leading slash. E.g., "com.foo.Foo/Bar"
*/
public ServerMethodDefinition<?, ?> getMethod(String name) {
- return methodLookup.get(name);
+ return methods.get(name);
}
/** Builder for constructing Service instances. */
public static final class Builder {
private final String serviceName;
- private final ImmutableList.Builder<ServerMethodDefinition<?, ?>> methods
- = ImmutableList.builder();
- private final Map<String, ServerMethodDefinition<?, ?>> methodLookup
- = new HashMap<String, ServerMethodDefinition<?, ?>>();
+ private final Map<String, ServerMethodDefinition<?, ?>> methods =
+ new HashMap<String, ServerMethodDefinition<?, ?>>();
private Builder(String serviceName) {
this.serviceName = serviceName;
@@ -96,29 +95,27 @@
// TODO(zhangkun83): since the handler map uses fully qualified names as keys, we should
// consider removing ServerServiceDefinition to and let the registry to have a big map of
// handlers.
- Preconditions.checkArgument(
+ checkArgument(
serviceName.equals(MethodDescriptor.extractFullServiceName(method.getFullMethodName())),
"Service name mismatch. Expected service name: '%s'. Actual method name: '%s'.",
this.serviceName, method.getFullMethodName());
return addMethod(new ServerMethodDefinition<ReqT, RespT>(
- Preconditions.checkNotNull(method, "method must not be null"),
- Preconditions.checkNotNull(handler, "handler must not be null")));
+ checkNotNull(method, "method must not be null"),
+ checkNotNull(handler, "handler must not be null")));
}
/** Add a method to be supported by the service. */
public <ReqT, RespT> Builder addMethod(ServerMethodDefinition<ReqT, RespT> def) {
- if (methodLookup.containsKey(def.getMethodDescriptor().getFullMethodName())) {
- throw new IllegalStateException("Method by same name already registered");
- }
- methodLookup.put(def.getMethodDescriptor().getFullMethodName(), def);
- methods.add(def);
+ String name = def.getMethodDescriptor().getFullMethodName();
+ checkState(!methods.containsKey(name), "Method by same name already registered: %s", name);
+ methods.put(name, def);
return this;
}
/** Construct new ServerServiceDefinition. */
public ServerServiceDefinition build() {
- return new ServerServiceDefinition(serviceName, methods.build(), methodLookup);
+ return new ServerServiceDefinition(serviceName, methods);
}
}
}
diff --git a/core/src/test/java/io/grpc/MutableHandlerRegistryImplTest.java b/core/src/test/java/io/grpc/MutableHandlerRegistryImplTest.java
index e914679..5d51513 100644
--- a/core/src/test/java/io/grpc/MutableHandlerRegistryImplTest.java
+++ b/core/src/test/java/io/grpc/MutableHandlerRegistryImplTest.java
@@ -31,6 +31,8 @@
package io.grpc;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.getOnlyElement;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -38,8 +40,6 @@
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
-import com.google.common.base.Preconditions;
-
import io.grpc.HandlerRegistry.Method;
import io.grpc.MethodDescriptor.MethodType;
@@ -65,7 +65,8 @@
requestMarshaller, responseMarshaller),
handler).build();
@SuppressWarnings("rawtypes")
- private ServerMethodDefinition flowMethodDefinition = basicServiceDefinition.getMethods().get(0);
+ private ServerMethodDefinition flowMethodDefinition =
+ getOnlyElement(basicServiceDefinition.getMethods());
private ServerServiceDefinition multiServiceDefinition = ServerServiceDefinition.builder("multi")
.addMethod(
MethodDescriptor.create(MethodType.UNKNOWN, "multi", "couple",
@@ -77,10 +78,10 @@
handler).build();
@SuppressWarnings("rawtypes")
private ServerMethodDefinition coupleMethodDefinition =
- Preconditions.checkNotNull(multiServiceDefinition.getMethod("multi/couple"));
+ checkNotNull(multiServiceDefinition.getMethod("multi/couple"));
@SuppressWarnings("rawtypes")
private ServerMethodDefinition fewMethodDefinition =
- Preconditions.checkNotNull(multiServiceDefinition.getMethod("multi/few"));
+ checkNotNull(multiServiceDefinition.getMethod("multi/few"));
/** Final checks for all tests. */
@After
@@ -143,7 +144,7 @@
.addMethod(MethodDescriptor.create(MethodType.UNKNOWN, "basic", "another",
requestMarshaller, responseMarshaller), handler).build();
ServerMethodDefinition<?, ?> anotherMethodDefinition =
- replaceServiceDefinition.getMethods().get(0);
+ replaceServiceDefinition.getMethod("basic/another");
assertSame(basicServiceDefinition, registry.addService(replaceServiceDefinition));
assertNull(registry.lookupMethod("basic/flow"));
diff --git a/core/src/test/java/io/grpc/ServerInterceptorsTest.java b/core/src/test/java/io/grpc/ServerInterceptorsTest.java
index ec5f3a0..95d6eec 100644
--- a/core/src/test/java/io/grpc/ServerInterceptorsTest.java
+++ b/core/src/test/java/io/grpc/ServerInterceptorsTest.java
@@ -31,6 +31,7 @@
package io.grpc;
+import static com.google.common.collect.Iterables.getOnlyElement;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
import static org.mockito.Matchers.same;
@@ -243,7 +244,7 @@
if (serviceDef.getMethods().size() != 1) {
throw new AssertionError("Not exactly one method present");
}
- return (ServerMethodDefinition<String, Integer>) serviceDef.getMethods().get(0);
+ return (ServerMethodDefinition<String, Integer>) getOnlyElement(serviceDef.getMethods());
}
@SuppressWarnings("unchecked")