Performance optimization for import

This commit is contained in:
Constantin Graf
2024-06-10 17:03:55 +02:00
committed by Constantin Graf
parent 90480f3bb8
commit 0eef5ffcfa
8 changed files with 478 additions and 3 deletions

View File

@@ -12,6 +12,27 @@ use App\Models\TimeEntry;
class BillableRateService class BillableRateService
{ {
public function getBillableRateForTimeEntryWithGivenRelations(TimeEntry $timeEntry, ?ProjectMember $projectMember, ?Project $project, ?Member $member, ?Organization $organization): ?int
{
if (! $timeEntry->billable) {
return null;
}
if ($projectMember !== null && $projectMember->billable_rate !== null) {
return $projectMember->billable_rate;
}
if ($project !== null && $project->billable_rate !== null) {
return $project->billable_rate;
}
if ($member !== null && $member->billable_rate !== null) {
return $member->billable_rate;
}
if ($organization !== null && $organization->billable_rate !== null) {
return $organization->billable_rate;
}
return null;
}
public function getBillableRateForTimeEntry(TimeEntry $timeEntry): ?int public function getBillableRateForTimeEntry(TimeEntry $timeEntry): ?int
{ {
if (! $timeEntry->billable) { if (! $timeEntry->billable) {

View File

@@ -30,6 +30,16 @@ class ImportDatabaseHelper
*/ */
private ?array $mapIdentifierToKey = null; private ?array $mapIdentifierToKey = null;
/**
* @var array<string, TModel|null>|null
*/
private ?array $mapKeyToModel = null;
/**
* @var array<string, TModel|null>|null
*/
private ?array $mapIdentifierToModel = null;
/** /**
* @var array<string, string> * @var array<string, string>
*/ */
@@ -148,6 +158,47 @@ class ImportDatabaseHelper
} }
} }
/**
* @return TModel
*/
public function getModelById(string $id): ?Model
{
if ($this->mapKeyToModel === null) {
$this->mapKeyToModel = [];
}
if (isset($this->mapKeyToModel[$id])) {
return $this->mapKeyToModel[$id];
}
/** @var TModel|null $model */
$model = $this->getModelInstance()->find($id);
if ($model !== null) {
$this->mapKeyToModel[$id] = $model;
}
return $model;
}
/**
* @param array<string, mixed> $identifierData
* @return TModel|null
*/
public function getModel(array $identifierData): ?Model
{
if ($this->mapIdentifierToModel === null) {
$this->mapIdentifierToModel = [];
}
$hash = $this->getHash($identifierData);
if (isset($this->mapIdentifierToModel[$hash])) {
return $this->mapIdentifierToModel[$hash];
}
$model = $this->getModelInstance()->where($identifierData)->first();
if ($model !== null) {
$this->mapIdentifierToModel[$hash] = $model;
}
return $model;
}
/** /**
* @param array<string, mixed> $identifierData * @param array<string, mixed> $identifierData
* *

View File

@@ -64,6 +64,7 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
], [ ], [
'role' => Role::Placeholder->value, 'role' => Role::Placeholder->value,
]); ]);
$member = $this->memberImportHelper->getModelById($memberId);
$clientId = null; $clientId = null;
if ($record['Client'] !== '') { if ($record['Client'] !== '') {
$clientId = $this->clientImportHelper->getKey([ $clientId = $this->clientImportHelper->getKey([
@@ -72,6 +73,8 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
]); ]);
} }
$projectId = null; $projectId = null;
$project = null;
$projectMember = null;
if ($record['Project'] !== '') { if ($record['Project'] !== '') {
$projectId = $this->projectImportHelper->getKey([ $projectId = $this->projectImportHelper->getKey([
'name' => $record['Project'], 'name' => $record['Project'],
@@ -81,6 +84,11 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
'color' => $this->colorService->getRandomColor(), 'color' => $this->colorService->getRandomColor(),
'is_billable' => false, 'is_billable' => false,
]); ]);
$project = $this->projectImportHelper->getModelById($projectId);
$projectMember = $this->projectMemberImportHelper->getModel([
'project_id' => $projectId,
'member_id' => $memberId,
]);
} }
$taskId = null; $taskId = null;
if ($record['Task'] !== '') { if ($record['Task'] !== '') {
@@ -137,7 +145,13 @@ class ClockifyTimeEntriesImporter extends DefaultImporter
throw new ImportException('End date ("'.$record['End Date'].'") or time ("'.$record['End Time'].'") are invalid'); throw new ImportException('End date ("'.$record['End Date'].'") or time ("'.$record['End Time'].'") are invalid');
} }
$timeEntry->end = $end->utc(); $timeEntry->end = $end->utc();
$timeEntry->setComputedAttributeValue('billable_rate'); $timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$this->organization
);
$timeEntry->save(); $timeEntry->save();
$this->timeEntriesCreated++; $this->timeEntriesCreated++;
} }

View File

@@ -12,6 +12,7 @@ use App\Models\ProjectMember;
use App\Models\Tag; use App\Models\Tag;
use App\Models\Task; use App\Models\Task;
use App\Models\User; use App\Models\User;
use App\Service\BillableRateService;
use App\Service\ColorService; use App\Service\ColorService;
use App\Service\Import\ImportDatabaseHelper; use App\Service\Import\ImportDatabaseHelper;
use App\Service\TimezoneService; use App\Service\TimezoneService;
@@ -62,6 +63,8 @@ abstract class DefaultImporter implements ImporterContract
*/ */
protected ImportDatabaseHelper $projectMemberImportHelper; protected ImportDatabaseHelper $projectMemberImportHelper;
protected BillableRateService $billableRateService;
public function init(Organization $organization): void public function init(Organization $organization): void
{ {
$this->organization = $organization; $this->organization = $organization;
@@ -141,6 +144,7 @@ abstract class DefaultImporter implements ImporterContract
$this->timeEntriesCreated = 0; $this->timeEntriesCreated = 0;
$this->colorService = app(ColorService::class); $this->colorService = app(ColorService::class);
$this->timezoneService = app(TimezoneService::class); $this->timezoneService = app(TimezoneService::class);
$this->billableRateService = app(BillableRateService::class);
} }
#[\Override] #[\Override]

View File

@@ -64,6 +64,7 @@ class TogglTimeEntriesImporter extends DefaultImporter
], [ ], [
'role' => Role::Placeholder->value, 'role' => Role::Placeholder->value,
]); ]);
$member = $this->memberImportHelper->getModelById($memberId);
$clientId = null; $clientId = null;
if ($record['Client'] !== '') { if ($record['Client'] !== '') {
$clientId = $this->clientImportHelper->getKey([ $clientId = $this->clientImportHelper->getKey([
@@ -72,6 +73,8 @@ class TogglTimeEntriesImporter extends DefaultImporter
]); ]);
} }
$projectId = null; $projectId = null;
$project = null;
$projectMember = null;
if ($record['Project'] !== '') { if ($record['Project'] !== '') {
$projectId = $this->projectImportHelper->getKey([ $projectId = $this->projectImportHelper->getKey([
'name' => $record['Project'], 'name' => $record['Project'],
@@ -81,6 +84,11 @@ class TogglTimeEntriesImporter extends DefaultImporter
'is_billable' => false, 'is_billable' => false,
'color' => $this->colorService->getRandomColor(), 'color' => $this->colorService->getRandomColor(),
]); ]);
$project = $this->projectImportHelper->getModelById($projectId);
$projectMember = $this->projectMemberImportHelper->getModel([
'project_id' => $projectId,
'member_id' => $memberId,
]);
} }
$taskId = null; $taskId = null;
if ($record['Task'] !== '') { if ($record['Task'] !== '') {
@@ -123,7 +131,13 @@ class TogglTimeEntriesImporter extends DefaultImporter
throw new ImportException('End date ("'.$record['End date'].'") or time ("'.$record['End time'].'") are invalid'); throw new ImportException('End date ("'.$record['End date'].'") or time ("'.$record['End time'].'") are invalid');
} }
$timeEntry->end = $end->utc(); $timeEntry->end = $end->utc();
$timeEntry->setComputedAttributeValue('billable_rate'); $timeEntry->billable_rate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$this->organization
);
$timeEntry->save(); $timeEntry->save();
$this->timeEntriesCreated++; $this->timeEntriesCreated++;
} }

