Package[Repository] vs InstalledPackage[Repository],

[GaryJones]

There's a little bit of mismatch between Package and InstalledPackage, and likewise for PackageRepository and InstalledPackageRepository.

Consider code like this:

https://github.com/blazersix/satispress/blob/65b9662f910a1b9272f7ac8f1da290904e21ea76/src/Archiver.php#L50-L52

The object returned on line 50 is a Package (according to the DocBlock), but the methods called on line 51 and 52 are only contracted by InstalledPackage.

Does a Release always reference an installed package?

---

Likewise, this code:

https://github.com/blazersix/satispress/blob/65b9662f910a1b9272f7ac8f1da290904e21ea76/src/Route/Download.php#L135

is calling a get_installed_version(), which is only defined in InstalledPackage, yet the repository passed in to the constructor is a PackageRepository, which is a collection of Package objects, and not specifically InstalledPackage objects.

A new InstalledPackageRepository, containing InstalledPackage objects would better define the contracts that are currently being assumed.

---

Here are the undefined methods violations:

• Archiver.php
  • Method 'get_directory' not found: line 51
  • Method 'get_files' not found: line 52
• Download.php
  • Method 'get_installed_version' not found: line 133
• PackageArchiver.php
  • Method 'get_installed_release' not found: line 201
• ReleaseManager.php
  • Method 'get_installed_version' not found: line 90
• Upgrade.php
  • Method 'get_installed_release' not found: line 121

[bradyvercher]

A release always references the package it belongs to, so it won't always be an installed package. Say in the future someone defines a package in the admin panel manually through a form and they upload the artifact, that won't be an InstalledPackage, but it will have one or more releases.

An InstalledPackageRepository might help in a couple places, but I'd have to see what impact it has on being able to easily decorate repositories.

For Archiver.php, that method signature could probably change to accept an InstalledPackage instance and a version number instead. For all the others, it looks like those methods should be checking Package::is_installed() or if the package is an instance of InstalledPackage first.

[bradyvercher]

This should be fixed by #68.