Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean warnings in persistent #1448

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/haskell.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
env:
CONFIG: "--enable-tests --enable-benchmarks"
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- uses: haskell/actions/setup@v2
id: setup-haskell-cabal
with:
Expand Down
9 changes: 3 additions & 6 deletions persistent/Database/Persist/Class/PersistEntity.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module Database.Persist.Class.PersistEntity
, Filter (..)
, FilterValue (..)
, BackendSpecificFilter
, Entity (.., Entity, entityKey, entityVal)
, Entity (..)

, recordName
, entityValues
Expand All @@ -39,8 +39,6 @@ module Database.Persist.Class.PersistEntity
, SafeToInsertErrorMessage
) where

import Data.Functor.Constant

import Data.Aeson
( FromJSON(..)
, ToJSON(..)
Expand All @@ -63,7 +61,6 @@ import qualified Data.Aeson.KeyMap as AM
import qualified Data.HashMap.Strict as AM
#endif

import GHC.Records
import Data.List.NonEmpty (NonEmpty(..))
import Data.Maybe (isJust)
import Data.Text (Text)
Expand Down Expand Up @@ -553,9 +550,9 @@ instance SymbolToField sym rec typ => IsLabel sym (EntityField rec typ) where
class SafeToInsert a where

type SafeToInsertErrorMessage a
= 'Text "The PersistEntity " ':<>: ShowType a ':<>: 'Text " does not have a default primary key."
= 'Text "The PersistEntity " ':<>: 'ShowType a ':<>: 'Text " does not have a default primary key."
':$$: 'Text "This means that 'insert' will fail with a database error."
':$$: 'Text "Please provide a default= clause inthe entity definition,"
':$$: 'Text "Please provide a default= clause in the entity definition,"
':$$: 'Text "or use 'insertKey' instead to provide one."

instance (TypeError (FunctionErrorMessage a b)) => SafeToInsert (a -> b)
Expand Down
3 changes: 2 additions & 1 deletion persistent/Database/Persist/Class/PersistField.hs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import qualified Data.Text.Encoding.Error as TERR
import qualified Data.Text.Lazy as TL
import qualified Data.Vector as V
import Data.Word (Word, Word8, Word16, Word32, Word64)
import Numeric.Natural (Natural)
import Numeric.Natural
import Text.Blaze.Html
import Text.Blaze.Html.Renderer.Text (renderHtml)
import GHC.TypeLits
Expand All @@ -44,6 +44,7 @@ import Data.Time (defaultTimeLocale)
import Data.Time.Clock.POSIX (posixSecondsToUTCTime)
#endif

import Prelude
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why import this? There is no NoImplicitPrelude pragma in there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a trick to get fewer warnings over time. By putting Prelude import last, you don't get a warning if you have import Data.Semigroup (Semigroup(..)) above. This is nice for reducing warnings when compiling with different versions of base where more and more things are added to Prelude.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, didn't know that. I skimmed over this PR, looks fine. It'd be good to get this in, so I can build on it.


-- | This class teaches Persistent how to take a custom type and marshal it to and from a 'PersistValue', allowing it to be stored in a database.
--
Expand Down
1 change: 1 addition & 0 deletions persistent/Database/Persist/Sql/Run.hs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE RecordWildCards #-}

module Database.Persist.Sql.Run where

import Control.Monad.IO.Unlift
Expand Down
8 changes: 8 additions & 0 deletions persistent/Database/Persist/SqlBackend/SqlPoolHooks.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ module Database.Persist.SqlBackend.SqlPoolHooks
, modifyRunAfter
, setRunAfter
, getRunOnException
, modifyRunOnException
, setRunOnException
Comment on lines +14 to +15
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, this warrants a minor bump!

)
where

Expand Down Expand Up @@ -83,9 +85,15 @@ setRunAfter hooks f = hooks { runAfter = f }
getRunOnException :: SqlPoolHooks m backend -> (backend -> Maybe IsolationLevel -> SomeException -> m ())
getRunOnException = runOnException

-- |
--
-- @since 2.14.5.0
modifyRunOnException :: SqlPoolHooks m backend -> ((backend -> Maybe IsolationLevel -> SomeException -> m ()) -> (backend -> Maybe IsolationLevel -> SomeException -> m ())) -> SqlPoolHooks m backend
modifyRunOnException hooks f = hooks { runOnException = f $ runOnException hooks }