View File

@@ -185,7 +185,7 @@ return [
'watch' => [ 'watch' => [
'app', 'app',
'bootstrap', 'bootstrap',
'config', 'config/**/*.php',
'database/**/*.php', 'database/**/*.php',
'public/**/*.php', 'public/**/*.php',
'resources/**/*.php', 'resources/**/*.php',

View File

@@ -257,4 +257,286 @@ class BillableRateServiceTest extends TestCase
// Assert // Assert
$this->assertSame(null, $billableRate); $this->assertSame(null, $billableRate);
} }
public function test_billable_rate_with_given_relations_returns_null_if_not_billable(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => 3003,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => 4004,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => false,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(null, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_project_member_rate_as_first_priority(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => 3003,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => 4004,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(4004, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_project_rate_as_second_priority_using_null_values_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => 3003,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => null,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(3003, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_project_rate_as_second_priority_using_non_existing_entities_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => 3003,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
null,
$project,
$member,
$organization
);
// Assert
$this->assertSame(3003, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_organization_member_rate_as_third_priority_using_null_values_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => null,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => null,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(2002, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_organization_member_rate_as_third_priority_using_non_existing_entities_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => 2002,
]);
$timeEntry = TimeEntry::factory()->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
null,
null,
$member,
$organization
);
// Assert
$this->assertSame(2002, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_organization_rate_as_fourth_priority_using_null_values_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => null,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => null,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => null,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(1001, $billableRate);
}
public function test_billable_rate_with_given_relations_uses_organization_rate_as_fourth_priority_using_non_existing_entities_before(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => 1001,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => null,
]);
$timeEntry = TimeEntry::factory()->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
null,
null,
$member,
$organization
);
// Assert
$this->assertSame(1001, $billableRate);
}
public function test_billable_rate_with_given_relations_is_null_if_billable_rate_on_all_levels_are_null(): void
{
// Arrange
$organization = Organization::factory()->create([
'billable_rate' => null,
]);
$user = User::factory()->create();
$member = Member::factory()->forOrganization($organization)->forUser($user)->create([
'billable_rate' => null,
]);
$project = Project::factory()->forOrganization($organization)->create([
'billable_rate' => null,
]);
$projectMember = ProjectMember::factory()->forMember($member)->forProject($project)->create([
'billable_rate' => null,
]);
$timeEntry = TimeEntry::factory()->forProject($project)->forMember($member)->forOrganization($organization)->create([
'billable' => true,
]);
// Act
$billableRate = $this->billableRateService->getBillableRateForTimeEntryWithGivenRelations(
$timeEntry,
$projectMember,
$project,
$member,
$organization
);
// Assert
$this->assertSame(null, $billableRate);
}
} }

