diff --git a/examples/regression_jbeam/no-beams-repro.jbeam b/examples/regression_jbeam/no-beams-repro.jbeam new file mode 100644 index 00000000..2c38b8a5 --- /dev/null +++ b/examples/regression_jbeam/no-beams-repro.jbeam @@ -0,0 +1,18 @@ +{ +"testpart":{ + "nodes":[ + ["id", "posX", "posY", "posZ"], + // Synthetic regression-test fixture, not vetted by the jbeam + // maintainer and not intended as a demo/example. + // + // Nodes and nothing else. A jbeam file is not obliged to have a beams + // section, and classifying, sorting and renaming need no beams at all. + // Only support classification does, and its answer here is simply that + // there are none. + ["nl0", 0.9, -1.0, 0.1], + ["nl1", 0.9, 0.0, 0.1], + ["nr2", -0.9, -1.0, 0.1], + ["nr3", -0.9, 0.0, 0.1], + ], +}, +} diff --git a/src-extra/transformation/JbeamEdit/Transformation.hs b/src-extra/transformation/JbeamEdit/Transformation.hs index 2f4f5e9c..203415af 100644 --- a/src-extra/transformation/JbeamEdit/Transformation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation.hs @@ -227,9 +227,9 @@ moveVerticesInVertexForest triangleVertexNames topNode newNames tfCfg vertexTree Right movableVertices' -> let groupedVertices = M.fromListWith (++) movableVertices' in do - (badBeamNodes, conns) <- - vertexConns (maxSupportCoordinates tfCfg) topNode groupedVertices - let (supportForest, nonSupportVertices) = + let (badBeamNodes, conns) = + vertexConns (maxSupportCoordinates tfCfg) topNode groupedVertices + (supportForest, nonSupportVertices) = moveSupportVertices triangleVertexNames newNames tfCfg conns groupedVertices newForest <- foldM diff --git a/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs b/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs index a39d4957..42bd1a9d 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/BeamExtraction.hs @@ -1,5 +1,7 @@ module JbeamEdit.Transformation.BeamExtraction (vertexConns, possiblyBeam, extractBeams, extractBeamsWithMeta, beamInKnownSet) where +import Data.Bool (bool) +import Data.Either (fromRight) import Data.List (genericTake, sortOn) import Data.Map (Map) import Data.Map qualified as M @@ -49,10 +51,11 @@ extractBeamFromArray sectionMeta vec maybeObject n@(Object _) = Just n maybeObject _ = Nothing -extractBeams :: Node -> Either Text (Vector Node) +extractBeams :: Node -> Vector Node extractBeams topNode = - NP.queryNodes beamQuery topNode - >>= NP.expectArray beamQuery + fromRight + V.empty + (NP.queryNodes beamQuery topNode >>= NP.expectArray beamQuery) extractBeamsWithMeta :: Vector Node -> ([Node], [Beam]) extractBeamsWithMeta = go M.empty [] [] . V.toList @@ -73,9 +76,8 @@ vertexConns :: Natural -> Node -> Map VertexTreeType [AnnotatedVertex] - -> Either Text ([Node], VertexConnMap) -vertexConns maxSupport topNode vsPerType = - go <$> extractBeams topNode + -> ([Node], VertexConnMap) +vertexConns maxSupport topNode vsPerType = go (extractBeams topNode) where knownNodeNames = S.fromList $ concatMap (map anVertexName) vsPerType go beamNodes = @@ -103,7 +105,7 @@ vertexConns maxSupport topNode vsPerType = | (t, vs) <- M.toList topVerticesPerType , (v, c) <- vs ] - in (badNodes, vertexConnMap) + in bool (badNodes, vertexConnMap) (badNodes, M.empty) (null beamNodes) beamInKnownSet :: Set Text -> Beam -> Bool beamInKnownSet known (Beam (BeamPair a b) _) = diff --git a/src-extra/transformation/JbeamEdit/Transformation/BeamValidation.hs b/src-extra/transformation/JbeamEdit/Transformation/BeamValidation.hs index e4e34df3..d4502235 100644 --- a/src-extra/transformation/JbeamEdit/Transformation/BeamValidation.hs +++ b/src-extra/transformation/JbeamEdit/Transformation/BeamValidation.hs @@ -14,7 +14,6 @@ import Data.Set (Set) import Data.Set qualified as S import Data.Text (Text) import Data.Text qualified as T -import Data.Vector qualified as V import JbeamEdit.Core.Node (Node) import JbeamEdit.IOUtils (humanJoin, putErrorStringLn, tryReadFile) import JbeamEdit.Parsing.Jbeam (parseNodes) @@ -56,11 +55,7 @@ extractVertexNames node = Right (_, _, vf) -> Right (allVerticesInForest vf) extractFileBeams :: Node -> [Beam] -extractFileBeams node = - case extractBeams node of - Right bs - | not (V.null bs) -> snd (extractBeamsWithMeta bs) - _ -> [] +extractFileBeams topNode = snd . extractBeamsWithMeta $ extractBeams topNode findInvalidRefs :: Set Text -> [Beam] -> [(Text, Text, Set Text)] findInvalidRefs allVertexNames beams = diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index 7adc9011..5d3fde82 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -162,6 +162,7 @@ main = hspec $ do letterEndingNodesSpec ySortingBandingSpec xColumnSortingSpec + noBeamsSpec metadataAcrossTreesSpec metadataPreservedSpec vertexTextSpec diff --git a/test-extra/transformation/Spec/Helpers.hs b/test-extra/transformation/Spec/Helpers.hs index c5918c22..31b472d0 100644 --- a/test-extra/transformation/Spec/Helpers.hs +++ b/test-extra/transformation/Spec/Helpers.hs @@ -18,7 +18,11 @@ import Data.Text (Text) import Data.Text qualified as T import Data.Vector qualified as V import GHC.IsList (fromList) -import JbeamEdit.Core.Node (Node (..), NumberValue (..), expectArray) +import JbeamEdit.Core.Node ( + Node (..), + NumberValue (..), + expectArray, + ) import JbeamEdit.Core.NodePath qualified as NP import JbeamEdit.IOUtils (tryReadFile) import JbeamEdit.Parsing.Jbeam (parseNodes) diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index 0c7ffcc4..51b4a326 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -10,15 +10,30 @@ module Spec.Regression ( metadataAcrossTreesSpec, metadataPreservedSpec, xColumnSortingSpec, + noBeamsSpec, vertexTextSpec, ) where import Data.List (sort) +import Data.List.NonEmpty qualified as NE import Data.Map qualified as M import Data.Set qualified as S +import Data.Text (Text) import Data.Text qualified as T +import GHC.IsList (toList) +import JbeamEdit.Core.Node (Node) import JbeamEdit.Transformation +import JbeamEdit.Transformation.BeamExtraction (vertexConns) import JbeamEdit.Transformation.Config +import JbeamEdit.Transformation.Types ( + AnnotatedVertex (..), + VertexTree (..), + ) +import JbeamEdit.Transformation.VertexExtraction ( + determineGroup', + getVertexForest, + verticesQuery, + ) import Spec.Helpers import Test.Hspec @@ -223,6 +238,52 @@ xColumnSortingSpec = Left err -> expectationFailure ("transform failed: " ++ T.unpack err) Right (_, _, _, resultNode) -> assert resultNode +{- | A jbeam file is not obliged to have a beams section. Classifying, sorting +and renaming need none: only support classification reads beams, and its +answer without them is that there are no support nodes. Issue #229. + +`transform` instead fails the whole file, and the tool still exits 0, so a +run over a directory leaves such files untouched without saying why. +-} +noBeamsFixture :: FilePath +noBeamsFixture = "examples/regression_jbeam/no-beams-repro.jbeam" + +noBeamsSpec :: Spec +noBeamsSpec = + describe "a file with no beams section" $ do + it "is transformed, keeping every node" $ do + topNode <- parseJbeamFile noBeamsFixture + case transform M.empty newTransformationConfig topNode of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right (_, _, _, resultNode) -> + length (vertexCoordinates resultNode) `shouldBe` 4 + + it "counts a connection for every beamed vertex when there are beams" $ do + topNode <- parseJbeamFile supportRenameIdempotencyFixture + connectionCounts topNode + `shouldBe` Right [("nl0", 3), ("nl10", 3), ("nl20", 3)] + + it "counts nothing at all when there are none" $ do + topNode <- parseJbeamFile noBeamsFixture + connectionCounts topNode `shouldBe` Right [] + +{- | Grouping the vertices by tree type is what `transform` does before it +asks for the counts, and is repeated here because the wrapper it uses is +internal. +-} +connectionCounts :: Node -> Either Text [(Text, Int)] +connectionCounts topNode = do + (_, _, forest) <- getVertexForest brks verticesQuery topNode + let annotated = + concatMap (concatMap (NE.toList . tAnnotatedVertices . snd) . toList) forest + grouped <- M.fromListWith (++) <$> mapM withGroup annotated + let (_, conns) = + vertexConns (maxSupportCoordinates newTransformationConfig) topNode grouped + pure (sort [(name, count) | (name, (_, count)) <- M.toList conns]) + where + brks = xGroupBreakpoints newTransformationConfig + withGroup av = (,[av]) <$> determineGroup' brks (aVertex av) + {- | The transformation reorders and renames, and never changes a coordinate, so a number has to come back out spelled the way the file wrote it. It used to be read as a value and written back from that value, which dropped the point