From e1f4224bedc5db2dc03be985f60417e27ab789e1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Thu, 9 Jul 2020 11:28:09 +0200
Subject: [PATCH] [libcalamaresui] Fix slideshowAPI loading

In 022045ae05 a regression was introduced: if no *slideshowAPI*
is specified in the branding file, Calamares refuses to start, with
a YAML failure.

Before the refactoring, we had `YAML::Node doc` and looked up
the *slideshowAPI* in it with `doc["slideshowAPI"]`. After the
refactoring, we had `const YAML::Node& doc`. The `const` makes
all the difference:
 - subscripting a non-existent key in a mutable Node silently
   returns a Null node (and possibly inserts the key);
 - subscripting a non-existent key in a const Node returns an
   invalid or undefined node.

Calling IsNull() or IsScalar() on a Null node works: the functions
return a bool. Calling them on an invalid node throws an exception.

So in the **const** case, this code can throws an exception that it
doesn't in the non-const case:
    `doc[ "slideshowAPI" ].IsScalar()`

- Massage the code to check for validity before checking for scalar
- Add a `get()` that produces more useful exception types when
  looking up an invalid key
- Use `get()` to lookup the slideshow node just once.
---
 src/libcalamaresui/Branding.cpp | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/libcalamaresui/Branding.cpp b/src/libcalamaresui/Branding.cpp
index cf66097c9..4cfe7ea40 100644
--- a/src/libcalamaresui/Branding.cpp
+++ b/src/libcalamaresui/Branding.cpp
@@ -367,7 +367,7 @@ Branding::WindowDimension::isValid() const
 }
 
 
-/// @brief Guard against cases where the @p key doesn't exist in @p doc
+/// @brief Get a string (empty is @p key doesn't exist) from @p key in @p doc
 static inline QString
 getString( const YAML::Node& doc, const char* key )
 {
@@ -378,6 +378,18 @@ getString( const YAML::Node& doc, const char* key )
     return QString();
 }
 
+/// @brief Get a node (throws if @p key doesn't exist) from @p key in @p doc
+static inline YAML::Node
+get( const YAML::Node& doc, const char* key )
+{
+    auto r = doc[ key ];
+    if ( !r.IsDefined() )
+    {
+        throw YAML::KeyNotFound( YAML::Mark::null_mark(), std::string( key ) );
+    }
+    return r;
+}
+
 static inline void
 flavorAndSide( const YAML::Node& doc, const char* key, Branding::PanelFlavor& flavor, Branding::PanelSide& side )
 {
@@ -514,10 +526,11 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
 {
     QDir componentDir( componentDirectory() );
 
-    if ( doc[ "slideshow" ].IsSequence() )
+    auto slideshow = get( doc, "slideshow" );
+    if ( slideshow.IsSequence() )
     {
         QStringList slideShowPictures;
-        doc[ "slideshow" ] >> slideShowPictures;
+        slideshow >> slideShowPictures;
         for ( int i = 0; i < slideShowPictures.count(); ++i )
         {
             QString pathString = slideShowPictures[ i ];
@@ -535,9 +548,9 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
         m_slideshowAPI = -1;
     }
 #ifdef WITH_QML
-    else if ( doc[ "slideshow" ].IsScalar() )
+    else if ( slideshow.IsScalar() )
     {
-        QString slideshowPath = QString::fromStdString( doc[ "slideshow" ].as< std::string >() );
+        QString slideshowPath = QString::fromStdString( slideshow.as< std::string >() );
         QFileInfo slideshowFi( componentDir.absoluteFilePath( slideshowPath ) );
         if ( !slideshowFi.exists() || !slideshowFi.fileName().toLower().endsWith( ".qml" ) )
             bail( m_descriptorPath,
@@ -546,7 +559,9 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
         m_slideshowPath = slideshowFi.absoluteFilePath();
 
         // API choice is relevant for QML slideshow
-        int api = doc[ "slideshowAPI" ].IsScalar() ? doc[ "slideshowAPI" ].as< int >() : -1;
+        // TODO:3.3: use get(), make slideshowAPI required
+        int api
+            = ( doc[ "slideshowAPI" ] && doc[ "slideshowAPI" ].IsScalar() ) ? doc[ "slideshowAPI" ].as< int >() : -1;
         if ( ( api < 1 ) || ( api > 2 ) )
         {
             cWarning() << "Invalid or missing *slideshowAPI* in branding file.";
@@ -555,7 +570,7 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
         m_slideshowAPI = api;
     }
 #else
-    else if ( doc[ "slideshow" ].IsScalar() )
+    else if ( slideshow.IsScalar() )
     {
         cWarning() << "Invalid *slideshow* setting, must be list of images.";
     }