View File

@@ -9,6 +9,7 @@ use App\Models\Project;
use App\Models\User; use App\Models\User;
use App\Service\Import\ImportDatabaseHelper; use App\Service\Import\ImportDatabaseHelper;
use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Str;
use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesClass;
use Tests\TestCase; use Tests\TestCase;
@@ -140,4 +141,92 @@ class ImportDatabaseHelperTest extends TestCase
$this->assertContains($externalIdentifier1, $externalKeys); $this->assertContains($externalIdentifier1, $externalKeys);
$this->assertContains($externalIdentifier2, $externalKeys); $this->assertContains($externalIdentifier2, $externalKeys);
} }
public function test_get_model_by_identifier_returns_model_for_identifier(): void
{
// Arrange
$user = User::factory()->create();
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
// Act
$model = $helper->getModel([
'email' => $user->email,
]);
// Assert
$this->assertSame($user->getKey(), $model->getKey());
}
public function test_get_model_by_identifier_returns_null_for_not_existing_identifier(): void
{
// Arrange
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
// Act
$model = $helper->getModel([
'email' => '',
]);
// Assert
$this->assertNull($model);
}
public function test_get_model_by_identifier_caches_result(): void
{
// Arrange
$user = User::factory()->create();
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
$helper->getModel([
'email' => $user->email,
]);
$user->delete();
// Act
$model1 = $helper->getModel([
'email' => $user->email,
]);
// Assert
$this->assertSame($user->getKey(), $model1->getKey());
}
public function test_get_model_by_id_returns_model_for_id(): void
{
// Arrange
$user = User::factory()->create();
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
// Act
$model = $helper->getModelById($user->getKey());
// Assert
$this->assertSame($user->getKey(), $model->getKey());
}
public function test_get_model_by_id_returns_null_for_not_existing_id(): void
{
// Arrange
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
// Act
$model = $helper->getModelById(Str::uuid()->toString());
// Assert
$this->assertNull($model);
}
public function test_get_model_by_id_caches_result(): void
{
// Arrange
$user = User::factory()->create();
$helper = new ImportDatabaseHelper(User::class, ['email'], true);
$helper->getModelById($user->getKey());
$user->delete();
// Act
$model1 = $helper->getModelById($user->getKey());
// Assert
$this->assertSame($user->getKey(), $model1->getKey());
}
} }