From 096e37ef3539692041d57b6332990f3a52c3fae1 Mon Sep 17 00:00:00 2001 From: Deon George Date: Fri, 15 Sep 2023 08:09:42 +1000 Subject: [PATCH] Removed packet cache, it wasnt used and not needed since we can queue large packets. Renamed to for consistent variable when using Packet::process() --- app/Classes/FTN/Packet.php | 30 ++++++++++-------------------- app/Jobs/PacketProcess.php | 24 ++++++++++++------------ tests/Feature/PacketTest.php | 14 +++++++------- 3 files changed, 29 insertions(+), 39 deletions(-) diff --git a/app/Classes/FTN/Packet.php b/app/Classes/FTN/Packet.php index 7b31b14..16e9ff0 100644 --- a/app/Classes/FTN/Packet.php +++ b/app/Classes/FTN/Packet.php @@ -13,13 +13,15 @@ use App\Classes\FTN as FTNBase; use App\Models\{Address,Software,System,Zone}; /** - * Represents the structure of a message bundle + * Represents a Fidonet Packet, that contains an array of messages. + * + * Thus this object is iterable as an array of Message::class. */ class Packet extends FTNBase implements \Iterator, \Countable { private const LOGKEY = 'PKT'; - private const BLOCKSIZE = 1024; + private const BLOCKSIZE = 1024; protected const PACKED_MSG_LEAD = "\02\00"; public const PACKET_TYPES = [ @@ -35,8 +37,7 @@ class Packet extends FTNBase implements \Iterator, \Countable public File $file; // Packet filename public Collection $messages; // Messages in the Packet public Collection $errors; // Messages that fail validation - public bool $use_cache = FALSE; // Use a cache for messages. - protected int $index; // Our array index + protected int $index; // Our array index /** * @param string|null $header @@ -172,11 +173,10 @@ class Packet extends FTNBase implements \Iterator, \Countable * @param string $name * @param int $size * @param System|null $system - * @param bool $use_cache * @return Packet * @throws InvalidPacketException */ - public static function process(mixed $f,string $name,int $size,System $system=NULL,bool $use_cache=FALSE): self + public static function process(mixed $f,string $name,int $size,System $system=NULL): self { Log::debug(sprintf('%s:+ Opening Packet [%s] with size [%d]',self::LOGKEY,$name,$size)); @@ -207,7 +207,6 @@ class Packet extends FTNBase implements \Iterator, \Countable if (! $o) throw new InvalidPacketException('Cannot determine type of packet.'); - $o->use_cache = $use_cache; $o->name = $name; $x = fread($f,2); @@ -338,12 +337,12 @@ class Packet extends FTNBase implements \Iterator, \Countable public function current(): Message { - return $this->use_cache ? unserialize(Cache::pull($this->key())) : $this->messages->get($this->index); + return $this->messages->get($this->index); } public function key(): mixed { - return $this->use_cache ? $this->messages->get($this->index) : $this->index; + return $this->index; } public function next(): void @@ -358,7 +357,7 @@ class Packet extends FTNBase implements \Iterator, \Countable public function valid(): bool { - return (! is_null($this->key())) && ($this->use_cache ? Cache::has($this->key()) : $this->messages->has($this->key())); + return (! is_null($this->key())) && $this->messages->has($this->key()); } /* METHODS */ @@ -514,15 +513,6 @@ class Packet extends FTNBase implements \Iterator, \Countable } } - if ($this->use_cache) { - $key = urlencode($msg->msgid ?: sprintf('%s %s',$msg->fftn,Carbon::now()->timestamp)); - if (! Cache::forever($key,serialize($msg))) - throw new \Exception(sprintf('Caching failed for key [%s]?',$key)); - - $this->messages->push($key); - - } else { - $this->messages->push($msg); - } + $this->messages->push($msg); } } \ No newline at end of file diff --git a/app/Jobs/PacketProcess.php b/app/Jobs/PacketProcess.php index a0f1eeb..f66e3c9 100644 --- a/app/Jobs/PacketProcess.php +++ b/app/Jobs/PacketProcess.php @@ -59,31 +59,31 @@ class PacketProcess implements ShouldQueue $processed = FALSE; foreach ($f as $packet) { - $po = Packet::process($packet,Arr::get(stream_get_meta_data($packet),'uri'),$f->itemSize(),$this->ao->system); + $pkt = Packet::process($packet,Arr::get(stream_get_meta_data($packet),'uri'),$f->itemSize(),$this->ao->system); // Check the messages are from the uplink - if ($this->ao->system->addresses->search(function($item) use ($po) { return $item->id === $po->fftn_o->id; }) === FALSE) { - Log::error(sprintf('%s:! Packet [%s] is not from this link? [%d]',self::LOGKEY,$po->fftn_o->ftn,$this->ao->system_id)); + if ($this->ao->system->addresses->search(function($item) use ($pkt) { return $item->id === $pkt->fftn_o->id; }) === FALSE) { + Log::error(sprintf('%s:! Packet [%s] is not from this link? [%d]',self::LOGKEY,$pkt->fftn_o->ftn,$this->ao->system_id)); break; } // Check the packet password - if ($this->ao->session('pktpass') !== $po->password) { - Log::error(sprintf('%s:! Packet from [%s] with password [%s] is invalid.',self::LOGKEY,$this->ao->ftn,$po->password)); + if ($this->ao->session('pktpass') !== $pkt->password) { + Log::error(sprintf('%s:! Packet from [%s] with password [%s] is invalid.',self::LOGKEY,$this->ao->ftn,$pkt->password)); - Notification::route('netmail',$this->ao)->notify(new PacketPasswordInvalid($po->password,$this->file->nameas)); + Notification::route('netmail',$this->ao)->notify(new PacketPasswordInvalid($pkt->password,$this->file->nameas)); break; } - Log::info(sprintf('%s:- Packet has [%d] messages',self::LOGKEY,$po->count())); + Log::info(sprintf('%s:- Packet has [%d] messages',self::LOGKEY,$pkt->count())); // Queue messages if there are too many in the packet. - if ($queue = ($po->count() > config('app.queue_msgs'))) + if ($queue = ($pkt->count() > config('app.queue_msgs'))) Log::info(sprintf('%s:- Messages will be sent to the queue for processing',self::LOGKEY)); $count = 0; - foreach ($po as $msg) { + foreach ($pkt as $msg) { Log::info(sprintf('%s:- Mail from [%s] to [%s]',self::LOGKEY,$msg->fftn,$msg->tftn)); // @todo Quick check that the packet should be processed by us. @@ -106,9 +106,9 @@ class PacketProcess implements ShouldQueue try { // Dispatch job. if ($queue) - MessageProcess::dispatch($msg,$f->pktName(),$this->ao,$po->fftn_o,$this->rcvd_time); + MessageProcess::dispatch($msg,$f->pktName(),$this->ao,$pkt->fftn_o,$this->rcvd_time); else - MessageProcess::dispatchSync($msg,$f->pktName(),$this->ao,$po->fftn_o,$this->rcvd_time); + MessageProcess::dispatchSync($msg,$f->pktName(),$this->ao,$pkt->fftn_o,$this->rcvd_time); } catch (\Exception $e) { Log::error(sprintf('%s:! Got error dispatching message [%s] (%d:%s-%s).',self::LOGKEY,$msg->msgid,$e->getLine(),$e->getFile(),$e->getMessage())); @@ -117,7 +117,7 @@ class PacketProcess implements ShouldQueue $count++; } - if ($count === $po->count()) + if ($count === $pkt->count()) $processed = TRUE; } diff --git a/tests/Feature/PacketTest.php b/tests/Feature/PacketTest.php index dc5b836..bbd2734 100644 --- a/tests/Feature/PacketTest.php +++ b/tests/Feature/PacketTest.php @@ -35,7 +35,7 @@ class PacketTest extends TestCase // This packet has an incorrect zone in the Origin $f = new File(__DIR__.'/data/test_nomsgid_origin.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),NULL,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize()); $this->assertEquals(1,$pkt->count()); @@ -70,7 +70,7 @@ class PacketTest extends TestCase // This packet has an incorrect zone in the Origin $f = new File(__DIR__.'/data/test_nomsgid_noorigin.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),$this->so,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),$this->so); $this->assertEquals(1,$pkt->count()); @@ -101,7 +101,7 @@ class PacketTest extends TestCase // This packet has an incorrect zone in the Origin $f = new File(__DIR__.'/data/test_msgid_origin.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),$this->so,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),$this->so); $this->assertEquals(1,$pkt->count()); @@ -126,7 +126,7 @@ class PacketTest extends TestCase // This packet has a SOHSOH sequence $f = new File(__DIR__.'/data/test_binary_content-2.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),NULL,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize()); $this->assertEquals(1,$pkt->count()); @@ -152,7 +152,7 @@ class PacketTest extends TestCase // This packet has SOH in the message content $f = new File(__DIR__.'/data/test_binary_content.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),NULL,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize()); $this->assertEquals(1,$pkt->count()); @@ -178,7 +178,7 @@ class PacketTest extends TestCase // This packet has an incorrect zone in the Origin $f = new File(__DIR__.'/data/test_msgid_origin.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),NULL,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize()); $this->assertEquals(1,$pkt->count()); @@ -208,7 +208,7 @@ class PacketTest extends TestCase $f = new File(__DIR__.'/data/test_msg_with_soh_in_origin.pkt'); foreach ($f as $packet) { - $pkt = Packet::process($packet,$f->itemName(),$f->itemSize(),NULL,FALSE); + $pkt = Packet::process($packet,$f->itemName(),$f->itemSize()); $this->assertEquals(9,$pkt->count());