-- |
--
-- @since 2.14.5.0
setRunOnException :: SqlPoolHooks m backend -> (backend -> Maybe IsolationLevel -> SomeException -> m ()) -> SqlPoolHooks m backend
setRunOnException hooks f = hooks { runOnException = f }

Expand Down
2 changes: 1 addition & 1 deletion persistent/persistent.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ test-suite test
hs-source-dirs:
test/

ghc-options: -Wall
ghc-options: -Wall -Wno-incomplete-uni-patterns

default-extensions: FlexibleContexts
, MultiParamTypeClasses
Expand Down
3 changes: 0 additions & 3 deletions persistent/test/Database/Persist/PersistValueSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@ module Database.Persist.PersistValueSpec where

import Test.Hspec
import Database.Persist.PersistValue
import Data.List.NonEmpty (NonEmpty(..), (<|))
import qualified Data.Text as T
import Test.Hspec
import Test.Hspec.QuickCheck
import Test.QuickCheck
import Data.Aeson
import qualified Data.ByteString.Char8 as BS8


spec :: Spec
spec = describe "PersistValueSpec" $ do
describe "PersistValue" $ do
Expand Down
49 changes: 28 additions & 21 deletions persistent/test/Database/Persist/QuasiSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -507,9 +507,9 @@ User

UniqueEmail emailFirst emailSecond
|]
let [user] = parse lowerCaseSettings definitions
let user = head $ parse lowerCaseSettings definitions
uniques = entityUniques (unboundEntityDef user)
[dbNames] = fmap snd . uniqueFields <$> uniques
dbNames = head $ fmap snd . uniqueFields <$> uniques
errMsg = unwords
[ "Unknown column in \"UniqueEmail\" constraint: \"emailSecond\""
, "possible fields: [\"name\",\"emailFirst\"]"
Expand All @@ -523,7 +523,7 @@ User
age Text
Unique some
|]
let [user] = parse lowerCaseSettings definitions
let user = head $ parse lowerCaseSettings definitions
evaluate (unboundPrimarySpec user) `shouldErrorWithMessage`
"invalid unique constraint on table[\"User\"] expecting an uppercase constraint name xs=[\"some\"]"

Expand All @@ -548,10 +548,10 @@ Notification
let
flippedFK (EntityNameHS entName) (ConstraintNameHS conName) =
conName <> entName
[_user, notification] =
parse (setPsToFKName flippedFK lowerCaseSettings) validDefinitions
[notificationForeignDef] =
unboundForeignDef <$> unboundForeignDefs notification
(_user, notification) =
listToTwoTuple $ parse (setPsToFKName flippedFK lowerCaseSettings) validDefinitions
notificationForeignDef =
head $ unboundForeignDef <$> unboundForeignDefs notification
foreignConstraintNameDBName notificationForeignDef
`shouldBe`
ConstraintNameDB "fk_noti_user_notification"
Expand All @@ -571,7 +571,8 @@ Notification
sentToSecond Text
Foreign User
|]
let [_user, notification] = parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) definitions
let (_user, notification) =
listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) definitions
mapM (evaluate . unboundForeignFields) (unboundForeignDefs notification)
`shouldErrorWithMessage`
"invalid foreign key constraint on table[\"Notification\"] expecting a lower case constraint name or a cascading action xs=[]"
Expand All @@ -591,7 +592,8 @@ Notification
sentToSecond Text
Foreign User fk_noti_user
|]
let [_user, notification] = parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) definitions
let (_user, notification) =
listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) definitions
mapM (evaluate . unboundForeignFields) (unboundForeignDefs notification)
`shouldErrorWithMessage`
"No fields on foreign reference."
Expand All @@ -611,7 +613,8 @@ Notification
sentToSecond Text
Foreign User fk_noti_user sentToFirst sentToSecond References emailFirst
|]
let [_user, notification] = parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) definitions
let (_user, notification) =
listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) definitions
mapM (evaluate . unboundForeignFields) (unboundForeignDefs notification)
`shouldErrorWithMessage`
"invalid foreign key constraint on table[\"Notification\"] Found 2 foreign fields but 1 parent fields"
Expand All @@ -631,7 +634,7 @@ Notification
sentToSecond Text
Foreign User OnDeleteCascade OnDeleteCascade
|]
let [_user, notification] = parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) definitions
let (_user, notification) = listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) definitions
mapM (evaluate . unboundForeignFields) (unboundForeignDefs notification)
`shouldErrorWithMessage`
"invalid foreign key constraint on table[\"Notification\"] found more than one OnDelete actions"
Expand All @@ -651,16 +654,16 @@ Notification
sentToSecond Text
Foreign User OnUpdateCascade OnUpdateCascade
|]
let [_user, notification] = parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) definitions
let (_user, notification) = listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) definitions
mapM (evaluate . unboundForeignFields) (unboundForeignDefs notification)
`shouldErrorWithMessage`
"invalid foreign key constraint on table[\"Notification\"] found more than one OnUpdate actions"

