qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@apache.org
Subject [15/38] qpid-proton git commit: PROTON-881: tidy up comments and TODO's
Date Sun, 05 Jul 2015 23:45:16 GMT
PROTON-881: tidy up comments and TODO's

Remove TODO's if they were already done, downgrade some TODO's to comments
(if they were simply observations), and remove some comments that consisted
of proton-c code - pasted in as a reference.


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/46b9d848
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/46b9d848
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/46b9d848

Branch: refs/heads/master
Commit: 46b9d848999e993f238b37c9a0598035ccd64b27
Parents: d6c4ba7
Author: Adrian Preston <prestona@uk.ibm.com>
Authored: Thu May 7 01:21:41 2015 +0100
Committer: Adrian Preston <prestona@uk.ibm.com>
Committed: Thu May 7 01:21:41 2015 +0100

----------------------------------------------------------------------
 .../org/apache/qpid/proton/reactor/Reactor.java | 13 ++--
 .../apache/qpid/proton/reactor/Selectable.java  |  3 +-
 .../qpid/proton/reactor/impl/AcceptorImpl.java  |  2 +-
 .../qpid/proton/reactor/impl/IOHandler.java     | 14 ++--
 .../qpid/proton/reactor/impl/ReactorImpl.java   | 81 ++------------------
 .../reactor/impl/ReactorInternalException.java  | 21 +++++
 .../qpid/proton/reactor/impl/SelectorImpl.java  |  4 +-
 7 files changed, 43 insertions(+), 95 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java
index e658057..f91f376 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Reactor.java
@@ -56,14 +56,11 @@ public interface Reactor {
 
     public void setHandler(Handler handler);
 
-/* TODO
- * pn_io_t *pn_reactor_io(pn_reactor_t *reactor) {
-166   assert(reactor);
-167   return reactor->io;
-168 }
-169
-
- */
+    // XXX: The C reactor has a pn_reactor_io() function.  The closest Java equivalent
+    //      would be a factory for creating SocketChannel's, ServerSocketChannelsm and Selectors.
+    //      This seems like overkill unless there's a use for this in unit testing, or the
+    //      Reactor needs to be integrated with an exotic Java environment which provides
its
+    //      own networking implementation.
 
     public Set<ReactorChild> children();
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/Selectable.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Selectable.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Selectable.java
index 1bdaa1e..e839b10 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/Selectable.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/Selectable.java
@@ -67,9 +67,10 @@ public interface Selectable extends ReactorChild {
 
     void release() ;
 
+    @Override
     void free() ;
 
-    // These are equivalent to the C code's set/get file descritor functions.
+    // These are equivalent to the C code's set/get file descriptor functions.
     void setChannel(SelectableChannel channel) ;
 
     public SelectableChannel getChannel() ;

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java
b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java
index 38d5416..243a963 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/AcceptorImpl.java
@@ -97,7 +97,7 @@ public class AcceptorImpl implements Acceptor {
         sel = ((ReactorImpl)reactor).selectable(this);
         sel.setChannel(ssc);
         sel.onReadable(new AcceptorReadable());
-        sel.onFree(new AcceptorFree()); // TODO: currently, this is not called from anywhere!!
+        sel.onFree(new AcceptorFree());
         sel.setReactor(reactor);
         sel.setAttachment(handler);
         sel.setReading(true);

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java
index 6199c56..872b6ff 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/IOHandler.java
@@ -99,7 +99,7 @@ public class IOHandler extends BaseHandler {
         }
 
         Transport transport = event.getConnection().getTransport();
-        Socket socket = null;   // TODO: null is our equivalent of PN_INVALID_SOCKET
+        Socket socket = null;   // In this case, 'null' is the proton-j equivalent of PN_INVALID_SOCKET
         try {
             SocketChannel socketChannel = SocketChannel.open();
             socketChannel.connect(new InetSocketAddress(hostname, port));
@@ -111,7 +111,7 @@ public class IOHandler extends BaseHandler {
             transport.setCondition(condition);
             transport.close_tail();
             transport.close_head();
-            transport.pop(transport.pending());   // TODO: force generation of TRANSPORT_HEAD_CLOSE
(not in C code)
+            transport.pop(transport.pending());   // Force generation of TRANSPORT_HEAD_CLOSE
(not in C code)
         }
         selectableTransport(reactor, socket, transport);
     }
@@ -183,9 +183,9 @@ public class IOHandler extends BaseHandler {
                     transport.close_tail();
                 }
             }
-            // TODO: comment from C code...
-            // occasionally transport events aren't generated when expected, so
-            // the following hack ensures we always update the selector
+            // (Comment from C code:) occasionally transport events aren't
+            // generated when expected, so the following hack ensures we
+            // always update the selector
             update(selectable);
             reactor.update(selectable);
         }
@@ -264,8 +264,8 @@ public class IOHandler extends BaseHandler {
     }
 
     // pn_reactor_selectable_transport
+    // Note the socket argument can, validly be 'null' this is the equivalent of proton-c's
PN_INVALID_SOCKET
     protected static Selectable selectableTransport(Reactor reactor, Socket socket, Transport
transport) {
-        // TODO: this code needs to be able to deal with a null socket (this is our equivalent
of PN_INVALID_SOCKET)
         Selectable selectable = reactor.selectable();
         selectable.setChannel(socket != null ? socket.getChannel() : null);
         selectable.onReadable(new ConnectionReadable());    // TODO: *IF* these callbacks
are stateless, do we more than one instance of them?
@@ -296,7 +296,7 @@ public class IOHandler extends BaseHandler {
             ReactorImpl reactor = (ReactorImpl)event.getReactor();
             Selector selector = reactor.getSelector();
             if (selector == null) {
-                selector = new SelectorImpl();     // TODO: the C code supplies the reactor's
pn_io object here...
+                selector = new SelectorImpl();
                 reactor.setSelector(selector);
             }
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java
index cde5f42..c2c30ef 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java
@@ -47,23 +47,6 @@ import org.apache.qpid.proton.reactor.Task;
 
 public class ReactorImpl implements Reactor {
 
-    /*
-     *   pn_record_t *attachments;
- 41   pn_io_t *io;
- 42   pn_collector_t *collector;
- 43   pn_handler_t *global;
- 44   pn_handler_t *handler;
- 45   pn_list_t *children;
- 46   pn_timer_t *timer;
- 47   pn_socket_t wakeup[2];
- 48   pn_selectable_t *selectable;
- 49   pn_event_type_t previous;
- 50   pn_timestamp_t now;
- 51   int selectables;
- 52   int timeout;
- 53   bool yield;
-     */
-
     private Object attachment;
     private CollectorImpl collector;
     private long now;
@@ -77,6 +60,7 @@ public class ReactorImpl implements Reactor {
     private Type previous;
     private Timer timer;
     private final Pipe wakeup;
+    private Selector selector;
 
     @Override
     public long mark() {
@@ -88,25 +72,7 @@ public class ReactorImpl implements Reactor {
     public long now() {
         return now;
     }
-/*
- * tatic void pn_reactor_initialize(pn_reactor_t *reactor) {
- 68   reactor->attachments = pn_record();
- 69   reactor->io = pn_io();    TODO: pn_io most literally translates to SocketFactory
(and possibly also ServerSocketFactory...)
- 70   reactor->collector = pn_collector();
- 71   reactor->global = pn_iohandler();
- 72   reactor->handler = pn_handler(NULL);
- 73   reactor->children = pn_list(PN_OBJECT, 0);
- 74   reactor->timer = pn_timer(reactor->collector);
- 75   reactor->wakeup[0] = PN_INVALID_SOCKET;
- 76   reactor->wakeup[1] = PN_INVALID_SOCKET;
- 77   reactor->selectable = NULL;
- 78   reactor->previous = PN_EVENT_NONE;
- 79   reactor->selectables = 0;
- 80   reactor->timeout = 0;
- 81   reactor->yield = false;
- 82   pn_reactor_mark(reactor);
- 83 }
- 84  */
+
     public ReactorImpl() throws IOException {
         collector = (CollectorImpl)Proton.collector();
         global = new IOHandler();
@@ -184,15 +150,6 @@ public class ReactorImpl implements Reactor {
         this.handler = handler;
     }
 
-/* TODO
- * pn_io_t *pn_reactor_io(pn_reactor_t *reactor) {
-166   assert(reactor);
-167   return reactor->io;
-168 }
-169
-
- */
-
     @Override
     public Set<ReactorChild> children() {
         return children;
@@ -248,13 +205,7 @@ public class ReactorImpl implements Reactor {
         }
     }
 
-    // TODO: pn_record_get_handler
-    // TODO: pn_record_set_handler
-    // TODO: pn_class_reactor
-    // TODO: pn_object_reactor
-    // TODO: pn_event_reactor
-
-    // pn_event_handler - TODO: this is copied from the Reactor.java code, so might need
some tweaks...
+    // pn_event_handler
     private Handler eventHandler(Event event) {
         Handler result;
         if (event.getLink() != null) {
@@ -269,10 +220,6 @@ public class ReactorImpl implements Reactor {
             result = ((HandlerEndpointImpl)event.getConnection()).getHandler();
             if (result != null) return result;
         }
-//        if (event.getTransport() != null) { // TODO: do we want to allow handlers to be
added to the Transport object?
-//            result = ((EndpointImpl)event.getTransport()).getHandlers();
-//            if (result.hasNext()) return result;
-//        }
 
         if (event.getTask() != null) {
             result = event.getTask().getHandler();
@@ -347,7 +294,6 @@ public class ReactorImpl implements Reactor {
 
     @Override
     public void wakeup() throws IOException {
-        //selector.wakeup();
         wakeup.sink().write(ByteBuffer.allocate(1));    // TODO: c version returns a value!
     }
 
@@ -355,7 +301,6 @@ public class ReactorImpl implements Reactor {
     public void start() {
         collector.put(Type.REACTOR_INIT, this);
         selectable = timerSelectable();
-        //selectable.setDeadline(now + timeout);      // TODO: this isn't in the C code...
     }
 
     @Override
@@ -372,7 +317,7 @@ public class ReactorImpl implements Reactor {
 
     @Override
     public void run() {
-        setTimeout(3141);   // TODO: eh?
+        setTimeout(3141);
         start();
         while(process()) {}
         stop();
@@ -390,24 +335,11 @@ public class ReactorImpl implements Reactor {
         }
         return task;
     }
-    // TODO: acceptor
-    // TODO: connection
-    // TODO: acceptorClose
 
     private class TimerReadable implements Callback {
 
         @Override
         public void run(Selectable selectable) {
-            // TODO: the implication is that this will be called when the selectable is woken-up
-/*
-  434 static void pni_timer_readable(pn_selectable_t *sel) {
-  435   char buf[64];
-  436   pn_reactor_t *reactor = pni_reactor(sel);
-  437   pn_socket_t fd = pn_selectable_get_fd(sel);
-  438   pn_read(reactor->io, fd, buf, 64);
-  439   pni_timer_expired(sel);
-  440 }
- */
             // TODO: this could be more elegant...
             new TimerExpired().run(selectable);
         }
@@ -433,7 +365,7 @@ public class ReactorImpl implements Reactor {
                 selectable.getChannel().close();
             } catch(IOException e) {
                 e.printStackTrace();
-                // TODO: no idea what to do here...
+                // TODO: what to do here...
             }
         }
     }
@@ -450,9 +382,6 @@ public class ReactorImpl implements Reactor {
         return sel;
     }
 
-    // TODO: the C code allows records to be associated with a Reactor and the Selector is
get/set using that capability.
-    private Selector selector;
-
     protected Selector getSelector() {
         return selector;
     }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorInternalException.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorInternalException.java
b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorInternalException.java
index d8d67ad..6dde424 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorInternalException.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorInternalException.java
@@ -1,3 +1,24 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.qpid.proton.reactor.impl;
 
 /**

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/46b9d848/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/SelectorImpl.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/SelectorImpl.java
b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/SelectorImpl.java
index cf50456..28ea1ae 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/SelectorImpl.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/SelectorImpl.java
@@ -40,12 +40,12 @@ public class SelectorImpl implements Selector {
     private final HashSet<Selectable> error = new HashSet<Selectable>();
 
     public SelectorImpl() throws IOException {
-        selector = java.nio.channels.Selector.open();   // TODO: need to ensure we close
this somewhere...
+        selector = java.nio.channels.Selector.open();
     }
 
     @Override
     public void add(Selectable selectable) throws IOException {
-        // TODO: valid for selectable to have a 'null' channel - in this case it can only
expire...
+        // Selectable can be 'null' - if this is the case it can only ever receive expiry
events.
         if (selectable.getChannel() != null) {
             selectable.getChannel().configureBlocking(false);
             SelectionKey key = selectable.getChannel().register(selector, 0);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org


Mime
View raw message