How a recently opened critical Drupal Core bug is 15 years old

As we approach the release of Drupal 8.3.0 and start working on 8.4.x, I want to take a look at technical debt.

Here's a quick overview of technical debt in the past 18 months as expressed in major and critical bugs (other technical debt isn't represented by these numbers):

• 800 major bugs fixed
• 200 critical bugs fixed
• 21 critical bugs currently open
• Over 600 major bugs currently open

 

Sometimes code is buggy in itself, no one spots the bug in review, test coverage is incomplete, and then these get uncovered after commit.

However, sometimes there is code that all works independently, but the interactions of a complex system like Drupal results in critical bugs arising seemingly out of the ether. As an example, let's talk about a fifteen-year-old bug in a sixteen-year-old module that got written up as a new core bug report just this week.

While this may be the oldest undiscovered critical bug in core, it points to a wider mechanism whereby technical debt is often introduced – simply via the addition of incomplete features over time.

Here's Book module's birthday commit in March 2001.

commit 6275348098baa21523f31d8c228cd7fa0dd64b2d

Author: Dries Buytaert Date: Sat Mar 24 16:36:13 2001 +0000

 

the "faq module" and the "documentation module" are going to be bundled into a much more powerful and easier to maintain "book module": each "page"in the big "drop.org/drupal book" is a node and everyone with a user account  can suggest new pages or updates of existing pages.

 

Book module allows you to organise nodes into a hierarchy, which allows for a table of contents and previous/next links.

Here's the commit that added revision support to nodes in November 2001.

 

commit a2e6910902bfb1263e1b6363e2c29ede68f89918

Author: Dries Buytaert

Date:   Sat Nov 3 18:38:30 2001 +0000

 

   - Made the node forms support "help texts": it is not possible to configure

     Drupal to display submission guidelines, or any other kind of explanation

     such as "NO TEST POSTS", for example.

   

   - Added node versioning: it is possible to create revisions, to view old

     revisions and to roll-back to older revisions.  You'll need to apply a

     SQL update.

   

     I'm going to work on the book module now, so I might be changing a few

     things to enable collaborative, moderated revisions - but feel free to

     send some first feedback, if you like.

   

   - Added some configuration options which can be used to set the minimum

     number of words a blog/story should consist of.  Hopefully this will

     be usefull to stop the (almost empty) test blogs.

   

   - Various improvements:

      + Fine-tuned new node permission system.

      + Fine-tuned the functions in node.inc.

      + Fine-tuned some forms.

      + XHTML-ified some code.

 

One important aspect of this is that the storage of that hierarchy happens in a dedicated table, controlled by the book module, which has not been kept up-to-date with changes in node storage.

I don't know if there was a time in between 2001 and 2017 where book storage supported revisions, but it didn't with the original commit, and it still doesn't now.

