[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [PATCH] Request for adding internal meta-data support to KIO...
From:       Dawit A <adawit () kde ! org>
Date:       2010-05-06 16:14:32
Message-ID: 201005061214.32192.adawit () kde ! org
[Download RAW message or body]

On Thursday, May 06, 2010 04:31:30 David Faure wrote:
> > #3. When internal metadata is received, update ALL ioslaves, not only the
> > idles ones, that match the criteria specified through the keyword.
> 
> I guess there's no other solution, but this sounds a bit like a race. 
> A running slave might or might not get this new metadata in time for
> whatever it's currently doing. Then again, we're talking about
> authentication, so obviously we can't send auth data before it's typed in
> by the user :-) OK, ignore this.

Well, even for circumstances other than authentication, there is nothing we 
can really do about an ioslave that is already in the middle of processing a 
request. We just want to ensure that it gets updated for the next request, 
that includes the ioslave that originally sent the internal meta-data.

For authentication this is actually not an issue because not only do you have 
to wait for user input as you stated, but also subsequent requests into the 
protected site are completely dependent about the first request. IOW, if you do 
not get the first page, well you cannot parse it and make other requests.

> [...]
>      ProtoQueue *pq = m_protocols.value(jobPriv->m_protocol);
>      pq->removeJob(job);
>      if (slave) {
> +        // Reconfigure the slave that sent the original internal meta-data
> and +        // all other idle slaves that are currently servi
> 
> Unfinished sentence?

Fixed...

> +        if (jobPriv->m_internalMetaData.count()) {
> 
> We generally use !isEmpty() rather than count(), when the count isn't
> needed.
> 
> +            kDebug(7006) << "Updating ioslaves with new internal metadata
> information"; +           
> m_protocols.value(slave->protocol())->updateSlaveConfigFor(slave->host());
> 
> Can this value() call ever return 0?

Indeed... it is a possibility. Fixed...

Updated patch attached...

["kio_internal_metadata.patch" (text/x-patch)]

Index: kio/kio/job.cpp
===================================================================
--- kio/kio/job.cpp	(revision 1123389)
+++ kio/kio/job.cpp	(working copy)
@@ -588,7 +588,15 @@
 void SimpleJob::slotMetaData( const KIO::MetaData &_metaData )
 {
     Q_D(SimpleJob);
-    d->m_incomingMetaData += _metaData;
+
+    QMapIterator<QString,QString> it (_metaData);
+    while (it.hasNext()) {
+        it.next();
+        if (it.key().startsWith(QLatin1String("{internal~"), Qt::CaseInsensitive))
+            d->m_internalMetaData.insert(it.key(), it.value());
+        else
+            d->m_incomingMetaData.insert(it.key(), it.value());
+    }
 }
 
 void SimpleJob::storeSSLSessionFromJob(const KUrl &redirectionURL)
Index: kio/kio/job_p.h
===================================================================
--- kio/kio/job_p.h	(revision 1123389)
+++ kio/kio/job_p.h	(working copy)
@@ -55,6 +55,7 @@
         Job* m_parentJob;
         int m_extraFlags;
         MetaData m_incomingMetaData;
+        MetaData m_internalMetaData;
         MetaData m_outgoingMetaData;
 
         inline KIO::JobUiDelegate *ui() const
Index: kio/kio/scheduler.cpp
===================================================================
--- kio/kio/scheduler.cpp	(revision 1123389)
+++ kio/kio/scheduler.cpp	(working copy)
@@ -133,7 +133,6 @@
     return m_idleSlaves.values();
 }
 
-
 void SlaveKeeper::scheduleGrimReaper()
 {
     if (!m_grimTimer.isActive()) {
@@ -570,7 +569,18 @@
     return ret;
 }
 
+void ProtoQueue::updateSlaveConfigFor(const QString& host)
+{
+    Q_FOREACH(Slave *slave, allSlaves()) {
+        if (slave->host() == host) {
+            slave->setConfig(SlaveConfig::self()->configData(slave->protocol(), host));
+            kDebug(7006) << "Updating configuration of" << slave->protocol()
+                         << "ioslave, pid=" << slave->slave_pid();
+        }
+    }
+}
 
+
 //private slot
 void ProtoQueue::startAJob()
 {
@@ -948,17 +958,42 @@
     }
 
     KIO::SimpleJobPrivate *const jobPriv = SimpleJobPrivate::get(job);
+
+    // Preserve all internal meta-data so they can be sent back to the
+    // ioslaves as needed...
+    const KUrl jobUrl = job->url();
+    QMapIterator<QString, QString> it (jobPriv->m_internalMetaData);
+    while (it.hasNext()) {
+        it.next();        
+        if (it.key().startsWith(QLatin1String("{internal~currenthost}"), Qt::CaseInsensitive)) {
+            SlaveConfig::self()->setConfigData(jobUrl.protocol(), jobUrl.host(), it.key().mid(22), it.value());
+        } else if (it.key().startsWith(QLatin1String("{internal~allhosts}"), Qt::CaseInsensitive)) {
+            SlaveConfig::self()->setConfigData(jobUrl.protocol(), QString(), it.key().mid(19), it.value());
+        }
+    }
+
     // make sure that we knew about the job!
     Q_ASSERT(jobPriv->m_schedSerial);
 
     ProtoQueue *pq = m_protocols.value(jobPriv->m_protocol);
     pq->removeJob(job);
     if (slave) {
+        // If we have internal meta-data, tell existing ioslaves to reload
+        // their configuration.
+        if (jobPriv->m_internalMetaData.count()) {
+            kDebug(7006) << "Updating ioslaves with new internal metadata information";
+            ProtoQueue * queue = m_protocols.value(slave->protocol());
+            if (queue)
+                queue->updateSlaveConfigFor(slave->host());
+        }
         slave->setJob(0);
         slave->disconnect(job);
     }
     jobPriv->m_schedSerial = 0; // this marks the job as unscheduled again
     jobPriv->m_slave = 0;
+    // Clear the values in the internal metadata container since they have
+    // already been taken care of above...
+    jobPriv->m_internalMetaData.clear();
 }
 
 // static
@@ -1117,7 +1152,6 @@
 
 Slave *SchedulerPrivate::getConnectedSlave(const KUrl &url, const KIO::MetaData &config)
 {
-    kDebug(7006);
     QString proxy;
     QString protocol = KProtocolManager::slaveProtocol(url, proxy);
     ProtoQueue *pq = protoQ(protocol);
Index: kio/kio/scheduler_p.h
===================================================================
--- kio/kio/scheduler_p.h	(revision 1123389)
+++ kio/kio/scheduler_p.h	(working copy)
@@ -146,7 +146,8 @@
     KIO::Slave *createSlave(const QString &protocol, KIO::SimpleJob *job, const KUrl &url);
     bool removeSlave (KIO::Slave *slave);
     QList<KIO::Slave *> allSlaves() const;
-    ConnectedSlaveQueue m_connectedSlaveQueue;
+    void updateSlaveConfigFor(const QString&);
+    ConnectedSlaveQueue m_connectedSlaveQueue;    
 
 private slots:
     // start max one (non-connected) job and return
Index: kio/DESIGN.metadata
===================================================================
--- kio/DESIGN.metadata	(revision 1123389)
+++ kio/DESIGN.metadata	(working copy)
@@ -5,7 +5,22 @@
 the behavior of a slave and is usally protocol dependent. MetaData consists
 of two strings: a "key" and a "value".
 
+Any meta data whose "key" starts with the keywords {internal~currenthost} and
+"{internal~allhosts}" will be treated as internal metadata and will not be made
+available to client applications. Instead all such meta-data will be stored and
+will appropriately be sent back to all ioslaves along with the other regular
+metadata values.
 
+Use "{internal~currenthost}" to make the internal metadata available to all
+ioslaves of the same protocol and host as the ioslave that generated it. If
+you do not want to restrict the availability of the internal metadata to only
+the current host, then use {internal~allhosts}. In either case the internal
+metadata follows the rules of the regular metadata and therefore cannot be sent
+from one protocol such as "http" to a completely different one like "ftp".
+
+Please note that when internal meta-data values are sent back to ioslaves, the
+keyword used to mark them internal will be stripped from the key name.
+
 The following keys are currently in use:
 
 Key             Value(s)        Description


[prev in list] [next in list] [prev in thread] [next in thread]