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

Issue #24 - insert DTO type ENUM as string #29

Merged
merged 9 commits into from
Oct 9, 2024

Conversation

bwengert79
Copy link
Contributor

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.

@lganzzzo
Copy link
Member

lganzzzo commented Oct 4, 2024

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

Version two of the Docker Compose is invoked via docker compose
Removed hyphen from docker-compose
@bwengert79
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@lganzzzo
Copy link
Member

lganzzzo commented Oct 9, 2024

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 lganzzzo merged commit 27031df into oatpp:master Oct 9, 2024
1 check failed
@bwengert79 bwengert79 deleted the contribute branch October 9, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants