hirethunk / verbs Goto Github PK

What happened?

Verbs stores the user's event data mingled with internal Verbs data. Thus:

1. You cannot create an event with a phase property. (Type of App\Events\BrokeVerbs::$phase must be Thunk\Verbs\Lifecycle\Phase (as in class Thunk\Verbs\Event))
2. If you create an id property (of type int), no error will be thrown, but Verbs will nuke it and overwrite it with that row's id snowflake.

The phase one isn't too bad. You get an error, and anyone encountering it will likely realize they need to rename their phase in that event.

The id one is kinda nasty. It's highly likely someone will try this, and it just silently overwrites their data.

Some questions:

1. Could these internal fields be prefixed with an underscore to reduce the chance of accidental collision?
2. Is it necessary to store id in both the id column of an event and also in the JSON data column?
3. Is it necessary to store phase in the database at all?

How to reproduce the bug

<?php

namespace App\Events;

use Thunk\Verbs\Event;

class BrokeVerbsPhase extends Event {
	public function __construct(public string $phase) {
	}
	public function handle() {
	}
}

<?php

namespace App\Events;

use Thunk\Verbs\Event;

class BrokeVerbsId extends Event {
	public function __construct(public int $id) {
	}
	public function handle() {
	}
}

Then just fire those events.

Package Version

0.0.6

PHP Version

8.1.26

Laravel Version

10.10

Which operating systems does with happen with?

macOS

Notes

No response

What happened?

Upon trying to install Verbs itself, to check out the tests and such, I get this:

Could not fetch https://api.github.com/repos/DanielCoulbourne/bugg, please review your configured GitHub OAuth token or enter a new one to access private repos

How to reproduce the bug

1. Check out this repo.
2. composer install

Package Version

6f50d61

PHP Version

8.1.26

Laravel Version

x.x.x

Which operating systems does with happen with?

macOS, Windows, Linux

Notes

No response

What happened?

I was following the Verbs quickstart tutorial and ran into a bug while creating a new event using artisan.

The Problem

The problem appears to be on lines 75/76, basically when it checks if there is a path, it appends a '/', but for the first iteration, it is the full path to the app directory, in my case: "C:\laragon6\www\verbs\app\Events", so by adding a forward slash to the front it breaks.

I believe the intent was to use the for loop to create any needed subdirectories which makes sense. I changed the implementation slightly to use PHP constants for directory separators and to use the Illuminate Filesystem.

A related problem, while I was running the code below, I tested generating additional levels of directories, for example:

php artisan verbs:event Test\\Directory\\CustomerBeganTrial

The path is created correctly, but the class name and namespace are not correct, not a huge deal.

Suggested Change

use Illuminate\Support\Facades\File;

 protected function forceWrite($dir, $contents)
{
    // break the path into parts
    $parts = explode(DIRECTORY_SEPARATOR, $dir);

    // pop the last part off to get the file name
    $file = array_pop($parts);

    // rejoin the parts to get the directory
    $dir = implode(DIRECTORY_SEPARATOR, $parts);

    // okay make sure the directory exists
    File::ensureDirectoryExists($dir);

    // now write the file
    File::put($dir.DIRECTORY_SEPARATOR.$file, $contents);
}

I'd be happy to put together a PR with suggestions. Thanks! 😊

Script Output

php artisan verbs:event CustomerBeganTrial

 ErrorException 

mkdir(): No such file or directory

  at vendor\hirethunk\verbs\src\Helpers\Stub.php:76
     72▕         $file = array_pop($parts);
     73▕         $dir = '';
     74▕         foreach ($parts as $part) {
     75▕             if (! is_dir($dir .= "/$part")) {
  ➜  76▕                 mkdir($dir);
     77▕             }
     78▕         }
     79▕         file_put_contents("$dir/$file", $contents);
     80▕     }

  1   vendor\hirethunk\verbs\src\Helpers\Stub.php:76

  2   vendor\hirethunk\verbs\src\Helpers\Stub.php:48
      Thunk\Verbs\Helpers\Stub::forceWrite("/C:\laragon6\www\verbs\app\Events", "<?php

namespace App\Events;

use Thunk\Verbs\Event;

class CustomerBeganTrial extends Event
{
    public function handle()
    {
        // The best things in life are free, not cheap. - Lil Wayne
    }
}
")

How to reproduce the bug

• Install Verbs (0.0.6)
• Attempt to create a new event using artisan.

Package Version

0.0.6

PHP Version

8.2.0

Laravel Version

10.10.0

Which operating systems does with happen with?

Windows

Notes

No response

What happened?

On one of my events, I loaded a state, but ended up deleting its apply() function. This led to me getting an error message like this:

I could make this error go away by simply removing #[StateId(PlayerState::class)] from the $player_id, or by adding a public function apply(PlayerState $state). But if I declare the StateId and then never apply it, I got the confusing error above.

How to reproduce the bug

• Create an event with a state associated with it
• apply something to the state
• create another event with that same state associated with it, but on this second event, do not include an apply() for that state.
• run the code and receive the error message above.

Package Version

0.0.7

PHP Version

8.1

Laravel Version

10.10

Which operating systems does with happen with?

macOS

Notes

My suggestion would be to either:

1. make a more useful error message that explains "You must have an apply() function if you declare a state"
2. Or just allow this to happen.

What happened?

I get the following errors when trying to use Postgres. It seems to work in MySQL.

SQLSTATE[21000]: Cardinality violation: 7 ERROR:  ON CONFLICT DO UPDATE command cannot affect row a second time
HINT:  Ensure that no rows proposed for insertion within the same command have duplicate constrained values. (Connection: pgsql, SQL: insert into "verb_snapshots" ("created_at", "data", "id", "last_event_id", "type", "updated_at") values (2023-11-21 04:57:14, {"balance_in_cents":100000,"id":117488472096960512,"last_event_id":117488472088571904}, 117488472096960512, 117488472088571904, App\States\AccountState, 2023-11-21 04:57:14), (2023-11-21 04:57:14, {"balance_in_cents":100000,"id":117488472096960512,"last_event_id":117488472088571904}, 117488472096960512, 117488472088571904, App\States\AccountState, 2023-11-21 04:57:14) on conflict ("id") do update set "data" = "excluded"."data", "last_event_id" = "excluded"."last_event_id", "updated_at" = "excluded"."updated_at")

How to reproduce the bug

Follow one of the examples and try to run the tests.

Package Version

0.0.5

PHP Version

8.2.0

Laravel Version

10.10

Which operating systems does with happen with?

macOS

Notes

It doesn't seem to be calling upsert twice so I'm not sure why we're getting this error.

My question is: in the doc you are talking about replaying events.

How do we replay events using your packages ?

I don't see any console commands for that...