url cleanup upgrade #148

shay123g · 2021-03-13T16:29:24Z

Closes #16

codecov · 2021-03-13T16:29:27Z

Codecov Report

Merging #148 (75c9a8f) into release/0.2.0 (5afc392) will increase coverage by 0.51%.
The diff coverage is 25.92%.

@@                Coverage Diff                @@
##           release/0.2.0     #148      +/-   ##
=================================================
+ Coverage          18.11%   18.63%   +0.51%     
=================================================
  Files                 36       36              
  Lines               1325     1358      +33     
  Branches              46       47       +1     
=================================================
+ Hits                 240      253      +13     
- Misses              1057     1077      +20     
  Partials              28       28

Impacted Files	Coverage Δ
src/config/index.ts	`0.00% <0.00%> (ø)`
src/config/types.ts	`0.00% <0.00%> (ø)`
src/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/inMemory/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/relational/index.ts	`0.00% <0.00%> (ø)`
src/services/storage/drivers/relational/types.ts	`0.00% <0.00%> (ø)`
src/services/storage/index.ts	`0.00% <0.00%> (ø)`
src/config/validate.ts	`91.52% <75.00%> (-1.21%)`	⬇️
src/config/__test__/helpers.ts	`100.00% <100.00%> (ø)`
src/config/normalize.ts	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5afc392...75c9a8f. Read the comment docs.

SnirShechter · 2021-03-13T17:45:03Z

@shay123g

That's only for the InMemory driver. What about the Relational driver?

Also, we've talked about a possible flag in the config to specify whether to delete from created_at date or from last_used date.

shay123g · 2021-03-13T18:16:50Z

@shay123g

That's only for the InMemory driver. What about the Relational driver?

Also, we've talked about a possible flag in the config to specify whether to delete from created_at date or from last_used date.

I will fix
Relational - @Danielg212 still working on it, he need to complete the fix for deleteOverdue

into release/0.2.0 � Conflicts: � src/services/storage/drivers/inMemory/index.ts

shay123g · 2021-03-14T12:18:08Z

@SnirShechter pls check

.env.development

src/services/storage/drivers/inMemory/index.ts

Co-authored-by: lesagi <[email protected]>

lesagi

nothing crucial, everything looks fine
the .env I changed to the same format of choosing one option out of many as the 'storage' has

in the relativeDate, I'm trying to use const as much as I can, but that's just a general preference, you can ignore it if you think it's less readable

shay123g · 2021-03-16T14:14:23Z

@lesagi I already accept your changes and commited them.. thanks for the explanation though. I asked for review again to check those changes and approve\request more changes

SnirShechter · 2021-03-13T17:45:45Z

src/services/storage/drivers/inMemory/index.ts

