nomicflux / servant-opaleye-blog

Tutorial on setting up a blog backend with Servant and Opaleye
Other
16 stars 2 forks source link

Read & write data-types of different "shape" #3

Open saurabhnanda opened 8 years ago

saurabhnanda commented 8 years ago

Referring to https://github.com/nomicflux/servant-opaleye-blog/blob/lesson2_opaleye/README.md#step-55-set-up-conversions what if I need the write data-type (i.e. BlogPostWrite in this example) to not have id and timestamp? Is that possible?

Reason for this use-case: If I'm sharing my data-types between the server and the UI, I don't want my UI thinking that is can possibly send those fields down to the server. Only for the server to completely ignore them. I mean, isn't omitting those fields from BlogPostWrite the completely type-safe way to do it in the first place?

nomicflux commented 8 years ago

I'll have to look into that. You're right, creating a limited BlogPostWrite type would be the typesafe way to do it. But Opaleye is based heavily off of profunctors, which come into their own with aggregation queries in particular.

I think that it might be possible to set up a typeclass instead of a record, so that one could get the id, title, body, etc. using the same functions. bpID on BlogPostWrite would just return Nothing then, without BlogPostWrite needing a bpID field. Then one could set up Profunctors manually, through lmaps.

I'll work on a concrete example, but another solution would be to have a separate BlogPostEntry data type which could be shared between the server and UI, which would require a manual conversion to BlogPostWrite to ensure compilation. That would seem to be easier than messing with Profunctors, but now you've got me curious about the inner workings of Opaleye and how to mess with it.

tomjaguarpaw commented 8 years ago

what if I need the write data-type (i.e. BlogPostWrite in this example) to not have id and timestamp?

You can get an easy (but slightly unsightly) solution by replacing

type BlogPostWrite = BlogPost' (Maybe BlogPostID) String String Email (Maybe DateTime)

with

type BlogPostWrite = BlogPost' () String String Email ()

Then you have to explicitly initialise the "missing" record fields with () which is not ideal but is the simplest change to this particular example which does what you want.

saurabhnanda commented 8 years ago

Thanks for replying here @tomjaguarpaw I was not completely happy with using () types, but ran with it because it seemed pragmatic enough. However, it will lead to more boilerplate.

In the code given below, the createTenant function does not type-check because tenantTable expects a TenantPGWrite, but is being passed a NewTenant. Now, either I write more code to convert between NewTenant => TenantPGWrite, or I set up a separate tenantInsertionTable to accept NewTenant directly.

More. Boilerplate. Code. (Related to https://github.com/tomjaguarpaw/haskell-opaleye/issues/200 and https://github.com/tomjaguarpaw/haskell-opaleye/issues/196)

createTenant :: NewTenant -> AppM (Either TenantCreationError Tenant)
createTenant newTenant = do 
  conn <- askDbConnection
  tenant <- liftIO $ runInsertReturning conn tenantTable newTenant id
  return $ Right tenant

--
-- The data types
--
type TenantPGWrite = TenantPoly
  (Maybe (Column PGInt8)) -- key
  (Maybe (Column UTCTime)) -- createdAt
  (Maybe (Column UTCTime)) -- updatedAt
  (Column PGText) -- name
  (Column PGText) -- status
  (Column (Nullable PGInt8)) -- ownerId
  (Column PGText) -- backofficeDomain

type TenantPGRead = TenantPoly
  (Column PGInt8) -- key
  (Column UTCTime) -- createdAt
  (Column UTCTime) -- updatedAt
  (Column PGText) -- name
  (Column PGText) -- status
  (Column (Nullable PGInt8)) -- ownerId
  (Column PGText) -- backofficeDomain

type NewTenant = TenantPoly
  () -- key
  () -- createdAt
  () -- updatedAt
  (Column PGText) -- name
  () -- status
  (Column (Nullable PGInt8)) -- ownerId
  (Column PGText) -- backofficeDomain

type Tenant = TenantPoly
  TenantId -- key
  UTCTime -- createdAt
  UTCTime -- updatedAt
  Text -- name
  TenantStatus -- status
  (Maybe UserId) -- ownerId
  Text -- backofficeDomain

tenantTable :: Table TenantPGWrite TenantPGRead
tenantTable = Table "tenants" (pTenant Tenant{
                                  tenantKey = optional "id"
                                  ,tenantCreatedAt = optional "created_at"
                                  ,tenantUpdatedAt = optional "updated_at"
                                  ,tenantName = required "name"
                                  ,tenantStatus = required "status"
                                  ,tenantOwnerId = required "owner_id"
                                  ,tenantBackofficeDomain = required "backoffice_domain"
                                  })
saurabhnanda commented 8 years ago

I'm re-thinking this approach. Is it simpler to NOT export setters for tenantKey tenantCreatedAt and tenantUpdatedAt and call it a day? The JSON API can define something like a NewTenant type which has a different shape, but internally it converts it to the regular Tenant type.

tomjaguarpaw commented 8 years ago

TenantPGWrite has (Column PGText) -- status but NewTenant has () -- status. Is that intentional? If so, where does the status that you write come from?

saurabhnanda commented 8 years ago

TenantPGWrite has (Column PGText) -- status but NewTenant has () -- status. Is that intentional? If so, where does the status that you write come from?

@tomjaguarpaw when a new tenant is created, it needs to always be in the inactive state.

tomjaguarpaw commented 8 years ago

So when you write a new tenant it should always be written with its state "inactive"?

saurabhnanda commented 8 years ago

Yes.

On 11 Sep 2016 4:13 pm, "tomjaguarpaw" notifications@github.com wrote:

So when you write a new tenant it should always be written with its state "inactive"?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nomicflux/servant-opaleye-blog/issues/3#issuecomment-246173777, or mute the thread https://github.com/notifications/unsubscribe-auth/AABu0eW2O2G-IOIQzfQmBVzpNgTSn9qQks5qo9tJgaJpZM4J0lwM .

tomjaguarpaw commented 8 years ago

In that case I don't think there is any way to avoid boilerplate because there's no other way for the system to know what status new tenants should be given.