So now we have a node storage which maintains each version of a node, allowing you to roll back to earlier versions (and I never realised this was added in 2001 until today, that's impressive) combined with a book storage that is versionless.

Once those two features were combined, there's already a bug, or at least a limitation:

1. Create a new book node A as a child of a different node B, revision 1
2. Edit the node to change the parent node from B to C, creating a new revision 2
3. Revert the change you just made to node A back to the revision 1 - the parent node will still be C

I wasn't able to find any existing bug report for this, but it persists in Drupal 8 today. Changing book parents is quite rare, and reverting revisions is also quite rare, so it's possible no-one noticed in 16 years.

We then skip forward 11 years, to adding support for draft revisions to the core entity API (there had been various implementations of this in contrib before this commit, but they all required extensive workarounds).

commit e203b73b6292adb19fb9197d532dd2b928133163

Author: webchick

Date: Thu Sep 6 13:32:19 2012 -0700

 

   Issue #218755 by jstoller, Gábor Hojtsy, stevector, mradcliffe, agentrickard, catch, Crell: Added Support revisions in different states.

 

Now you can save a new draft revision, without changing the published one, then publish that draft later on. However, it's only API support, so nothing user-facing changes.

Skip forward another four years to 2016, and the addition of Content Moderation as an experimental module.

commit bc00f081e6b8e35e3b7ee57eb963b7e5b92593a2

Author: Nathaniel Catchpole

Date:Mon Aug 8 13:26:31 2016 +0100

 

   Issue #2725533 by timmillwood, alexpott, amateescu, webchick, dixon_, larowlan, dawehner, catch, Crell, Bojhan, jibran, Wim Leers, agentrickard, Berdir: Add experimental content_moderation module

 

Content Moderation adds a user interface for creating draft revisions – the first time it's been possible to do so in core since the API capacity was added.

Additionally, there is workflow and access support, so that some users can only create draft changes while others review and publish them.

This then adds two new bugs on top of the 15-year-old issue with reverting revisions:

1. Changing the book parent when creating a draft revision unexpectedly changes the book outline globally as a side effect. Deleting the draft won't put it back either.
2. Due to the first issue, where workflow access is set up so that people who are not allowed to do anything other than create drafts, this becomes an access bypass since they can make changes to the published book structure without permissions to publish.

 

If we look back at the original revisions commit in 2001, there's this note:

     I'm going to work on the book module now, so I might be changing a few

     things to enable collaborative, moderated revisions - but feel free to

     send some first feedback, if you like.
 

What we're doing with Drupal 8 and the Workflow Initiative is adding collaborative, moderated revisions to core fifteen years after it was first conceived. There are other revision issues with both menu links and path aliases, which also have a history going back more than ten years, also discovered within the past year as the result of working on the Workflow Initiative.

So when looking at where bugs are introduced, how can we deal with this? If we had a time machine to go back to 2001, would we tell Dries not to introduce revision support for nodes? I don't think we would since this is very important to allow collaborative editing without data loss. Do we blame initiatives like Workflow when they expose 15-year-old technical debt added before most or all participants had even heard of Drupal? I don't think we'd do that either.

What we can do though is to look for steps that might have been taken to either avoid the bug being introduced in the first place or to have mitigated it since - and looking at some more history of Book module, we can see tendencies towards both. When reviewing these issues, these point to an approach towards anticipating and removing technical debt more generally.

As early as 2005, there were suggestions to merge forum and Book modules into a generic ‘hierarchical entity outline' storage.

In 2007, Book module was refactored to rely on the menu link hierarchy rather than having its own separate implementation.

In 2008, there was work to refactor the hierarchy storage of taxonomy module into an improved and generic subsystem for managing hierarchy. This would then have been applied to the book and menu systems, however the issue was not completed and remains open.

In 2011, there was a proposal to remove the Book module from core and let it be maintained in contrib instead. This was not done and the issue remains open.

In 2013, Book's hierarchy storage had to be factored back out into a custom implementation as a side-effect of menu link system refactoring in Drupal 8 (which itself was a side-effect of adding a new routing system which had not taken into account interactions with the menu link system – and by extension, book hierarchy – in 2011).

If we look at these issues, both completed and open, there's an ‘alternative timeline' of the Book module, pointing towards either consolidation of Drupal core's three entity hierarchy storage implementations into a single implementation, or removing the Book module from core in order to have just two entity hierarchy systems to deal with. While refactoring and removal are very different approaches, they both stem from the understanding that it is hard work to maintain three separate entity hierarchy storage implementations in a single software product.

The Drupal 8 entity system work consolidated previously disparate implementations for user accounts, nodes, custom blocks, and taxonomy terms to rely on a single unified storage mechanism. This work does not immediately result in user-facing improvements. It does however make user-facing improvements much easier to apply across all those entity types once completed. Taxonomy terms will soon have revision support (and by extension, draft and workflow support) for the first time, relying on Drupal 8's entity storage to implement, but the storage of taxonomy hierarchy has the same inherent limitations as we just have with Book module. The adding of revisions to taxonomy module mirrors the addition of revisions to nodes 15 years ago, including the conflict with an existing unversioned hierarchy storage.

Now that we understand the process by which critical technical debt was introduced to book module and nodes fifteen years ago and stayed hidden most of that time, we can apply the same thinking to the addition of revisions to taxonomy module now. In future I hope we can broaden this analysis to recognise and anticipate technical debt much earlier in the future.