Skip to content

Add a QueryBuilder based Mapper#9444

Merged
rullzer merged 4 commits into
masterfrom
techdep/noid/appframework_mapper_to_qb
May 14, 2018
Merged

Add a QueryBuilder based Mapper#9444
rullzer merged 4 commits into
masterfrom
techdep/noid/appframework_mapper_to_qb

Conversation

@rullzer
Copy link
Copy Markdown
Member

@rullzer rullzer commented May 10, 2018

I noticed when I wanted to log the queries we make that not all of them are logged. Then I remembered that the Mapper doesn't use the query builder yet. So I decided to add a new one.

  • Add new QBMapper
  • Deprecate old Mapper
  • Moved over DefaultTokenMapper

We should probably add tests.

@rullzer rullzer added enhancement 2. developing Work in progress technical debt 🧱 🤔🚀 labels May 10, 2018
@rullzer rullzer added this to the Nextcloud 14 milestone May 10, 2018
rullzer added 3 commits May 10, 2018 19:47
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the techdep/noid/appframework_mapper_to_qb branch from 1a4261e to 610c665 Compare May 10, 2018 17:47
@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2018

Codecov Report

Merging #9444 into master will decrease coverage by 0.03%.
The diff coverage is 6.66%.

@@             Coverage Diff              @@
##             master    #9444      +/-   ##
============================================
- Coverage     51.61%   51.57%   -0.04%     
- Complexity    25694    25712      +18     
============================================
  Files          1638     1639       +1     
  Lines         96318    96398      +80     
  Branches       1393     1393              
============================================
+ Hits          49712    49716       +4     
- Misses        46606    46682      +76
Impacted Files Coverage Δ Complexity Δ
...rivate/Authentication/Token/DefaultTokenMapper.php 100% <ø> (ø) 11 <0> (ø) ⬇️
lib/public/AppFramework/Db/Mapper.php 94.95% <ø> (-4.17%) 30 <0> (ø)
lib/public/AppFramework/Db/QBMapper.php 6.66% <6.66%> (ø) 18 <18> (?)
apps/files_trashbin/lib/Expiration.php 90.32% <0%> (-1.62%) 29% <0%> (ø)

Copy link
Copy Markdown
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks great

Copy link
Copy Markdown
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code makes sense 👍 - please add it to the docs.

$getter = 'get' . ucfirst($property);
$value = $entity->$getter();

$qb->setValue($column, $qb->createNamedParameter($value));
Copy link
Copy Markdown
Member

@MorrisJobke MorrisJobke May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlInjectionChecker Potential SQL injection detected - method: no child method

$getter = 'get' . ucfirst($property);
$value = $entity->$getter();

$qb->set($column, $qb->createNamedParameter($value));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlInjectionChecker Potential SQL injection detected - method: no child method

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 14, 2018
@rullzer rullzer merged commit 497a4fa into master May 14, 2018
@rullzer rullzer deleted the techdep/noid/appframework_mapper_to_qb branch May 14, 2018 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants