From e0d0a59706f7e4b49106f810759e30650f9b4fa4 Mon Sep 17 00:00:00 2001
From: Mikael Nordfeldth <mmn@hethane.se>
Date: Thu, 27 Nov 2014 14:27:41 +0100
Subject: [PATCH] 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.
---
 classes/Conversation.php |  4 ++--
 classes/Notice.php       | 40 +++++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/classes/Conversation.php b/classes/Conversation.php
index fde9c07adb..67ac89cf1c 100644
--- 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 0074a37bdf..96637b8f32 100644
--- 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.