Move Conversation creation out of insert() to allow URI setting

This means we import the URI string from remote instances to track their
conversations and are able to stitch together replies in a single thread.

We might have to try to avoid collisions so noone remotely can predict
conversation URIs which we generate on our server, causing a DoS kind of
problem.

diff --git a/classes/Conversation.php b/classes/Conversation.php
index fde9c07adb061b5e1ded2a45a759c63e3dc51b9c..67ac89cf1c267efa4dab53912e1cc33746ecf582 100644 (file)
--- a/classes/Conversation.php
+++ b/classes/Conversation.php
@@ -63,7 +63,7 @@ class Conversation extends Managed_DataObject
      *
      * @return Conversation the new conversation DO
      */
-    static function create(Notice $notice)
+    static function create(Notice $notice, $uri=null)
     {
         if (empty($notice->id)) {
             throw new ServerException(_('Tried to create conversation for not yet inserted notice'));
@@ -71,7 +71,7 @@ class Conversation extends Managed_DataObject
         $conv = new Conversation();
         $conv->created = common_sql_now();
         $conv->id = $notice->id;
-        $conv->uri = sprintf('%s%s=%d:%s=%s',
+        $conv->uri = $uri ?: sprintf('%s%s=%d:%s=%s',
                              TagURI::mint(),
                              'noticeId', $notice->id,
                              'objectType', 'thread');

diff --git a/classes/Notice.php b/classes/Notice.php
index 0074a37bdf4c338546c4a04be7569666c92a0390..96637b8f323e8dda17c2423a4d181fbf7173a270 100644 (file)
--- a/classes/Notice.php
+++ b/classes/Notice.php
@@ -607,10 +607,19 @@ class Notice extends Managed_DataObject
             // unknown remote user - within a known conversation.
             if (empty($notice->conversation) and !empty($options['conversation'])) {
                 $conv = Conversation::getKV('uri', $options['conversation']);
-                if ($conv instanceof Conversation and $activity->time > $conv->created) {
-                    common_debug('Conversation stitched together from (probably) reply to unknown remote user. Activity creation time ('.$activity->time.') is greater than conversation creation time ('.$conv->created.').');
+                if ($conv instanceof Conversation) {
+                    common_debug('Conversation stitched together from (probably) reply to unknown remote user. Activity creation time ('.$activity->time.') should maybe be compared to conversation creation time ('.$conv->created.').');
                     $notice->conversation = $conv->id;
+                } else {
+                    // Conversation URI was not found, so we must create it. But we can't create it
+                    // until we have a Notice ID because of the database layout...
+                    $notice->tmp_conv_uri = $options['conversation'];
                 }
+            } else {
+                // If we're not using the attached conversation URI let's remove it
+                // so we don't mistake ourselves later, when creating our own Conversation.
+                // This implies that the notice knows which conversation it belongs to.
+                $options['conversation'] = null;
             }
         }
 
@@ -661,6 +670,15 @@ class Notice extends Managed_DataObject
 
             try {
                 $notice->insert();  // throws exception on failure
+                // If it's not part of a conversation, it's
+                // the beginning of a new conversation.
+                if (empty($notice->conversation)) { 
+                    $orig = clone($notice);
+                    // $act->context->conversation will be null if it was not provided
+                    $conv = Conversation::create($notice, $options['conversation']);
+                    $notice->conversation = $conv->id;
+                    $notice->update($orig);
+                }
             } catch (Exception $e) {
                 // Let's test if we managed initial insert, which would imply
                 // failing on some update-part (check 'insert()'). Delete if
@@ -869,13 +887,21 @@ class Notice extends Managed_DataObject
 
             try {
                 $stored->insert();    // throws exception on error
+                $orig = clone($stored); // for updating later in this try clause
+
+                // If it's not part of a conversation, it's
+                // the beginning of a new conversation.
+                if (empty($stored->conversation)) {
+                    // $act->context->conversation will be null if it was not provided
+                    $conv = Conversation::create($stored, $act->context->conversation);
+                    $stored->conversation = $conv->id;
+                }
 
                 $object = null;
                 Event::handle('StoreActivityObject', array($act, $stored, $options, &$object));
                 if (empty($object)) {
                     throw new ServerException('No object from StoreActivityObject '.$stored->uri . ': '.$act->asString());
                 }
-                $orig = clone($stored);
                 $stored->object_type = ActivityUtils::resolveUri($object->getObjectType(), true);
                 $stored->update($orig);
             } catch (Exception $e) {
@@ -2392,14 +2418,6 @@ class Notice extends Managed_DataObject
             $changed = true;
         }
 
-        // If it's not part of a conversation, it's
-        // the beginning of a new conversation.
-        if (empty($this->conversation)) {
-            $conv = Conversation::create($this);
-            $this->conversation = $conv->id;
-            $changed = true;
-        }
-
         if ($changed && $this->update($orig) === false) {
             common_log_db_error($notice, 'UPDATE', __FILE__);
             // TRANS: Server exception thrown when a notice cannot be updated.