standardebooks / web

The source code for the Standard Ebooks website.
https://standardebooks.org
Creative Commons Zero v1.0 Universal
234 stars 61 forks source link

Complete the DB write path #366

Closed colagrosso closed 3 months ago

colagrosso commented 3 months ago

There are 5 commits in this PR, 3 small, 1 large, and 1 extra large. I could send these commits one at a time in different PRs if you would prefer. I don't think breaking up the extra large commit would make it clearer because it's attacking the large constructor.

  1. (extra large) Rename the constructor Ebook::__construct() to static Ebook::FromFilesystem()

We discussed this back in February in #336.

I added GetFoo() methods for all the derived properties like GetUrl(), GetHasDownloads(), etc. That logic is reusable to Ebook objects no matter if they were created from Ebook::FromFilesystem() or Ebook::GetByIdentifier().

The result is that Ebook.php got more verbose. I personally think the logic for each property is now clearer, but it might be a shock to see how much bigger the file is.

Also in the commit: Collection, Contributor, EbookSource, and GitCommit need zero-argument constructors for DbConnection::ExecuteQuery(), so I renamed the existing constructors to static factory methods:

The DB path uses the zero-argument constructors, and the filesystem path uses the static factory methods.

  1. (large) I added a step to update-ebook-database so that it:

a. creates an Ebook from the filesystem b. updates the DB c. reads back the Ebook from the DB and ensures that the two Ebook instances match

It's extra work, and I'm bad at PHP reflection so the implementation could be better, but it was useful while developing and debugging. It gives me the confidence to move onto the read path tasks like updating ebook.php knowing that we can pull valid Ebook instances from the DB. My local DB has 961 Ebook records, and the script didn't find any object differences.

colagrosso commented 3 months ago

Thanks for the quick look! I'll fix the type errors. PHPStan returns no errors, but it's a good reminder to be extra careful.