oatpp-postgresql icon indicating copy to clipboard operation
oatpp-postgresql copied to clipboard

Issue #24 - insert DTO type ENUM as string

Open bwengert79 opened this issue 1 year ago • 2 comments

Issue #24 When serializing a DTO that contains a DTO_FIELD whose type is Enum<xxx>AsString, the resulting string ultimately goes out of scope. This will result in a failure when trying to insert this type of value into a database.

The solution is to utilize Serializer::OutputData::databuffer. Before the enum string goes out of scope, allocate a dataBuffer and copy the value to it.

EnumAsStringTest is included as a unit test.

If you run the unit test before applying the Serializer change you can see the failure take place.

Regarding Oatpp 1.4.0 My environment is set up using Oatpp 1.4.0.
I updated oatpp-postresql to 1.4.0 to explore. I then discovered issue #24 and created the solution in the 1.4.0 environment.

bwengert79 avatar Oct 02 '24 21:10 bwengert79

Hey @bwengert79 , It's a great PR!

Let's try to run CI: Please change docker-compose to docker compose without '-' in two places https://github.com/oatpp/oatpp-postgresql/blob/master/azure-pipelines.yml#L16 Let's see if it builds

lganzzzo avatar Oct 04 '24 01:10 lganzzzo

Got past the docker compose issue. The build is now failing because it looks like a cmake version issue. "CMake 3.20 or higher is required. You are running version 3.11.1" I changed cmake_minimum_required to 3.20 in CMakeList.txt to match what's in the oatpp repo. Is oatpp-postgresql building in a different environment that oatpp?

bwengert79 avatar Oct 04 '24 13:10 bwengert79

Changing cmake min to 3.1 got us further in the build process. However, the next stumbling block is oatpp 140 is not available in the current CI environment. Perhaps this PR needs to wait for the CI environment to match that of oatpp 140.

bwengert79 avatar Oct 09 '24 09:10 bwengert79

Hey @bwengert79 ,

Yes, the current implementation of CI pipeline is outdated and isn't flexible. I'll merge this PR as is, and later will rebuild the CI pipeline.

lganzzzo avatar Oct 09 '24 15:10 lganzzzo