@@ -43,8 +43,8 @@ export class InMemoryStorage implements StorageDriver {
 let deletedCount = 0


See the comment in the PR

src/config/validate.ts

SnirShechter · 2021-03-19T09:25:37Z

src/services/storage/drivers/inMemory/index.ts

-import type { StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation } from '../../types/url.js'
+import type {StoredUrl, UrlWithInformation, UrlRequestData, UrlInformation} from '../../types/url.js'
+import {logger} from "../../../logger/logger";
+import {getRawConfig} from "../../../../config/__test__/helpers";


It's better to pass the expireFrom via the driver configurations instead of importing the config.

Also, you imported functions from the tests.

SnirShechter · 2021-03-19T09:26:02Z

src/services/storage/drivers/inMemory/index.ts

- const updatedAt = new Date(storedUrl.updatedAt).getTime()
- if (updatedAt <= deleteBefore) {
+ const relativeDate = new Date(deleteFrom === 'update' ? storedUrl.updatedAt : storedUrl.createdAt).getTime()
+ if (relativeDate <= deleteBefore) {
 this.storage.data.urls.delete(storedUrl.id)
 deletedCount++
 }


Where's the Relational implementation?

SnirShechter · 2021-03-19T09:27:29Z

src/config/validate.ts

@@ -101,6 +102,13 @@ function validateUrlLifetime(urlLifetime: string): void {
 }
 }

+function validateUrlExpireFrom(urlExpire: string): void {
+ logger.debug(`Start validateUrlExpireFrom with ${urlExpire}`)
+ if (!urlExpire || (urlExpire != 'create' && urlExpire != 'update')) {


The feature we talked about is "expire from either 'create' or 'last used' ". You should add the 'last used'.

Co-authored-by: Snir Shechter <[email protected]>

into 16 � Conflicts: � src/config/types.ts � src/index.ts � src/services/storage/drivers/inMemory/index.ts � src/services/storage/drivers/relational/index.ts

shay123g · 2021-03-29T11:56:00Z

@SnirShechter conflicts resolved

#16

756fc02

shay123g requested review from lesagi, Danielg212 and SnirShechter March 13, 2021 16:29

Shay.Gazit added 3 commits March 13, 2021 21:01

CR fixes

3720c2e

Merge branch 'release/0.2.0' of https://github.com/mini-services/miniurl

f05a3aa

into release/0.2.0 � Conflicts: � src/services/storage/drivers/inMemory/index.ts

resolve conflict

e3d2c32

shay123g requested review from SnirShechter and lesagi and removed request for lesagi, Danielg212 and SnirShechter March 16, 2021 11:13

lesagi reviewed Mar 16, 2021

View reviewed changes

.env.development Outdated Show resolved Hide resolved

lesagi reviewed Mar 16, 2021

View reviewed changes

src/services/storage/drivers/inMemory/index.ts Outdated Show resolved Hide resolved

shay123g requested a review from lesagi March 16, 2021 11:38

shay123g and others added 2 commits March 16, 2021 13:39

Update .env.development

9369ee5

Co-authored-by: lesagi <[email protected]>

Update src/services/storage/drivers/inMemory/index.ts

6ab1042

Co-authored-by: lesagi <[email protected]>

lesagi reviewed Mar 16, 2021

View reviewed changes

shay123g requested a review from lesagi March 16, 2021 14:14

fix test & config

71e967f

shay123g changed the title ~~#16~~ url cleanup upgrade Mar 17, 2021

shay123g linked an issue Mar 17, 2021 that may be closed by this pull request

Url cleanup upgrades #16

Closed

SnirShechter requested changes Mar 19, 2021

View reviewed changes

shay123g and others added 2 commits March 22, 2021 10:04

Update src/config/validate.ts

ad41274

Co-authored-by: Snir Shechter <[email protected]>

Update src/config/validate.ts

2278c4a

Co-authored-by: Snir Shechter <[email protected]>

shay123g mentioned this pull request Mar 23, 2021

complete Redis imp with ttl from config after #148 #164

Closed

Shay.Gazit added 2 commits March 23, 2021 21:17

.

ff7533a

fix imports

2cf1176

shay123g requested a review from SnirShechter March 23, 2021 19:32

Shay.Gazit added 3 commits March 28, 2021 19:37

Merge branch 'release/0.2.0' of https://github.com/mini-services/miniurl

0198804

into 16 � Conflicts: � src/config/types.ts � src/index.ts � src/services/storage/drivers/inMemory/index.ts � src/services/storage/drivers/relational/index.ts

fix tests

7c53866

fix tests

75c9a8f

shay123g requested a review from Danielg212 April 1, 2021 11:52

SnirShechter assigned Danielg212 May 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

url cleanup upgrade #148

url cleanup upgrade #148

shay123g commented Mar 13, 2021

codecov bot commented Mar 13, 2021 •

edited

Loading

SnirShechter commented Mar 13, 2021

shay123g commented Mar 13, 2021 •

edited

Loading

shay123g commented Mar 14, 2021

lesagi left a comment

shay123g commented Mar 16, 2021

SnirShechter Mar 13, 2021

SnirShechter Mar 19, 2021

SnirShechter Mar 19, 2021

SnirShechter Mar 19, 2021

shay123g commented Mar 29, 2021

		@@ -43,8 +43,8 @@ export class InMemoryStorage implements StorageDriver {
		let deletedCount = 0

url cleanup upgrade #148

Are you sure you want to change the base?

url cleanup upgrade #148

Conversation

shay123g commented Mar 13, 2021

codecov bot commented Mar 13, 2021 • edited Loading

Codecov Report

SnirShechter commented Mar 13, 2021

shay123g commented Mar 13, 2021 • edited Loading

shay123g commented Mar 14, 2021

lesagi left a comment

Choose a reason for hiding this comment

shay123g commented Mar 16, 2021

SnirShechter Mar 13, 2021

Choose a reason for hiding this comment

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

SnirShechter Mar 19, 2021

Choose a reason for hiding this comment

shay123g commented Mar 29, 2021

codecov bot commented Mar 13, 2021 •

edited

Loading

shay123g commented Mar 13, 2021 •

edited

Loading