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.

Sign up for GitHub

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

Jump to bottom

Fix webapi returns variables in JSON using snake case and I need came… #31001

Open
wants to merge 4 commits into
base: 2.4-develop
Choose a base branch
from
+27 −2
Open

Fix webapi returns variables in JSON using snake case and I need came… #31001

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ae193d
Fix webapi returns variables in JSON using snake case and I need came…
jayankaghosh Nov 20, 2020
f2bdb50
Change mapObjectKey to private
jayankaghosh Nov 24, 2020
f6d4cbe
Encapsulate ltrim inside getKeyByObjectType function
jayankaghosh Nov 24, 2020
5fef144
Encapsulate ltrim inside getKeyByObjectType function
jayankaghosh Nov 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 26 additions & 2 deletions lib/internal/Magento/Framework/Reflection/DataObjectProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,36 @@ class DataObjectProcessor
*/
private $processors;

/**
* @var array
*/
private $objectKeyMap;

/**
* @param MethodsMap $methodsMapProcessor
* @param TypeCaster $typeCaster
* @param FieldNamer $fieldNamer
* @param CustomAttributesProcessor $customAttributesProcessor
* @param ExtensionAttributesProcessor $extensionAttributesProcessor
* @param array $processors
* @param array $objectKeyMap
*/
public function __construct(
MethodsMap $methodsMapProcessor,
TypeCaster $typeCaster,
FieldNamer $fieldNamer,
CustomAttributesProcessor $customAttributesProcessor,
ExtensionAttributesProcessor $extensionAttributesProcessor,
array $processors = []
array $processors = [],
array $objectKeyMap = []
) {
$this->methodsMapProcessor = $methodsMapProcessor;
$this->typeCaster = $typeCaster;
$this->fieldNamer = $fieldNamer;
$this->extensionAttributesProcessor = $extensionAttributesProcessor;
$this->customAttributesProcessor = $customAttributesProcessor;
$this->processors = $processors;
$this->objectKeyMap = $objectKeyMap;
}

/**
Expand Down Expand Up @@ -128,7 +136,7 @@ public function buildOutputDataArray($dataObject, $dataObjectType)
}
}

$outputData[$key] = $value;
$outputData[$this->mapObjectKey($key, ltrim($dataObjectType, '\\'))] = $value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move ltrim inside the mapObjectKey method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could. But that wouldn't make any difference to the flow right? The mapObjectKey should only be responsible for mapping the object keys. Which is why I had kept the ltrim (to include any class names with preceding slashes given in the di.xml as well) outside of it

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue with it that it might not find the correct key, if in the client code we'll not do ltrim.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I understand. The mapObjectKey can't be called from any other class. It'll just be called from the buildOutputDataArray function, and we have done the ltrim there

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. but in case if someone will adjust this code a bit later or will call it in the another method - this issue will appear. It's better to incapsulate this logic inside the method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Okay. In that case I think it's better that we name the function something other than mapObjectKey?

Copy link
Contributor

Choose a reason for hiding this comment

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

getKeyByObjectType?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guess that'll work

}

$outputData = $this->changeOutputArray($dataObject, $outputData);
Expand All @@ -153,4 +161,20 @@ private function changeOutputArray($dataObject, array $outputData): array

return $outputData;
}

/**
* @param string $key
* @param string $dataObjectType
* @return string
*/
protected function mapObjectKey(string $key, string $dataObjectType): string
jayankaghosh marked this conversation as resolved.
Show resolved Hide resolved
{
if (
array_key_exists($dataObjectType, $this->objectKeyMap) &&
array_key_exists($key, $this->objectKeyMap[$dataObjectType])
) {
$key = $this->objectKeyMap[$dataObjectType][$key];
}
return $key;
}
}