Interface update, TODO

I would be surprised if this compiled...

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac408@3580 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/nsas/TODO

@@ -0,0 +1,24 @@
+* Make it aware of two address kinds, execute the callbacks if there is at
+  last one kind ready.
+
+Nameserver entries:
+* Upgrade the nameserver entries to own the callbacks (they will hold shared
+  pointers to zones). This will ensure the zone is not deleted at wrong time.
+* There's only one function for adding callbacks, put them together.
+* Return if we timed out.
+
+Zone entries:
+* They will be nameserver callbacks themself. This ensures that they are not
+  destroyed, since the callback is owned by the nameserver and will easy up
+  the accessing of internal data structures. Maybe a private inheritance?
+* Add ability to time out
+* Implement the new interface
+* They will create new nameserver entries in constructor.
+* Move the logic here from NSAS
+* If there are callbacks for this kind of address, do not look into the
+  nameserver entries, just add it.
+* Even if the TTL is 0, we should accept the first callback.
+
+The NSAS itself:
+* If the zone rejects the callback, remove it and call recursively the same
+  one. As it will accept at last one, it should not become a loop.

src/lib/nsas/nameserver_address_store.h

@@ -28,6 +28,7 @@
 #include "lru_list.h"
 #include "zone_entry.h"
 #include "resolver_interface.h"
+#include "nsas_types.h"
 
 namespace isc {
 namespace nsas {
@@ -103,10 +104,12 @@ public:
     /// are taken from the referral.
     /// \param callback Callback object used to pass the result back to the
     /// caller.
+    /// \param request Which address is requested.
     void lookup(const std::string& zone, uint16_t class_code,
         const isc::dns::AbstractRRset& authority,
         const std::vector<isc::dns::AbstractRRset>& additional,
-        boost::shared_ptr<AddressRequestCallback> callback);
+        boost::shared_ptr<AddressRequestCallback> callback, AddressRequest
+        request = ANY_OK);
 
     /// \brief Protected Members
     ///
@@ -130,30 +133,6 @@ protected:
     //}@
 private:
     ResolverInterface& resolver_;
-    /**
-     * \short Find if any callbacks may be called.
-     *
-     * This is called when new callback or new data arrive to a zone. In both
-     * cases this may trigger executing some of the callbacks or additional
-     * lookups.
-     * \param zone Which zone to process
-     * \todo Should this be part of the zone entry possibly?
-     * \todo Pass some of the referral stuff there?
-     */
-    void processZone(boost::shared_ptr<ZoneEntry> zone);
-    /// \short Callback from nameserver entry to process zone.
-    class Callback : public NameserverEntry::Callback {
-        public:
-            Callback(NameserverAddressStore& store) :
-                store_(store)
-            { }
-            virtual void operator()(boost::shared_ptr<ZoneEntry> zone) {
-                store_.processZone(zone);
-            }
-        private:
-            NameserverAddressStore& store_;
-    } callback_;
-    friend class Callback;
 };
 
 } // namespace nsas

src/lib/nsas/nameserver_entry.h

@@ -27,6 +27,7 @@
 #include "address_entry.h"
 #include "asiolink.h"
 #include "nsas_entry.h"
+#include "nsas_types.h"
 #include "hash_key.h"
 #include "lru_list.h"
 #include "fetchable.h"
@@ -80,6 +81,7 @@ class ResolverInterface;
 ///
 /// As this object will be stored in the nameserver address store LRU list,
 /// it is derived from the LRU list entry class.
+/// \todo Is it needed to have virtual functions? Will we subclass it?
 
 class NameserverEntry : public NsasEntry<NameserverEntry>, public Fetchable {
 public:
@@ -117,18 +119,20 @@ public:
     /// \brief Return Address
     ///
     /// Returns a vector of addresses corresponding to this nameserver.
-    /// It is up to the caller to 
     ///
     /// \param addresses Vector of address entries into which will be appended
     /// addresses that match the specified criteria. (The reason for choosing
     /// this signature is that addresses from more than one nameserver may be
     /// retrieved, in which case appending to an existing list of addresses is
     /// convenient.)
-    /// \param family Set to AF_INET/AF_INET6 for V6/V6 addresses, anything
+    /// \param family The family of address that is requested.
     /// else for all addresses.
-    virtual void getAddresses(NameserverEntry::AddressVector& addresses,
-        short family = 0) const;
+    /// \return The state this is currently in. If the TTL expires, it enters
+    ///     the NOT_ASKED state by itself.
+    virtual Fetchable::State getAddresses(
+        NameserverEntry::AddressVector& addresses, AddressRequest family);
 
+    // TODO Is this one of any use at all?
     /// \brief Return Address that corresponding to the index
     ///
     /// \param index The address index in the address vector
@@ -201,7 +205,7 @@ public:
     //@{
     /// \short A callback that some information here arrived (or are unavailable).
     struct Callback {
-        virtual void operator()(boost::shared_ptr<ZoneEntry>) = 0;
+        virtual void operator()(NameserverEntry* self) = 0;
     };
 
     /**
@@ -210,38 +214,25 @@ public:
      * Adds a callback for given zone when they are ready or the information
      * is found unreachable.
      *
-     * This does not lock and expects that the entry is already locked.
+     * If it is not in NOT_ASKED state, it does not ask the for the IP address
+     * again, it just inserts the callback. It is up to the caller not to
+     * insert one callback multiple times.
      *
-     * Expects that the nameserver entry is in NOT_ASKED state,
-     * throws BadValue otherwise.
+     * The callback might be called directly from this function.
      *
      * \param resolver Who to ask.
-     * \param zone The callbacks are named, so we can check if we already have
-     *     a callback for given zone. This is the name and the zone will be
-     *     passed to the callback when called.
      * \param callback The callback.
+     * \param family Which addresses are interesting to the caller. This does
+     *     not change which adresses are requested, but the callback might
+     *     be executed when at last one requested type is available (eg. not
+     *     waiting for the other one).
      * \param self Since we need to pass a shared pointer to the resolver, we
      *     need to get one. However, we can not create one from this, because
      *     it would have different reference count. So the caller must pass it.
      */
-    void askIP(ResolverInterface& resolver, boost::shared_ptr<ZoneEntry> zone,
-        Callback& callback, boost::shared_ptr<NameserverEntry> self);
-    /**
-     * \short Ensures that zone has a callback registered.
-     *
-     * This adds a given callback to this nameserver entry, but only if
-     * the zone does not have one already.
-     *
-     * Does not lock and expects that the entry is already locked.
-     *
-     * Expects that the nameserver entri is in IN_PROGRESS state, throws
-     * BadValue otherwise.
-     *
-     * \param zone Whose callback we add.
-     * \param callback The callback.
-     */
-    void ensureHasCallback(boost::shared_ptr<ZoneEntry> zone,
-        Callback& callback);
+    void askIP(ResolverInterface& resolver,
+        boost::shared_ptr<Callback> callback, AddressRequest family,
+        boost::shared_ptr<NameserverEntry> self);
     //@}
 
 private:
@@ -252,10 +243,6 @@ private:
     std::vector<AddressEntry> address_; ///< Set of V4/V6 addresses
     time_t          expiration_;        ///< Summary expiration time
     time_t          last_access_;       ///< Last access time to the structure
-    // We allow ZoneEntry to lock us
-    friend class ZoneEntry;
-    // We store the callbacks of zones asking for addresses here
-    std::map<boost::shared_ptr<ZoneEntry>, Callback*> ipCallbacks_;
     // This is our callback class to resolver
     class ResolverCallback;
     friend class ResolverCallback;

src/lib/nsas/nsas_types.h

@@ -23,10 +23,25 @@
 /// \file nsas_types.h
 /// \brief Nameserver Address Store Types
 ///
-/// Defines a set of typedefs used within the Network Address Store.
+/// Defines a set of types used within the Network Address Store.
+
+namespace isc {
+namespace nsas {
 
 /// \brief Array of nameserver addresses
 typedef std::vector<ip::address>    NsasAddress
 
+/// \brief Address requested
+enum AddressRequest {
+    /// \short Interested only in IPv4 address
+    V4_ONLY,
+    /// \short Interested only in IPv6 address
+    V6_ONLY,
+    /// \short Any address is good
+    ANY_OK
+};
+
+}
+}
 
 #endif // __NSAS_TYPES_H

src/lib/nsas/zone_entry.h

@@ -57,6 +57,8 @@ public:
         name_(name), classCode_(class_code)
     {}
 
+    // TODO Constructor from namesarver table and referral information
+
     /// \brief Constructor
     ///
     /// Creates a zone entry object with an RRset representing the nameservers,
@@ -83,88 +85,27 @@ public:
         return HashKey(name_, classCode_);
     }
 
