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

Ensure the constructor arguments are passed to custom classes #3893

Merged
merged 1 commit into from
Mar 27, 2020

Conversation

duncan3dc
Copy link
Contributor

Q A
Type bug
BC Break no

Summary

Currently using FetchMode::CUSTOM_OBJECT with constructor arguments doesn't work as the constructor arguments are silently dropped in the statement wrapper.
I had a look around and couldn't find any suggestion this is intentional, it's fixed in master as a by-product of 82f369f so this PR proposes a fix for 2.*

@morozov
Copy link
Member

morozov commented Mar 18, 2020

This API is not portable and personally I'd rather drop it than support. See the details in #3070.

What makes you want to construct objects by DBAL instead of constructing them in the application?

@duncan3dc
Copy link
Contributor Author

I have an object that does not map to a table, I want to construct it from the result of a complex query. I couldn't see any Doctrine API to achieve this, but FetchMode::CUSTOM_OBJECT seems to work exactly as I need (populating the private properties of my object).

If there's another way then this can be closed but could you point me in the right direction please?

@morozov
Copy link
Member

morozov commented Mar 26, 2020

Why don't you fetch the data and instantiate the object using the new keyword? Why do you need the DBAL or the ORM to do that for you?

@duncan3dc
Copy link
Contributor Author

That would require a named constructor with many parameters. I wanted to leverage Doctrine functionality to populate private properties

@morozov
Copy link
Member

morozov commented Mar 26, 2020

I wanted to leverage Doctrine functionality to populate private properties.

So without being able to construct the object without DBAL, how are you going to unit-test it?

@duncan3dc
Copy link
Contributor Author

We integration test against a temporary database

@ostrolucky
Copy link
Member

I think such simple, ready bugfix should be accepted for 2.x. It's fixing bug after all and I can't immediately see what dev resources would it take to merge this, even if support is removed later. Removing support of it is concern of 3.x.

@morozov
Copy link
Member

morozov commented Mar 27, 2020

Thank you for the reminder, @ostrolucky. That was the plan.

@morozov morozov merged commit ced5d1a into doctrine:2.10.x Mar 27, 2020
@morozov
Copy link
Member

morozov commented Mar 27, 2020

Thank you @duncan3dc.

@morozov morozov self-assigned this Mar 27, 2020
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Apr 22, 2020
Release [2.10.2](https://github.com/doctrine/dbal/milestone/75)

2.10.2
======

- Total issues resolved: **4**
- Total pull requests resolved: **19**
- Total contributors: **10**

Improvement,Static Analysis
---------------------------

 - [3964: Mark every exception as immutable](doctrine#3964) thanks to @greg0ire

CI,Improvement,Static Analysis
------------------------------

 - [3961: Stop relying on the master version of Psalm](doctrine#3961) thanks to @greg0ire
 - [3951: Setup static analysis with Psalm](doctrine#3951) thanks to @greg0ire
 - [3799: Upgrade to PHPStan v0.12](doctrine#3799) thanks to @lcobucci

Improvement,Logging,Test Suite
------------------------------

 - [3957: Reworked LoggingTest to be able to test Statement::executeUpdate()](doctrine#3957) thanks to @morozov

CI,Code Style,Improvement,Strict Typing
---------------------------------------

 - [3955: Remove baseline](doctrine#3955) thanks to @greg0ire

Bug,SQLite,Schema Introspection,Schema Managers
-----------------------------------------------

 - [3937: Column comment incorrectly introspected on SQLite](doctrine#3937) thanks to @morozov

Bug,Documentation,Prepared Statements,Query
-------------------------------------------

 - [3896: Updated documentation for QueryBuilder::execute() return value type](doctrine#3896) thanks to @morozov

Bug,Prepared Statements
-----------------------

 - [3894: Make sure that the $types array has the same keys $params](doctrine#3894) thanks to @morozov
 - [3893: Ensure the constructor arguments are passed to custom classes](doctrine#3893) thanks to @duncan3dc
 - [3843: Fix unquoted stmt fragments backslash escaping](doctrine#3843) thanks to @morozov

Documentation,Improvement
-------------------------

 - [3886: Update readme](doctrine#3886) thanks to @greg0ire
 - [3834: Fix docblock typos in DriverManager docs](doctrine#3834) thanks to @CHItA

CI,Improvement,MariaDB,MySQL
----------------------------

 - [3884: Use Docker consistently](doctrine#3884) thanks to @greg0ire
 - [3478: Improve readiness probe stability for containerized databases on CI](doctrine#3478) thanks to @morozov

 - [3883: Fix broken build](doctrine#3883) thanks to @greg0ire

Bug,Documentation,Query,Query Limit/Offset Modification
-------------------------------------------------------

 - [3842: Fixed the QueryBuilder::setMaxResults() signature to accept NULL](doctrine#3842) thanks to @morozov

Bug,Query
---------

 - [3832: Fix JOIN with no condition bug](doctrine#3832) thanks to @BenMorel

Bug,PostgreSQL,Schema Introspection
-----------------------------------

 - [3821: &doctrine#91;pg&doctrine#93; fix getting table information if search&doctrine#95;path contains escaped schema name](doctrine#3821) thanks to @linniksa

Documentation,Improvement,Logging
---------------------------------

 - [3812: Fix DebugStack#queries docblock type](doctrine#3812) thanks to @ostrolucky

Bug,Regression,Schema
---------------------

 - [3790: fixed unqualified table name of fk constraints when using schemas](doctrine#3790) thanks to @stlrnz and @Alarich

# gpg: Signature made Mon Apr 20 19:59:36 2020
# gpg:                using DSA key 2C3A645671828132
# gpg: Can't check signature: public key not found

# Conflicts:
#	README.md
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants