feat: add timezone in Prefer header

[taimoorzaeem]

This should fix #2799.

[taimoorzaeem]

    context "check behaviour of Prefer: timezone=America/Los_Angeles" $
          it "should change timezone" $
            request methodGet "/timezone_values?select=*"
            [("Prefer", "handling=strict, timezone=America/Los_Angeles")]
            ""
            `shouldRespondWith`
            [json|[{"t":"2023-10-18T05:37:59.611-07:00"}, {"t":"2023-10-18T07:37:59.611-07:00"}, {"t":"2023-10-18T09:37:59.611-07:00"}]|]
            { matchStatus = 200
            , matchHeaders = [matchContentTypeJson] }

Strangely, this test is failing. But, if I run this test locally with curl it works fine.

$ postgrest-with-postgresql-15 postgrest-run

$ curl 'localhost:3000/timezone_values' -H "Prefer: handling=strict, timezone=America/Los_Angeles"

[{"t":"2023-10-18T05:37:59.611-07:00"}, 
 {"t":"2023-10-18T07:37:59.611-07:00"}, 
 {"t":"2023-10-18T09:37:59.611-07:00"}]

$ curl 'localhost:3000/timezone_values' -H "Prefer: handling=strict, timezone=America/New_York"

[{"t":"2023-10-18T08:37:59.611-04:00"}, 
 {"t":"2023-10-18T10:37:59.611-04:00"}, 
 {"t":"2023-10-18T12:37:59.611-04:00"}]

@steve-chavez @laurenceisla Is there something missing in the spec tests? Should I move these to io-tests.

[steve-chavez]

@steve-chavez @laurenceisla Is there something missing in the spec tests?

@taimoorzaeem Yes, the spec tests don't load the in-db config. This is done with AppState.reReadConfig, and only runs for the CLI.

To make it work, the resulting AppConfig from reReadConfig result would need to be cached and then used inside app config...

postgrest/test/spec/Main.hs

Lines 75 to 84 in 1e0b205

	-- cached schema cache so most tests run fast
	baseSchemaCache <- loadSchemaCache pool testCfg
	let
	-- For tests that run with the same refSchemaCache
	app config = do
	appState <- AppState.initWithPool pool config
	AppState.putPgVersion appState actualPgVersion
	AppState.putSchemaCache appState (Just baseSchemaCache)
	return ((), postgrest config appState $ pure ())

But that might require some refactoring. Could be done in other PR.

Should I move these to io-tests.

That's another option too. It would be the simplest one for now.

[taimoorzaeem]

@steve-chavez Hmm, the io-tests are running fine locally however, on CI they are failing. Strange!

[taimoorzaeem]

@steve-chavez @laurenceisla

def test_prefer_timezone_with_invalid_timezone(defaultenv):
    "timezone=America/XXX should set timezone to default timezone"

    env = {**defaultenv, "PGRST_DB_CONFIG": "true", "PGRST_JWT_SECRET": SECRET}

    headers = {
        "Prefer": "handling=strict, timezone=America/XXX",
    }

    with run(env=env) as postgrest:
        response = postgrest.session.get("/timezone_values", headers=headers)
        response_body = '[{"t":"2023-10-18T17:37:59.611+05:00"}, \n {"t":"2023-10-18T19:37:59.611+05:00"}, \n {"t":"2023-10-18T21:37:59.611+05:00"}]'
        assert response.text == response_body

This test sets the timezone to default timezone for invalid timezones. On my system, we have PKT timezone GMT+05 and so the tests are working fine locally. However, on github CI the default timezone is +00:00 apparently. The local timezone is different for every developer here and so this would cause the io-tests to fail locally.
How should I write this test case so it matches the response body irrespective of timezone?

[laurenceisla]

@taimoorzaeem

How should I write this test case so it matches the response body irrespective of timezone?

Edited: According to the docs, the environment variable used to set a db timezone is TZ . So we should fix the creation script instead:

postgrest/nix/tools/withTools.nix

Line 75 in b235227

PGTZ=UTC initdb --no-locale --encoding=UTF8 --nosync -U "${superuserRole}" --auth=trust \

Replace PGTZ=UTC with TZ=$PGTZ.

Looks like it was setting the default time zone of the host, with this change that should be fixed.

[laurenceisla]

Looking good. Just those refactors and it should be ready to merge.

BTW. Do you know why is it giving those IO errors?

[taimoorzaeem]

BTW. Do you know why is it giving those IO errors?

Nope, I don't know why IO-tests are failing on some pg versions.

[taimoorzaeem]

@taimoorzaeem Just tried this and it doesn't seem correct according to Prefer: handling=strict semantics, we should respond with an error in this case.

@steve-chavez Good catch! The only thing left here I guess is that these tests don't belong to io-tests. We should move them to spec-tests but that needs refactoring as you stated earlier which could be done in another PR.

[laurenceisla]

The only thing left here I guess is that these tests don't belong to io-tests. We should move them to spec-tests but that needs refactoring as you stated earlier which could be done in another PR.

I'm thinking about this, can the result of the queryTimezones be handled by the Schema Cache, instead of using the App Config as it is now? Or is there a reason against doing so? It could be alongside the queries in:

postgrest/src/PostgREST/SchemaCache.hs

Lines 132 to 133 in f10d413

	querySchemaCache :: AppConfig -> SQL.Transaction SchemaCache
	querySchemaCache AppConfig{..} = do

If I'm not mistaken, the Spec Tests should work OK then.

[steve-chavez]

I'm thinking about this, can the result of the queryTimezones be handled by the Schema Cache, instead of using the App Config as it is now? Or is there a reason against doing so?

@laurenceisla You're right. So the only reason to use the in-db config for db settings is to not expose them in the OpenAPI output (which uses the scheme cache). Some things like the roleSettings we cache in AppConfig should definitely not be exposed but the available timezones should.

---

@taimoorzaeem One more thing, the Preference-Applied header is not including the timezone currently:

curl localhost:3000/projects -H "Prefer: handling=strict, timezone=Europe/Berlin" -i
HTTP/1.1 200 OK
Preference-Applied: handling=strict

This might be easier to test once you move the io tests to spec tests.

[steve-chavez]

@taimoorzaeem Also a note. When moving the timezones to the Schema Cache, you can just leave the toJSON output as empty for now. Something like:

instance JSON.ToJSON SchemaCache where
  toJSON (SchemaCache tabs rels routs reps _ _) = JSON.object [
      "dbTables"          .= JSON.toJSON tabs
    , "dbRelationships"   .= JSON.toJSON rels
    , "dbRoutines"        .= JSON.toJSON routs
    , "dbRepresentations" .= JSON.toJSON reps
    , "dbMediaHandlers"   .= JSON.emptyArray
    , "dbTimezones"       .= JSON.emptyArray
    ]

We're working on a pure SQL generator for OpenAPI on https://github.com/PostgREST/postgrest-openapi/. So it's not worth the effort to do it on the Haskell side for now.

[steve-chavez]

Will DRY this a bit. I think the BS.drop 9/BS.take 9 can be removed.

[steve-chavez]

I get this locally too. I think the increased memory is because of the cached timezones (~1K).

[steve-chavez]

@taimoorzaeem Great work! 💯