-    // TODO The callbacks must be distinguished - A, AAAA or any of them
-
-    /// \short Add another callback here
-    void addCallback(boost::shared_ptr<AddressRequestCallback> callback);
-    /// \short Is there at last one callback waiting?
-    bool hasCallbacks() const;
-    /// \short Remove a callback from queue and return it
-    boost::shared_ptr<AddressRequestCallback> popCallback();
-
-    /// \short Nameserver entry pointer
-    typedef boost::shared_ptr<NameserverEntry> NameserverPtr;
-    /// \short Vector of nameservers
-    typedef std::vector<NameserverPtr> NameserverVector;
-    /**
-     * \name Iterators
-     *
-     * They iterate over the nameservers.
-     */
-    //@{
-    typedef NameserverVector::iterator iterator;
-    typedef NameserverVector::const_iterator const_iterator;
-    //@}
-
-    /**
-     * \short Add a nameserver pointer to this zone.
-     *
-     * This does not lock, as it should be called while it is being created.
-     * No new nameservers should be added later (it should timeout first and
-     * be rebuild). Calling this after addition to the NameserverAddressStore
-     * is undefined (it is not thread safe).
-     */
-    void nameserverAdd(NameserverPtr ns) { nameservers_.push_back(ns); }
     /**
-     * \name Iterator access
+     * \short Put another callback inside.
      *
-     * They work similar to usual stl iterator access functions. They iterate
-     * over the nameservers.
-     *
-     * They do not lock, as the nameservers should be read only during
-     * the life of the zone.
-     */
-    //@{
-    iterator begin() { return (nameservers_.begin()); }
-    iterator end() { return (nameservers_.end()); }
-    const_iterator begin() const { return (nameservers_.begin()); }
-    const_iterator end() const { return (nameservers_.end()); }
-    //@}
-
-    // TODO Get rid of this
-    /**
-     * \short Lock of the zone entry.
-     *
-     * Something like a scope lock for the zone entry. It can be copyed (so
-     * the result of the getLock() can be assigned to a local variable). The
-     * lock is released once all copies of the getLock result are destroyed.
-     * However, it is not reentrant (another call to getLock will block).
-     *
-     * This locks both the zone entry and all nameserver entries in a manner
-     * avoiding deadlocks (sorts the nameserver entry pointers before trying to
-     * lock them). However, it asumes no one does any other kind of locking
-     * of multiple mutices.
-     *
-     * Copy constructor, assignment operator and destructor are default.
-     * The constructor that creates a new lock is private, use getLock()
-     * to lock a zone entry.
-     *
-     * It is an error for the lock to survive destruction of its zone entry.
+     * This callback is either executed right away, if it is possible,
+     * or queued for later.
+     * \param callback The callback itself.
+     * \param v4ok Is it ok to give the callback a IPv4 address?
+     * \param v6ok Is it ok to give the callback a IPv6 address? (At last one
+     *     of them must be true or isc::BadValue is thrown)
+     * \param self A shared pointer to this zone entry. It is not possible to
+     *     create one from C++ this pointer, since another shared pointer
+     *     will already exist at that point, however it is needed to callback.
+     *     When calling function on the zone entry, you should already have
+     *     one.
+     * \return True if the zone is still valid and accepted the callback.
+     *     If it returns false, it should be discarded (it has timed out)
+     *     and new instance should be created.
      */
-    class Lock {
-        private:
-            struct Impl;
-            boost::shared_ptr<Impl> impl_;
-            Lock(boost::shared_ptr<Impl>);
-            friend class ZoneEntry;
-    };
+    bool addCallback(boost::shared_ptr<AddressRequestCallback>
+        callback, bool v4ok, bool v6ok, boost::shared_ptr<ZoneEntry> self);
 
-    /**
-     * \short Acquire a lock.
-     *
-     * \see Lock
-     */
-    Lock getLock();
 private:
     // TODO Read-Write lock?
     mutable boost::mutex    mutex_;     ///< Mutex protecting this zone entry
@@ -173,6 +114,12 @@ private:
     NameserverVector nameservers_; ///< Nameservers
     time_t          expiry_;    ///< Expiry time of this entry
     std::list<boost::shared_ptr<AddressRequestCallback> > callbacks_;
+    // Internal function that adds a callback (if there's one) and processes
+    // the nameservers (if there's chance there's some info) and calls
+    // callbacks. If nameserver is given, it is considered new and valid
+    // even if its TTL is 0.
+    void process(boosh::shared_ptr<AddressRequestCallback> callback,
+         bool v4ok, bool v6ok, NameserverEntry* nameserver);
 };
 
 } // namespace nsas