Better Package's Status Checks

[gmazzap]

Please check if the PR fulfills these requirements

• [x] The commit message follows our guidelines
• [x] Tests for the changes have been added (for bug fixes/features)
• [ ] Docs have been added/updated (for bug fixes/features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature.

What is the current behavior? (You can also link to an open issue here)

The only public method to check package status is Package::statusIs(), which only chechs equality. A more flexible status check is possible via Package::checkStatus(), but that is proivate.

Moreover, there's no way to check if the Package has a container already generated. The Package has a hasContainer property, but that is provate.

Because of the two above points, PackageProxyContainer can not 100% determine when it is "safe" to access the proxied container.

What is the new behavior (if this is a feature change)?

Three new methods are added:

• Package::isFailed() which is a simpler way to do Package::statusIs(Package::STATUS_FAILED)
• Package::hasReachedStatus() which can check if the status is the one above or above
• Package::hasContainer() which tells when a Container is already created regardless teh status

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

No.

Other information:

This PR includes (in a separate commit) some CS fixes caused by new rules added to the Inpsyde Coding Standards.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.43%. Comparing base (d4ea195) to head (657ed77). Report is 1 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #51 +/- ## ============================================ + Coverage 98.42% 98.43% +0.01% - Complexity 238 241 +3 ============================================ Files 10 10 Lines 570 576 +6 ============================================ + Hits 561 567 +6 Misses 9 9 ``` | [Flag](https://app.codecov.io/gh/inpsyde/modularity/pull/51/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inpsyde) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/inpsyde/modularity/pull/51/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inpsyde) | `98.43% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=inpsyde#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[gmazzap]

Happy to discuss this here, or elsewhere. Also happy for you to discard this altogether, in case you don't expect these issues to happen anytime soon.

Let me recap, to see if I understand your points @tfrommen

I see you basically have 2 points:

1. Introducing hasReachedStatus() method we make the status constants value relevant, because we use it for comparison, when before it was unrelevant.
2. The implementation of the method is poor because, for exaple, might return true passing an argument like 3 when that number points to no status.

---

Let me start from the second point, which is the easiest.

I wish we could use enums here but we can't right now. Introduce a custom class to represent status could work, but it is a pretty big breaking change. So I think it is better to stick with integers. But I totally agree the implementation can be improved, and I will.

---

Regarding the first point, let me start saying that having a linear step-after-step forwarding of statuses (with the exceptions of "failed" which could happen at any time) was a fundational idea on how the status would work. It is even documented this way: https://github.com/inpsyde/modularity/blob/master/docs/Application-flow.md#building-stage

So I don't think in the foresable future this will change. I do think that the actual statuses will change (as we have already dicussed in another issue), but the idea of consequential and enumerable steps is something that I don't see being challenged in the foresable future.

That said, not all the future is foresable :) And this is why I did not just make Package::checkStatus() public. That method exposes the logic of numerical comparison by accepting a comparison operator, and while making it public would have allowed for greater flexibility now, and would have saved me to introduce a new method, it would have forced us to stick with this logic.

By introducing the hasReachedStatus() method, it is true that the statuses constant' value is now relevant, but it is also true that the "comparison logic" is still encapsulated, in other words, the value is relevant internally, not for consumers.

So, if in the non-foreseable future we will introduce optional statuses, multiple error statuses, or anyway something that does not fit the "numerical sequence" logic we have right now, we can still do that without bothering consumers.

Let's assume we introduce a simple state machine, and we use a Statuses class to represent it, we could have someting like Statuses->push($newStatus) whenever the package enters that state, and then we could have the hasReachedStatus() method look like:

    public function hasReachedStatus(int $status): bool
    {
        $this->states->has($status);
    }

For consumers nothing would change. Because we don't expose the comparison logic (even if we use it internally), we can change course if we have to, without breaking backward compatibility.

So, why doing now a more impactful change when, as of today, we don't have any reason to think it will be needed, and if in the future it will be needed we have ways to achieve it without causing much trouble? Even because, if we will have reasons to have a state machine in the future, maybe in that future we will have PHP 8.1+ as min PHP version, and we would be able to use enums, for example, so a state machine we would write today would be refactored again.