Return an Either from parse_xmlfid instead of a Maybe for better error reporting.

diff --git a/htsn.cabal b/htsn.cabal
index 55dca86e03ddd96e8e1aa34b04364ca6ad1d2116..8aa384ff1de925b0b61d3a36b0c760de69c42cf1 100644 (file)
--- a/htsn.cabal
+++ b/htsn.cabal
@@ -24,6 +24,7 @@ executable htsn
     filepath                    == 1.3.*,
     hslogger                    == 1.2.*,
     hxt                         == 9.3.*,
+    MissingH                    == 1.2.*,
     network                     == 2.4.*,
     tasty                       == 0.5.*,
     tasty-hunit                 == 0.4.*,
@@ -73,6 +74,7 @@ test-suite testsuite
     filepath                    == 1.3.*,
     hslogger                    == 1.2.*,
     hxt                         == 9.3.*,
+    MissingH                    == 1.2.*,
     network                     == 2.4.*,
     tasty                       == 0.5.*,
     tasty-hunit                 == 0.4.*,

diff --git a/src/Main.hs b/src/Main.hs
index f36e2ba502d743e2d9d9f2665d366df385730a16..4ebada07e858fb53d2ca983503ced14201a1fd22 100644 (file)
--- a/src/Main.hs
+++ b/src/Main.hs
@@ -55,29 +55,45 @@ import TSN.FeedHosts ( FeedHosts(..) )
 import TSN.Xml ( parse_xmlfid )
 
 
--- | Warning! This does not automatically append a newline. The output
--- is displayed/logged as-is, for, you know, debug purposes.
+-- | Display and log debug information. WARNING! This does not
+--   automatically append a newline. The output is displayed/logged
+--   as-is, for, you know, debug purposes.
 report_debug :: String -> IO ()
 report_debug s = do
   display_debug s
   log_debug s
 
+
+-- | Display and log an error condition. This will prefix the error
+--   with "ERROR: " when displaying (but not logging) it so that it
+--   stands out.
+--
 report_error :: String -> IO ()
 report_error s = do
   display_error $ "ERROR: " ++ s
   log_error s
 
+
+-- | Display and log an informational (status) message.
 report_info :: String -> IO ()
 report_info s = do
   display_info s
   log_info s
 
--- | Warning! This does not automatically append a newline.
+
+-- | A special case of report_debug for reporting the two bits of data
+--   that we sent to TSN: the username and password.
+--
 report_sent :: String -> IO ()
 report_sent s = do
   display_sent s
   log_debug s
 
+
+-- | Display and log a warning. This will prefix the warning with
+--   "WARNING: " when displaying (but not logging) it so that it
+--   stands out.
+--
 report_warning :: String -> IO ()
 report_warning s = do
   display_warning $ "WARNING: " ++ s
@@ -103,10 +119,9 @@ recv_line h = do
 --
 save_document :: Configuration -> String -> IO ()
 save_document cfg doc =
-  case maybe_path of
-    Nothing ->
-      report_error "Document missing XML_File_ID element."
-    Just path -> do
+  case either_path of
+    Left err -> report_error err
+    Right path -> do
       already_exists <- doesFileExist path
       when already_exists $ do
         let msg = "File " ++ path ++ " already exists, overwriting."
@@ -114,9 +129,10 @@ save_document cfg doc =
       writeFile path doc
       report_info $ "Wrote file: " ++ path ++ "."
   where
+    -- All the fmaps are because we're working inside a Maybe.
     xmlfid = fmap show (parse_xmlfid doc)
     filename = fmap (++ ".xml") xmlfid
-    maybe_path = fmap ((output_directory cfg) </>) filename
+    either_path = fmap ((output_directory cfg) </>) filename
 
 
 -- | Loop forever, writing the buffer to file whenever a </message>

diff --git a/src/TSN/Xml.hs b/src/TSN/Xml.hs
index 4c1123f0c71f5aa4b20794031b262a862363ccc7..9d83f58ba448c0a63a048c3329a28ea657b1234a 100644 (file)
--- a/src/TSN/Xml.hs
+++ b/src/TSN/Xml.hs
@@ -6,7 +6,7 @@ module TSN.Xml (
   xml_tests )
 where
 
-import Data.Maybe ( listToMaybe, mapMaybe )
+import Data.Either.Utils ( maybeToEither )
 import Test.Tasty ( TestTree, testGroup )
 import Test.Tasty.HUnit ( (@?=), Assertion, testCase )
 import Text.Read ( readMaybe )
@@ -19,11 +19,32 @@ import Text.XML.HXT.Core (
   runLA,
   xreadDoc )
 
+
 -- | A tiny parser written in HXT to extract the "XML_File_ID" element
---   from a document.
-parse_xmlfid :: String -> Maybe Integer
-parse_xmlfid =
-  listToMaybe . mapMaybe readMaybe . parse
+--   from a document. If we fail to parse an XML_File_ID, we return
+--   the reason wrapped in a 'Left' constructor. The reason should be
+--   one of two things:
+--
+--     1. No XML_File_ID elements were found.
+--
+--     2. An XML_File_ID element was found, but it could not be read
+--        into an Integer.
+--
+--   We use an Either rather than a Maybe because we do expect some
+--   non-integer XML_File_IDs. In the examples, you will see
+--   NHL_DepthChart_XML.XML with an XML_File_ID of "49618.61" and
+--   CFL_Boxscore_XML1.xml with an XML_File_ID of "R28916". According
+--   to Brijesh Patel of TSN, these are special category files and not
+--   part of the usual feed.
+--
+--   We want to report them differently, "just in case."
+--
+parse_xmlfid :: String -- ^ The XML Document
+             -> Either String Integer
+parse_xmlfid doc =
+  case parse_results of
+    []    -> Left "No XML_File_ID elements found."
+    (x:_) -> x
   where
     parse :: String -> [String]
     parse =
@@ -33,6 +54,14 @@ parse_xmlfid =
                >>> getChildren
                >>> getText)
 
+    read_either_integer :: String -> Either String Integer
+    read_either_integer s =
+      let msg = "Could not parse XML_File_ID" ++ s ++ " as an integer."
+      in
+        maybeToEither msg (readMaybe s)
+
+    elements = parse doc
+    parse_results = map read_either_integer elements
 
 
 -- * Tasty Tests
@@ -53,5 +82,7 @@ xml_file_id_tests =
     check xmlfid = do
       xml <- readFile ("test/xml/" ++ xmlfid ++ ".xml")
       let actual = parse_xmlfid xml
-      let expected = readMaybe xmlfid
+      -- The maybeToEither should always succeed here, so the error
+      -- message goes unused.
+      let expected = maybeToEither "derp" (readMaybe xmlfid)
       actual @?= expected