Skip to content

Add methode to define an offset on OrderId #208

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

Open
wants to merge 2 commits into
base: 2.x
Choose a base branch
from
Open

Add methode to define an offset on OrderId #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45b0f16
feat(RequestFactory, V4): Add methode to set define an offset on OrderId
Feb 13, 2025
fbf73ce
fix(CryptService): Remove useless offset calculation from character code
Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/EbicsClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1436,4 +1436,14 @@ public function changeKeyringPassword(string $newPassword): void

$this->keyring->setPassword($newPassword);
}

public function setOrderIdOffset(int $offset): void
{
$this->requestFactory->setOrderIdOffset($offset);
}

public function getOrderIdOffset(): int
{
return $this->requestFactory->getOrderIdOffset();
}
}
12 changes: 12 additions & 0 deletions src/Factories/RequestFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ abstract class RequestFactory
protected UserSignatureHandler $userSignatureHandler;
protected CryptService $cryptService;
protected ZipService $zipService;
protected int $orderIdOffset = 0;


public function __construct(
Bank $bank,
Expand Down Expand Up @@ -1020,6 +1022,16 @@ public function prepareStandardContext(RequestContext $requestContext = null): R
return $requestContext;
}

public function setOrderIdOffset(int $orderIdOffset): void
{
$this->orderIdOffset = $orderIdOffset;
}

public function getOrderIdOffset(): int
{
return $this->orderIdOffset;
}

abstract public function prepareDownloadContext(RequestContext $requestContext = null): RequestContext;

abstract public function prepareUploadContext(RequestContext $requestContext = null): RequestContext;
Expand Down
2 changes: 1 addition & 1 deletion src/Factories/RequestFactoryV24.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ protected function addOrderType(
$orderAttribute = OrderDetailsBuilder::ORDER_ATTRIBUTE_DZHNN;
}

$orderId = $this->cryptService->generateOrderId($this->user->getPartnerId());
$orderId = $this->cryptService->generateOrderId($this->user->getPartnerId(), $this->getOrderIdOffset());

return $orderDetailsBuilder
->addOrderType($orderType)
Expand Down
17 changes: 15 additions & 2 deletions src/Services/CryptService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,17 +450,30 @@
}

/**
* Generate order id from A000 to ZZZZ
* Generate order id from A000 to ZZZZ with offset.
* Unique value per partner for customer.
*
* @param string $partnerId
* @param int $offset
*
* @return string
*/
public function generateOrderId(string $partnerId): string
public function generateOrderId(string $partnerId, int $offset = 0): string
Copy link
Member

@andrew-svirin andrew-svirin Feb 13, 2025

Choose a reason for hiding this comment

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

I believe it will be Okay just to change logic for generateOrderId without option to impact on algo by pass offset.

Copy link
Author

@benjamin-si benjamin-si Feb 13, 2025

Choose a reason for hiding this comment

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

@andrew-svirin I'm not sure I understand, the idea would be to pass another value than the partner ID?

Copy link
Member

@andrew-svirin andrew-svirin Feb 13, 2025

Choose a reason for hiding this comment

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

I mean just change previous logic to a new one. If it works with your bank and If tests will pass, then it will single correct variant for generation OrderId and no need to adopt different algorithms for different banks.

Copy link
Author

@benjamin-si benjamin-si Feb 13, 2025

Choose a reason for hiding this comment

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

I see, but the purpose is to be able to change this number. The banks block the orderId in case of failure on a call INI for example. It's seem to be a general case.
To make another you have to change this orderID. I don't see how I could change the logic of the orderId without passing an offset.

Copy link
Member

@andrew-svirin andrew-svirin Feb 13, 2025

Choose a reason for hiding this comment

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

@benjamin-si would it work if you make default logic with offset?

Copy link
Author

@benjamin-si benjamin-si Feb 24, 2025

Choose a reason for hiding this comment

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

or maybe just manual define a orderId, it will be easier

Copy link
Member

@andrew-svirin andrew-svirin Feb 24, 2025

Choose a reason for hiding this comment

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

Maybe add to algorith dependency on current time? To be sure that next time it will generate a new value?

Copy link
Author

@benjamin-si benjamin-si Feb 24, 2025

Choose a reason for hiding this comment

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

Yes, but you need the same orderId on an INI and HIA call. If it is made separately, it could generate two different orderId.

Copy link
Member

@andrew-svirin andrew-svirin Feb 24, 2025

Choose a reason for hiding this comment

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

Hm, As I remember INI and HIA does not require to have he same orderId. But I will check it in specification.

Copy link
Member

@andrew-svirin andrew-svirin Feb 24, 2025

Choose a reason for hiding this comment

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

I found only this information. Take in account that if you abort transaction without acknowladge, then this orderId can be hanged for some time.

зображення

{
$hash = $this->hash($partnerId, 'crc32', false);

$hashToDesc = hexdec($hash);

$hasDeschWithOffset = $hashToDesc + $offset;

if ($hasDeschWithOffset > 0xffffffff) {
$hasDeschWithOffset = $hasDeschWithOffset - 0xffffffff;
}

$hash = dechex($hasDeschWithOffset);

Check failure on line 473 in src/Services/CryptService.php

View workflow job for this annotation

GitHub Actions / PHPStan 

Parameter #1 $num of function dechex expects int, float|int given.

$hash = str_pad($hash, 8, '0', STR_PAD_LEFT);

$decs = str_split($hash, 2);

$first = true;
Expand Down
Loading