diff --git a/core/src/main/java/io/grpc/ServerServiceDefinition.java b/core/src/main/java/io/grpc/ServerServiceDefinition.java index 099f829629..a37a4f116d 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 @@ public final class ServerServiceDefinition { } private final String name; - private final ImmutableList> methods; - private final ImmutableMap> methodLookup; + private final ImmutableMap> methods; - private ServerServiceDefinition(String name, ImmutableList> methods, - Map> methodLookup) { - this.name = name; - this.methods = methods; - this.methodLookup = ImmutableMap.copyOf(methodLookup); + private ServerServiceDefinition( + String name, Map> 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 @@ public final class ServerServiceDefinition { return name; } - public ImmutableList> getMethods() { - return methods; + public Collection> getMethods() { + return methods.values(); } /** @@ -70,16 +71,14 @@ public final class ServerServiceDefinition { * @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> methods - = ImmutableList.builder(); - private final Map> methodLookup - = new HashMap>(); + private final Map> methods = + new HashMap>(); private Builder(String serviceName) { this.serviceName = serviceName; @@ -96,29 +95,27 @@ public final class ServerServiceDefinition { // 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( - 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 Builder addMethod(ServerMethodDefinition 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 e914679a2f..5d5151385e 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.assertSame; 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 @@ public class MutableHandlerRegistryImplTest { 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 @@ public class MutableHandlerRegistryImplTest { 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 @@ public class MutableHandlerRegistryImplTest { .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 ec5f3a0865..95d6eec4a1 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 @@ public class ServerInterceptorsTest { if (serviceDef.getMethods().size() != 1) { throw new AssertionError("Not exactly one method present"); } - return (ServerMethodDefinition) serviceDef.getMethods().get(0); + return (ServerMethodDefinition) getOnlyElement(serviceDef.getMethods()); } @SuppressWarnings("unchecked")