Cannot deploy book nodes - book metatdata not serialized

I'll take a stab at this if it's alright.

This is basically a copy & paste effort, as suggested. I'll be happy to add to that if needed.

Oops.

Yay, thanks for this! I must admit that I don't know too much about Book module, so this is a lot of nitpicks and a couple of general remarks, although a couple of things are specific to Rest, albeit not Book.

+/**
+ * @file
+ * Contains \Drupal\book\BookServiceProvider.
+ */

Here, and elsewhere: This is no longer required, and will bug anyone running PHPCS on core. Please remove.

+ * Creates a service modifier to add a book normalizer.

This is pretty nitpicky, but this doesn't actually create a service modifier, it is one. How about: Provides a book normalizer service if HAL module is installed.

+class BookServiceProvider implements ServiceModifierInterface {

We're adding a service, so we should instead implement ServiceProviderInterface. That has a register() method, instead of alter() but it works the same way. It just fits better semantically with what we're doing here.

+  /**
+   * Psuedo field name for embedding parent book.
+   *
+   * @var string
+   */
+  const PSUEDO_PARENT_FIELD_NAME = 'book_parent';
+
+  /**
+   * Psuedo field name for embedding book.
+   *
+   * @var string
+   */
+  const PSUEDO_BOOK_FIELD_NAME = 'book';

It's not clear to me from reading the patch why this is needed. Can we at least add some docs?

+    // Ensure this normalizer is only registered if hal.module is active.

Again very nitpicky, but:
hal.module -> HAL module
active -> installed

+      $service_definition = new Definition('Drupal\book\Normalizer\BookNormalizer', array(

Here, and elsewhere: Could be changed to use the new array syntax.

+      // The priority must be higher than that of
+      // serializer.normalizer.entity.hal in hal.services.yml.

Yay, thank you so much for this!!! In my opinion every service priority should be documented explicitly, but we rarely do it. Thanks, this is great!

+namespace Drupal\book\Normalizer;
+
+
+use Drupal\Core\Config\ConfigFactoryInterface;

Too many newlines.

+use Drupal\Core\Entity\EntityManagerInterface;

Let's use the EntityTypeManagerInterface instead.

+  protected $supportedInterfaceOrClass = 'Drupal\node\NodeInterface';

For extra credit we can use NodeInterface::class now.

+  /**
+   * Constructs a new BookNormalizer object.
+   *
+   * @param \Drupal\rest\LinkManager\LinkManagerInterface $link_manager
+   *   Link manager.
+   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
+   *   Entity manager.
+   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+   *   Module handler.
+   */

The config factory is missing.

+    $this->allowedTypes = $config_factory->get('book.settings')->get('allowed_types');

Because the config can be updated while this service persists, it's discouraged to actually store config values in a service instead of the config factory. In this particular the actual use-case that would be broken is probably a bit far-fetched, but I would still consider it a best practice to inject the config factory itself, instead of just the values.

+    $embedded_cache = [];

In my opinion the fact that this variable is being used as a cache is not as interesting as what it actually contains. I would suggest naming it $embedded_nodes.

+        'bid' => self::PSUEDO_BOOK_FIELD_NAME,
+        'pid' => self::PSUEDO_PARENT_FIELD_NAME

Let's be nice to the extenders (TM) and use static::.

+        if (!empty($context['included_fields']) && !in_array($field_name, $context['included_fields'], TRUE)) {
+          // Only normalizing specific fields.
+          continue;
+        }
+        if (isset($entity->book[$field_name]) && $embedded_entity = $this->entityManager->getStorage('node')
+            ->load($entity->book[$field_name])
+        ) {

I think it's strange to have a guard condition (the 1st if) and then wrap the rest of the code in another if. Either we should make all conditions part of the guard or just put all the conditions in the second if. Because there's another if-else inside there, I would lean towards the former. Also, the second if is formatted strangely.

+          if (!isset($embedded_cache[$embedded_entity->id()])) {

$embedded_cache is never actually populated inside of this if. That also means that the else-branch will never be reached. Given that I think it would be nice to add a unit test for this class.

+            $langcode = isset($context['langcode']) ? $context['langcode'] : NULL;
+
+            // Normalize the target entity.
+            $embedded = $this->serializer->normalize($embedded_entity, $format, ['included_fields' => ['uuid']]);
+            $link = $embedded['_links']['self'];
+            // If the field is translatable, add the langcode to the link
+            // relation object. This does not indicate the language of the
+            // target entity.
+            if ($langcode) {
+              $embedded['lang'] = $link['lang'] = $langcode;
+            }

This looks similar to some code in EntityReferenceItemTrait. Does it make sense to add a EmbeddingNormalizerTrait (or similar)?

+              $embedded['lang'] = $link['lang'] = $langcode;

I realize this is copied from EntityReferenceItemNormalizer but I don't think our coding standards allow this.

+            $link['lang'] = $embedded['lang'];

Per the above this code will never actually be reached, but it seems as though this should be wrapped in a if (isset($embedded['lang'])).

+          if ($entity = $this->entityManager->loadEntityByUuid('node', $uuid)) {

This is a genuine question, I really don't know: Should we use the EntityResolver for this?

+      'book' => [
+        'bid' => 'new',
+      ],

Is setting the bid to new something we should handle in the book denormalization?

+        'bid' => $top_level_node_id,
+        'pid' => $top_level_node_id,

It's a bit unfortunate that we don't use different IDs here for testing purposes.

I cannot address your questions, but tried to fix everything else. For some reason the modified test fails on my machine, but I cannot successfully debug it, so let's see what the testbot finds.

And, of course, I forgot to change the issue status. Sorry!

I hadn't caught comment #12, so for now we're not using the proposed trait, but i'll be happy to implement it if the general direction of this patch is fine.

On a related note, could https://drupal.org/node/2419825 be related to this issue in some way/would it make sense to leverage the work done there?

Hey, any updates on this issue, serializing book still does not include book metadata.

This extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.

Un-assigning per Assigning ownership of a Drupal core issue.