[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-03 23:08:35
Message-ID: 201005031908.35770.adawit () kde ! org
[Download RAW message or body]

Attached is the same patch with the following changes:

#1. Use descriptive names for the scope portion of the keyword. 0 was replaced 
with "currenthost" and 1 with "allhosts".

#2. Removed the "kio~" portion of the keyword as that is already implied. 
After all this is still the KIO metadata system ; so now the full keyword used 
to mark a metadata as internal is either {internal~currenthost} or 
{internal~allhosts}

#3. When internal metadata is received, update ALL ioslaves, not only the 
idles ones, that match the criteria specified through the keyword.


On Monday, May 03, 2010 09:31:58 David Faure wrote:
> On Friday 30 April 2010, Rolf Eike Beer wrote:
> > Dawit A wrote:
> > > On Thursday, April 29, 2010 02:14:45 Rolf Eike Beer wrote:
> > > > My point is less a thing that this (or any other) slave will do or
> > > > not with this data now or in one year. My point is that when this is
> > > > some internal state of whatever kind than we should under no way
> > > > allow remote systems to influence that in any way but through our
> > > > code that does it explicitely. It's the same like you make class
> > > > members private instead of making them public and hoping that noone
> > > > will touch them if you name them
> > > > _private_member_foo.
> > > 
> > > Again there is no way for a remote system to influence the meta-data
> > > system, internal or otherwise, right now unless the developer of an
> > > ioslave implicitly allows it to do so. As such I fail to see how you
> > > can protect against that by using special characters in the keyword.
> > > If the developer is going to expose it to influence by the remote
> > > system, what stops him from simply adding the keyword directly ? To me
> > > it is simply a pointless exercise since you cannot control what a
> > > developer will do in the end. What you are saying would make a great
> > > deal of sense if we automatically mapped meta-data sent by a remote
> > > system directly into our internal scheme, but we do not do that.
> > 
> > Ok, so if there is no way to put "global" entries in the metadata
> > everything is fine. The HTTP ioslave e.g. puts everything in the
> > Content-Disposition header as Content-Disposition-foo in the metadata. If
> > noone inserts un-prefixed wildcard data in the metadata we're save.
> 
> Yes. Don't confuse metadata with HTTP headers, even if in some cases
> there's some overlap.
> The HTTP headers sent by the webservers are NOT put into the metadata as
> one entry per header.
> 
> 
> About the naming: I think 0 and 1 should be replaced with descriptive
> names.

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

Index: kio/kio/job.cpp
===================================================================
--- kio/kio/job.cpp	(revision 1122431)
+++ 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 1122431)
+++ 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 1122431)
+++ 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,40 @@
     }
 
     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) {
+        // Reconfigure the slave that sent the original internal meta-data and
+        // all other idle slaves that are currently servi
+        if (jobPriv->m_internalMetaData.count()) {
+            kDebug(7006) << "Updating ioslaves with new internal metadata \
information"; +            \
m_protocols.value(slave->protocol())->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 +1150,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 1122431)
+++ 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 1122431)
+++ 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]