From 6f1d47a6ab2eaf1b760861f93eca91959983e234 Mon Sep 17 00:00:00 2001 From: Deon George Date: Sat, 15 Jan 2022 13:06:15 +1100 Subject: [PATCH] Fixes to message processing, now that we are using cockroachdb --- app/Classes/FTN/Message.php | 24 ++++++++--------- app/Classes/FTN/Packet.php | 33 +++++++++++------------ app/Jobs/MessageProcess.php | 20 ++++++-------- app/Models/Address.php | 29 +++++++++++++------- app/Models/Echomail.php | 2 +- app/Rules/TwoByteInteger.php | 40 ++++++++++++++-------------- app/Rules/TwoByteIntegerWithZero.php | 33 +++++++++++++++++++++++ tests/Feature/PacketTest.php | 20 +++++++------- 8 files changed, 119 insertions(+), 82 deletions(-) create mode 100644 app/Rules/TwoByteIntegerWithZero.php diff --git a/app/Classes/FTN/Message.php b/app/Classes/FTN/Message.php index 5d079dd..e47bea7 100644 --- a/app/Classes/FTN/Message.php +++ b/app/Classes/FTN/Message.php @@ -11,7 +11,7 @@ use Illuminate\Validation\Validator as ValidatorResult; use App\Classes\FTN as FTNBase; use App\Models\{Address,Domain,Zone}; -use App\Rules\TwoByteInteger; +use App\Rules\{TwoByteInteger,TwoByteIntegerWithZero}; use App\Traits\EncodeUTF8; /** @@ -117,7 +117,7 @@ class Message extends FTNBase private Collection $rogue_path; // Collection of FTNs in the Seen-by that are not defined private Collection $seenby; // FTS-0004.001 The message SEEN-BY lines private Collection $seenaddress; // Collection of Addresses after parsing seenby - private Collection $rogue_seen; // Collection of FTNs in the Seen-by that are not defined + private Collection $rogue_seenby; // Collection of FTNs in the Seen-by that are not defined private Collection $via; // The path the message has gone using Via lines (Netmail) private Collection $unknown; // Temporarily hold attributes we have no logic for. @@ -203,7 +203,7 @@ class Message extends FTNBase $this->seenby = collect(); $this->seenaddress = collect(); $this->pathaddress = collect(); - $this->rogue_seen = collect(); + $this->rogue_seenby = collect(); $this->rogue_path = collect(); $this->via = collect(); $this->unknown = collect(); @@ -262,7 +262,7 @@ class Message extends FTNBase $o->unpackMessage(substr($msg,self::HEADER_LEN+$ptr)); if (($x=$o->validate())->fails()) { - Log::debug('Message fails validation',['result'=>$x->errors()]); + Log::debug(sprintf('%s:Message fails validation (%s@%s->%s@%s)',self::LOGKEY,$o->user_from,$o->fftn,$o->user_to,$o->tftn),['result'=>$x->errors()]); //throw new \Exception('Message validation fails:'.join(' ',$x->errors()->all())); } @@ -388,7 +388,7 @@ class Message extends FTNBase case 'pathaddress': case 'seenaddress': case 'rogue_path': - case 'rogue_seen': + case 'rogue_seenby': case 'unknown': case 'via': @@ -842,7 +842,7 @@ class Message extends FTNBase } // Parse SEEN-BY if ($this->seenby->count()) - $this->seenaddress = $this->parseAddresses('seenby',$this->seenby,$this->rogue_seen); + $this->seenaddress = $this->parseAddresses('seenby',$this->seenby,$this->rogue_seenby); // Parse PATH if ($this->path->count()) @@ -861,10 +861,10 @@ class Message extends FTNBase 'user_from' => $this->user_from, 'user_to' => $this->user_to, 'subject' => $this->subject, - 'onode' => $this->fn, - 'dnode' => $this->ff, - 'onet' => $this->tn, - 'dnet' => $this->tf, + 'onode' => $this->ff, + 'dnode' => $this->tf, + 'onet' => $this->fn, + 'dnet' => $this->tn, 'flags' => $this->flags, 'cost' => $this->cost, 'echoarea' => $this->echoarea, @@ -874,8 +874,8 @@ class Message extends FTNBase 'user_from' => 'required|min:1|max:'.self::USER_FROM_LEN, 'user_to' => 'required|min:1|max:'.self::USER_TO_LEN, 'subject' => 'present|max:'.self::SUBJECT_LEN, - 'onode' => ['required',new TwoByteInteger], - 'dnode' => ['required',new TwoByteInteger], + 'onode' => ['required',new TwoByteIntegerWithZero], + 'dnode' => ['required',new TwoByteIntegerWithZero], 'onet' => ['required',new TwoByteInteger], 'dnet' => ['required',new TwoByteInteger], 'flags' => 'required|numeric', diff --git a/app/Classes/FTN/Packet.php b/app/Classes/FTN/Packet.php index 4d92840..7ba39db 100644 --- a/app/Classes/FTN/Packet.php +++ b/app/Classes/FTN/Packet.php @@ -5,8 +5,8 @@ namespace App\Classes\FTN; use Carbon\Carbon; use Illuminate\Support\Arr; use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; -use Illuminate\Support\Facades\Redis; use Symfony\Component\HttpFoundation\File\File; use App\Classes\FTN as FTNBase; @@ -59,8 +59,7 @@ class Packet extends FTNBase implements \Iterator, \Countable public Collection $messages; // Messages in the Packet public Collection $errors; // Messages that fail validation private string $name; // Packet name - private ?System $system; // System the packet is from - public bool $use_redis = TRUE; // Use redis for messages. + public bool $use_cache = TRUE; // Use a cache for messages. private int $index; // Our array index /** @@ -71,14 +70,14 @@ class Packet extends FTNBase implements \Iterator, \Countable return $this->messages->count(); } - public function current() + public function current(): Message { - return $this->use_redis ? unserialize(Redis::get($this->key())) : $this->messages->get($this->index); + return $this->use_cache ? unserialize(Cache::pull($this->key())) : $this->messages->get($this->index); } - public function key() + public function key(): mixed { - return $this->messages->get($this->index); + return $this->use_cache ? $this->messages->get($this->index) : $this->index; } public function next(): void @@ -86,14 +85,14 @@ class Packet extends FTNBase implements \Iterator, \Countable $this->index++; } - public function rewind() + public function rewind(): void { $this->index = 0; } - public function valid() + public function valid(): bool { - return $this->use_redis ? ($this->key() && Redis::exists($this->key())) : $this->key(); + return (! is_null($this->key())) && ($this->use_cache ? Cache::has($this->key()) : $this->messages->has($this->key())); } public function __construct(Address $o=NULL) @@ -113,11 +112,11 @@ class Packet extends FTNBase implements \Iterator, \Countable * * @param File $file * @param System|null $system - * @param bool $use_redis + * @param bool $use_cache * @return Packet * @throws InvalidPacketException */ - public static function open(File $file,System $system=NULL,bool $use_redis=TRUE): self + public static function open(File $file,System $system=NULL,bool $use_cache=TRUE): self { Log::debug(sprintf('%s:+ Opening Packet [%s]',self::LOGKEY,$file)); @@ -137,7 +136,7 @@ class Packet extends FTNBase implements \Iterator, \Countable throw new InvalidPacketException('Not a type 2 packet: '.$version); $o = new self; - $o->use_redis = $use_redis; + $o->use_cache = $use_cache; $o->name = (string)$file; $o->header = unpack(self::unpackheader(self::v2header),$header); @@ -428,7 +427,7 @@ class Packet extends FTNBase implements \Iterator, \Countable // If the message is invalid, we'll ignore it if ($msg->errors) { - Log::info(sprintf('%s:- Message has errors',self::LOGKEY)); + Log::info(sprintf('%s:- Message [%s] has errors',self::LOGKEY,$msg->msgid)); // If the from address doenst exist, we'll create a new entry if ($msg->errors->messages()->has('from')) { @@ -479,9 +478,9 @@ class Packet extends FTNBase implements \Iterator, \Countable } } - if ($this->use_redis) { - $key = $msg->msgid ?: sprintf('%s %s',$msg->fftn,Carbon::now()->timestamp); - Redis::set($key,serialize($msg)); + if ($this->use_cache) { + $key = urlencode($msg->msgid ?: sprintf('%s %s',$msg->fftn,Carbon::now()->timestamp)); + Cache::forever($key,serialize($msg)); $this->messages->push($key); } else { diff --git a/app/Jobs/MessageProcess.php b/app/Jobs/MessageProcess.php index 32f2b4f..059631d 100644 --- a/app/Jobs/MessageProcess.php +++ b/app/Jobs/MessageProcess.php @@ -166,16 +166,13 @@ class MessageProcess implements ShouldQueue if (! $o->msg_crc) $o->msg_crc = md5($this->msg->message); - // Using filter here, due to earlier bugs - and to get rid of the null values - $o->path = collect($o->getRawOriginal('path'))->filter()->toArray(); + $o->save(); // If the path is empty, then its probably because of the previous bug, we'll replace it. // @todo This duplicate message may have gone via a different path, be nice to record it. - if (! $o->path->count()) - $o->path = collect($o->getRawOriginal('path'))->merge($this->msg->pathaddress)->toArray(); - - $o->seenby = collect($o->getRawOriginal('seenby'))->merge($this->msg->seenaddress)->filter()->toArray(); - $o->save(); + //$o->path()->sync($o->path->pluck('id')->merge($this->msg->pathaddress)->toArray()); + // @todo if we have an export for any of the seenby addresses, remove it + $o->seenby()->sync($o->seenby->pluck('id')->merge($this->msg->seenaddress)->filter()->toArray()); return; } @@ -215,11 +212,10 @@ class MessageProcess implements ShouldQueue $o->msg = $this->msg->message_src; $o->msg_crc = md5($this->msg->message); - $o->path = $this->msg->pathaddress->jsonSerialize(); - $o->rogue_path = $this->msg->rogue_path->jsonSerialize(); - $o->seenby = $this->msg->seenaddress->jsonSerialize(); - $o->rogue_seen = $this->msg->rogue_seen->jsonSerialize(); - $o->toexport = TRUE; + $o->rogue_seenby = $this->msg->rogue_seenby; + $o->rogue_path = $this->msg->rogue_path; + $o->set_path = $this->msg->pathaddress->toArray(); + $o->set_seenby = $this->msg->seenaddress->toArray(); $o->save(); diff --git a/app/Models/Address.php b/app/Models/Address.php index 1005edc..be84ac2 100644 --- a/app/Models/Address.php +++ b/app/Models/Address.php @@ -122,12 +122,28 @@ class Address extends Model ->with(['zone.domain']); } + /** + * Echoareas this address is subscribed to + * + * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany + */ public function echoareas() { return $this->belongsToMany(Echoarea::class) ->withPivot(['subscribed']); } + /** + * Echomails that this address has seen + * + * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany + */ + public function echomails() + { + return $this->belongsToMany(Echomail::class,'echomail_seenby') + ->withPivot(['sent_at','packet']); + } + /** * Who we send this systems mail to. * @@ -346,9 +362,7 @@ class Address extends Model */ public function echomailWaiting(): Collection { - return Echomail::select(['echomails.*']) - ->join('echomail_seenby',['echomail_seenby.echomail_id'=>'echomails.id']) - ->where('echomail_seenby.address_id',$this->id) + return $this->echomails() ->whereNull('echomail_seenby.sent_at') ->whereNotNull('echomail_seenby.export_at') ->get(); @@ -372,6 +386,7 @@ class Address extends Model ->whereIn('echomail_id',$x->pluck('id')) ->where('address_id',$this->id) ->whereNull('sent_at') + ->whereNull('packet') ->whereNotNull('echomail_seenby.export_at') ->update(['sent_at'=>Carbon::now(),'packet'=>$pkt->name]); } @@ -411,15 +426,9 @@ class Address extends Model { $o = new Packet($this); - foreach ($c as $oo) { + foreach ($c as $oo) $o->addMail($oo->packet($this)); - $oo->packet = $o->name; - $oo->sent = TRUE; - $oo->sent_at = Carbon::now(); - $oo->save(); - } - return $o; } diff --git a/app/Models/Echomail.php b/app/Models/Echomail.php index 790d6a3..a97acbd 100644 --- a/app/Models/Echomail.php +++ b/app/Models/Echomail.php @@ -171,7 +171,7 @@ final class Echomail extends Model implements Packet $o->flags = $this->flags; if ($this->kludges) - $o->kludge = $this->kludges; + $o->kludge = collect($this->kludges); $o->kludge->put('dbid',$this->id); diff --git a/app/Rules/TwoByteInteger.php b/app/Rules/TwoByteInteger.php index e388edc..b04b521 100644 --- a/app/Rules/TwoByteInteger.php +++ b/app/Rules/TwoByteInteger.php @@ -8,26 +8,26 @@ use App\Http\Controllers\DomainController; class TwoByteInteger implements Rule { - /** - * Determine if the validation rule passes. + /** + * Determine if the validation rule passes. * This will check that a number used for zone, net, host is between 1 and DomainController::NUMBER_MAX. - * - * @param string $attribute - * @param mixed $value - * @return bool - */ - public function passes($attribute,$value) - { - return (is_numeric($value) && ($value > 0) && ($value < DomainController::NUMBER_MAX)); - } + * + * @param string $attribute + * @param mixed $value + * @return bool + */ + public function passes($attribute,$value) + { + return (is_numeric($value) && ($value > 0) && ($value < DomainController::NUMBER_MAX)); + } - /** - * Get the validation error message. - * - * @return string - */ - public function message() - { - return sprintf('The number must be between 1 and %d.',DomainController::NUMBER_MAX); - } + /** + * Get the validation error message. + * + * @return string + */ + public function message() + { + return sprintf('The number must be between 1 and %d.',DomainController::NUMBER_MAX); + } } diff --git a/app/Rules/TwoByteIntegerWithZero.php b/app/Rules/TwoByteIntegerWithZero.php new file mode 100644 index 0000000..1c6251c --- /dev/null +++ b/app/Rules/TwoByteIntegerWithZero.php @@ -0,0 +1,33 @@ += 0) && ($value < DomainController::NUMBER_MAX)); + } + + /** + * Get the validation error message. + * + * @return string + */ + public function message() + { + return sprintf('The number must be between 1 and %d.',DomainController::NUMBER_MAX); + } +} diff --git a/tests/Feature/PacketTest.php b/tests/Feature/PacketTest.php index 61f71a3..4145719 100644 --- a/tests/Feature/PacketTest.php +++ b/tests/Feature/PacketTest.php @@ -48,9 +48,9 @@ class PacketTest extends TestCase $this->assertCount(1,$msg->rogue_path); $this->assertNotFalse($msg->rogue_path->search('21:999/1')); - $this->assertCount(3,$msg->rogue_seen); - $this->assertNotFalse($msg->rogue_seen->search('21:999/1')); - $this->assertNotFalse($msg->rogue_seen->search('21:999/999')); + $this->assertCount(3,$msg->rogue_seenby); + $this->assertNotFalse($msg->rogue_seenby->search('21:999/1')); + $this->assertNotFalse($msg->rogue_seenby->search('21:999/999')); } $this->assertTrue($messages); @@ -79,7 +79,7 @@ class PacketTest extends TestCase $this->assertCount(1,$msg->rogue_path); $this->assertNotFalse($msg->rogue_path->search('10:1/1')); - $this->assertCount(0,$msg->rogue_seen); + $this->assertCount(0,$msg->rogue_seenby); } $this->assertTrue($messages); @@ -106,9 +106,9 @@ class PacketTest extends TestCase $this->assertNotTrue($msg->isNetmail()); $this->assertCount(0,$msg->rogue_path); - $this->assertCount(2,$msg->rogue_seen); - $this->assertNotFalse($msg->rogue_seen->search('10:1/1')); - $this->assertNotFalse($msg->rogue_seen->search('10:999/999')); + $this->assertCount(2,$msg->rogue_seenby); + $this->assertNotFalse($msg->rogue_seenby->search('10:1/1')); + $this->assertNotFalse($msg->rogue_seenby->search('10:999/999')); $this->assertNotFalse($msg->seenaddress->search($src->id)); } @@ -133,7 +133,7 @@ class PacketTest extends TestCase $this->assertSame('db727bd3778ddd457784ada4bf016010',md5($msg->message)); $this->assertSame('5b627ab5936b0550a97b738f4deff419',md5($msg->message_src)); $this->assertCount(3,$msg->rogue_path); - $this->assertCount(170,$msg->rogue_seen); + $this->assertCount(170,$msg->rogue_seenby); $this->assertContains('3/2744 4/100 106 5/100',$msg->seenby); $this->assertContains('1/151 100 3/100',$msg->path); $this->assertCount(11,$msg->seenby); @@ -157,7 +157,7 @@ class PacketTest extends TestCase $this->assertSame('5a525cc1c393292dc65160a852d4d615',md5($msg->message)); $this->assertSame('a3193edcc68521d4ed07da6db2aeb0b6',md5($msg->message_src)); $this->assertCount(3,$msg->rogue_path); - $this->assertCount(161,$msg->rogue_seen); + $this->assertCount(161,$msg->rogue_seenby); $this->assertContains('1/995 2/100 116 1202 3/100 105 107 108 109 110 111 112 113 117 119',$msg->seenby); $this->assertContains('1/126 100 3/100',$msg->path); $this->assertCount(10,$msg->seenby); @@ -181,7 +181,7 @@ class PacketTest extends TestCase $this->assertSame('61078e680cda04c8b5eba0f712582e70',md5($msg->message)); $this->assertSame('b9d65d4f7319ded282f3f1986276ae79',md5($msg->message_src)); $this->assertCount(1,$msg->pathaddress); - $this->assertCount(2,$msg->rogue_seen); + $this->assertCount(2,$msg->rogue_seenby); $this->assertContains('1/1 999/1 999',$msg->seenby); $this->assertContains('999/1',$msg->path); $this->assertCount(1,$msg->seenby);