it "should allow you to enable snake cased foriegn keys via a preset configuration function" $ do
let [_user, notification] =
parse (setPsUseSnakeCaseForiegnKeys lowerCaseSettings) validDefinitions
[notificationForeignDef] =
unboundForeignDef <$> unboundForeignDefs notification
let (_user, notification) =
listToTwoTuple $ parse (setPsUseSnakeCaseForeignKeys lowerCaseSettings) validDefinitions
notificationForeignDef =
head $ unboundForeignDef <$> unboundForeignDefs notification
foreignConstraintNameDBName notificationForeignDef
`shouldBe`
ConstraintNameDB "notification_fk_noti_user"
Expand All @@ -676,7 +679,7 @@ CustomerTransfer
currencyCode CurrencyCode
uuid TransferUuid
|]
let [customerTransfer] = parse lowerCaseSettings tickedDefinition
let customerTransfer = head $ parse lowerCaseSettings tickedDefinition
let expectedType =
FTTypeCon Nothing "MoneyAmount" `FTApp` FTTypePromoted "Customer" `FTApp` FTTypePromoted "Debit"

Expand All @@ -696,7 +699,7 @@ WithFinite
one (Finite 1)
twenty (Labelled "twenty")
|]
let [withFinite] = parse lowerCaseSettings tickedDefinition
let withFinite = head $ parse lowerCaseSettings tickedDefinition

(simplifyField <$> unboundEntityFields withFinite) `shouldBe`
[ (FieldNameHS "one", FTApp (FTTypeCon Nothing "Finite") (FTLit (IntTypeLit 1)))
Expand Down Expand Up @@ -1131,7 +1134,7 @@ Baz
, " Extra2"
, " something"
]
let [subject] = parse lowerCaseSettings lines
let subject = head $ parse lowerCaseSettings lines
it "produces the right name" $ do
getUnboundEntityNameHS subject `shouldBe` EntityNameHS "Foo"
describe "unboundEntityFields" $ do
Expand All @@ -1155,7 +1158,7 @@ Baz
, ("Extra2", [["something"]])
]
describe "works with extra blocks" $ do
let [_, lowerCaseTable, idTable] =
let (_, lowerCaseTable, idTable) =
case parse lowerCaseSettings $ T.unlines
[ ""
, "IdTable"
Expand All @@ -1178,7 +1181,7 @@ Baz
, ""
] of
[a, b, c] ->
[a, b, c] :: [UnboundEntityDef]
( a, b, c )
xs ->
error
$ "Expected 3 elements in list, got: "
Expand Down Expand Up @@ -1227,3 +1230,7 @@ shouldErrorWithMessage action expectedMsg = do
msg `shouldBe` expectedMsg
_ ->
expectationFailure "Expected `error` to have been called"

listToTwoTuple :: HasCallStack => [a] -> (a, a)
listToTwoTuple [a, b] = (a, b)
listToTwoTuple xs = error $ "Expected two items in list exactly, got: " <> show (length xs)
6 changes: 3 additions & 3 deletions persistent/test/Database/Persist/THSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import Data.Functor.Identity (Identity(..))
import Data.Int
import qualified Data.List as List
import Data.Proxy
import Data.Text (Text, pack)
import Data.Text (pack)
import Data.Time
import GHC.Generics (Generic)
import System.Environment
Expand Down Expand Up @@ -205,9 +205,9 @@ spec = describe "THSpec" $ do
JsonEncodingSpec.spec
CommentSpec.spec
CompositeKeyStyleSpec.spec
RequireOnlyPersistImportSpec.spec

describe "TestDefaultKeyCol" $ do
let EntityIdField FieldDef{..} =
entityId (entityDef (Proxy @TestDefaultKeyCol))
it "should be a BackendKey SqlBackend" $ do
-- the purpose of this test is to verify that a custom Id column of
-- the form:
Expand Down