Internal cleanup (#89)

Co-authored-by: Tilmann Zäschke <tilmann.zaeschke@inf.ethz.ch>

CHANGELOG.md

@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - **BREAKING CHANGE**: `DatagramChannel.receive()` returns a subclass of `InetSocketAddress` 
   [#86](https://github.com/scionproto-contrib/jpan/pull/86) 
 - Internal cleanup. [#88](https://github.com/scionproto-contrib/jpan/pull/88)
+- Deprecated `getPaths(InetSocketAddress)` because it wasn't clear that it did a SCION lookup. 
+  [#89](https://github.com/scionproto-contrib/jpan/pull/89)
 
 ### Fixed
 - Fixed locking and resizing of buffers. [#68](https://github.com/scionproto-contrib/jpan/pull/68)

src/main/java/org/scion/jpan/AbstractDatagramChannel.java

@@ -236,7 +236,7 @@ public C connect(SocketAddress addr) throws IOException {
         throw new IllegalArgumentException(
             "connect() requires an InetSocketAddress or a ScionSocketAddress.");
       }
-      return connect(pathPolicy.filter(getOrCreateService().getPaths((InetSocketAddress) addr)));
+      return connect(getOrCreateService().lookupAndGetPath((InetSocketAddress) addr, pathPolicy));
     }
   }
 

src/main/java/org/scion/jpan/ScionDatagramChannel.java

@@ -93,7 +93,7 @@ public void send(ByteBuffer srcBuffer, SocketAddress destination) throws IOExcep
       return;
     }
     InetSocketAddress dst = (InetSocketAddress) destination;
-    Path path = getPathPolicy().filter(getOrCreateService().getPaths(dst));
+    Path path = getOrCreateService().lookupAndGetPath(dst, getPathPolicy());
     send(srcBuffer, path);
   }
 

src/main/java/org/scion/jpan/ScionDatagramSocket.java

@@ -296,7 +296,7 @@ public void send(DatagramPacket packet) throws IOException {
         synchronized (pathCache) {
           path = pathCache.get(addr);
           if (path == null) {
-            path = channel.getPathPolicy().filter(channel.getOrCreateService2().getPaths(addr));
+            path = channel.getOrCreateService().lookupAndGetPath(addr, channel.getPathPolicy());
           } else if (path instanceof RequestPath
               && ((RequestPath) path).getExpiration() > Instant.now().getEpochSecond()) {
             // check expiration only for RequestPaths

src/main/java/org/scion/jpan/ScionService.java

@@ -306,6 +306,7 @@ List<Daemon.Path> getPathListDaemon(long srcIsdAs, long dstIsdAs) {
    * @return All paths returned by the path service.
    * @throws IOException if an errors occurs while querying paths.
    */
+  @Deprecated // Please use lookup() instead
   public List<RequestPath> getPaths(InetSocketAddress dstAddress) throws IOException {
     // Use getHostString() to avoid DNS reverse lookup.
     ScionAddress sa = getScionAddress(dstAddress.getHostString());
@@ -320,7 +321,6 @@ public List<RequestPath> getPaths(InetSocketAddress dstAddress) throws IOExcepti
    * @return All paths returned by the path service.
    */
   public List<RequestPath> getPaths(long dstIsdAs, InetSocketAddress dstScionAddress) {
-    // TODO change method API name to make clear that this requires a SCION IP.
     return getPaths(dstIsdAs, dstScionAddress.getAddress(), dstScionAddress.getPort());
   }
 
@@ -338,19 +338,44 @@ public List<RequestPath> getPaths(RequestPath path) {
    * Request paths from the local ISD/AS to the destination.
    *
    * @param dstIsdAs Destination ISD/AS
-   * @param dstAddress Destination IP address
+   * @param dstAddress A SCION-enabled Destination IP address
    * @param dstPort Destination port
-   * @return All paths returned by the path service.
+   * @return All paths returned by the path service. Returns an empty list if no paths are found.
    */
   public List<RequestPath> getPaths(long dstIsdAs, InetAddress dstAddress, int dstPort) {
-    long srcIsdAs = getLocalIsdAs();
-    List<Daemon.Path> paths = getPathList(srcIsdAs, dstIsdAs);
-    if (paths.isEmpty()) {
-      return Collections.emptyList();
+    return getPaths(ScionAddress.create(dstIsdAs, dstAddress), dstPort);
+  }
+
+  /**
+   * Resolves the address to a SCION address, request paths, and selects a path using the policy.
+   *
+   * @param dstAddr Destination address
+   * @param policy path policy
+   * @return All paths returned by the path service.
+   * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address.
+   */
+  public RequestPath lookupAndGetPath(InetSocketAddress dstAddr, PathPolicy policy)
+      throws ScionException {
+    if (policy == null) {
+      policy = PathPolicy.DEFAULT;
     }
+    return policy.filter(getPaths(lookupAddress(dstAddr.getHostString()), dstAddr.getPort()));
+  }
+
+  /**
+   * Request paths from the local ISD/AS to the destination.
+   *
+   * @param dstAddress Destination SCION address
+   * @return All paths returned by the path service.
+   */
+  public List<RequestPath> getPaths(ScionAddress dstAddress, int dstPort) {
+    long srcIsdAs = getLocalIsdAs();
+    List<Daemon.Path> paths = getPathList(srcIsdAs, dstAddress.getIsdAs());
     List<RequestPath> scionPaths = new ArrayList<>(paths.size());
     for (int i = 0; i < paths.size(); i++) {
-      scionPaths.add(RequestPath.create(paths.get(i), dstIsdAs, dstAddress, dstPort));
+      scionPaths.add(
+          RequestPath.create(
+              paths.get(i), dstAddress.getIsdAs(), dstAddress.getInetAddress(), dstPort));
     }
     return scionPaths;
   }
@@ -416,12 +441,19 @@ public long getIsdAs(String hostName) throws ScionException {
     throw new ScionException("No DNS TXT entry \"scion\" found for host: " + hostName);
   }
 
+  @Deprecated // Please use lookupScionAddress() instead.
+  public ScionAddress getScionAddress(String hostName) throws ScionException {
+    return lookupAddress(hostName);
+  }
+
   /**
+   * Uses DNS and hostfiles to look up a SCION enabled IP address for a give host string.
+   *
    * @param hostName hostName of the host to resolve
    * @return A ScionAddress
    * @throws ScionException if the DNS/TXT lookup did not return a (valid) SCION address.
    */
-  public ScionAddress getScionAddress(String hostName) throws ScionException {
+  private ScionAddress lookupAddress(String hostName) throws ScionException {
     ScionAddress scionAddress = scionAddressCache.get(hostName);
     if (scionAddress != null) {
       return scionAddress;

src/main/java/org/scion/jpan/ScmpChannel.java

@@ -400,7 +400,7 @@ public void close() throws IOException {
    * @see ScionDatagramChannel#getConnectionPath()
    */
   public RequestPath getConnectionPath() {
-    return (RequestPath) channel.getConnectionPath();
+    return channel.getConnectionPath();
   }
 
   public InetSocketAddress getLocalAddress() throws IOException {

src/test/java/org/scion/jpan/testutil/PingPongHelperBase.java

@@ -107,11 +107,11 @@ void runPingPong(ServerFactory serverFactory, ClientFactory clientFactory, boole
       }
       InetSocketAddress serverAddress = servers[0].getLocalAddress();
       MockDNS.install(MockNetwork.TINY_SRV_ISD_AS, serverAddress.getAddress());
-      RequestPath scionAddress = Scion.defaultService().getPaths(serverAddress).get(0);
+      RequestPath requestPath = Scion.defaultService().lookupAndGetPath(serverAddress, null);
 
       Thread[] clients = new Thread[nClients];
       for (int i = 0; i < clients.length; i++) {
-        clients[i] = clientFactory.create(i, scionAddress, nRounds);
+        clients[i] = clientFactory.create(i, requestPath, nRounds);
         clients[i].setName("Client-thread-" + i);
         clients[i].start();
       }