Remove from/to LocalTimestamp deprecated query params (#924)

* Remove from/to LocalTimestamp deprecated query params * Remove METRICS config var * Apply suggestions from code review Co-authored-by: jmoguilevsky <jmoguilevsky@gmail.com> Co-authored-by: jmoguilevsky <jmoguilevsky@gmail.com>
decentraland · Feb 18, 2022 · 5d426f4 · 5d426f4
1 parent fda710b
commit 5d426f4
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 115 deletions.
diff --git a/content/src/Environment.ts b/content/src/Environment.ts
@@ -85,8 +85,6 @@ export class Environment {
 export enum EnvironmentConfig {
   STORAGE_ROOT_FOLDER,
   SERVER_PORT,
-  // @deprecated
-  METRICS,
   LOG_REQUESTS,
   UPDATE_FROM_DAO_INTERVAL,
   SYNC_WITH_SERVERS_INTERVAL,
@@ -175,7 +173,6 @@ export class EnvironmentBuilder {
       EnvironmentConfig.SERVER_PORT,
       () => process.env.CONTENT_SERVER_PORT ?? DEFAULT_SERVER_PORT
     )
-    this.registerConfigIfNotAlreadySet(env, EnvironmentConfig.METRICS, () => process.env.METRICS !== 'false')
     this.registerConfigIfNotAlreadySet(env, EnvironmentConfig.LOG_REQUESTS, () => process.env.LOG_REQUESTS !== 'false')
     this.registerConfigIfNotAlreadySet(
       env,

diff --git a/content/src/controller/Controller.ts b/content/src/controller/Controller.ts
@@ -56,7 +56,7 @@ export class Controller {
     Controller.LOGGER = components.logs.getLogger('Controller')
   }
 
-  async getEntities(req: express.Request, res: express.Response) {
+  async getEntities(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /entities/:type
     // Query String: ?{filter}&fields={fieldList}
@@ -190,7 +190,7 @@ export class Controller {
     )
   }
 
-  async headContent(req: express.Request, res: express.Response) {
+  async headContent(req: express.Request, res: express.Response): Promise<void> {
     // Method: HEAD
     // Path: /contents/:hashId
     const hashId = req.params.hashId
@@ -206,7 +206,7 @@ export class Controller {
     }
   }
 
-  async getContent(req: express.Request, res: express.Response) {
+  async getContent(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /contents/:hashId
     const hashId = req.params.hashId
@@ -227,7 +227,7 @@ export class Controller {
     }
   }
 
-  async getAvailableContent(req: express.Request, res: express.Response) {
+  async getAvailableContent(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /available-content
     // Query String: ?cid={hashId1}&cid={hashId2}
@@ -247,7 +247,7 @@ export class Controller {
     }
   }
 
-  async getAudit(req: express.Request, res: express.Response) {
+  async getAudit(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /audit/:type/:entityId
     const type = parseEntityType(req.params.type)
@@ -282,18 +282,14 @@ export class Controller {
     }
   }
 
-  async getPointerChanges(req: express.Request, res: express.Response) {
+  async getPointerChanges(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /pointer-changes
     // Query String: ?from={timestamp}&to={timestamp}&offset={number}&limit={number}&entityType={entityType}&includeAuthChain={boolean}
     const stringEntityTypes = this.asArray<string>(req.query.entityType)
     const entityTypes: (EntityType | undefined)[] | undefined = stringEntityTypes
       ? stringEntityTypes.map((type) => parseEntityType(type))
       : undefined
-    // deprecated
-    const fromLocalTimestamp: Timestamp | undefined = this.asInt(req.query.fromLocalTimestamp)
-    // deprecated
-    const toLocalTimestamp: Timestamp | undefined = this.asInt(req.query.toLocalTimestamp)
     const from: Timestamp | undefined = this.asInt(req.query.from)
     const to: Timestamp | undefined = this.asInt(req.query.to)
     const offset: number | undefined = this.asInt(req.query.offset)
@@ -341,14 +337,10 @@ export class Controller {
       }
     }
 
-    // TODO: remove this when to/from localTimestamp parameter is deprecated to use to/from
-    const fromFilter = from ?? fromLocalTimestamp
-    const toFilter = to ?? toLocalTimestamp
-
     const requestFilters = {
       entityTypes: entityTypes as EntityType[] | undefined,
-      from: fromFilter,
-      to: toFilter,
+      from,
+      to,
       includeAuthChain,
       includeOverwrittenInfo: includeAuthChain
     }
@@ -397,7 +389,7 @@ export class Controller {
     return '?' + nextQueryParams
   }
 
-  async getActiveDeploymentsByContentHash(req: express.Request, res: express.Response) {
+  async getActiveDeploymentsByContentHash(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /contents/:hashId/active-entities
     const hashId = req.params.hashId
@@ -413,7 +405,7 @@ export class Controller {
     res.json(result)
   }
 
-  async getDeployments(req: express.Request, res: express.Response) {
+  async getDeployments(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /deployments
     // Query String: ?from={timestamp}&toLocalTimestamp={timestamp}&entityType={entityType}&entityId={entityId}&onlyCurrentlyPointed={boolean}
@@ -436,10 +428,6 @@ export class Controller {
       req.query.sortingOrder as string
     )
     const lastId: string | undefined = (req.query.lastId as string)?.toLowerCase()
-    // deprecated
-    const fromLocalTimestamp: number | undefined = this.asInt(req.query.fromLocalTimestamp)
-    // deprecated
-    const toLocalTimestamp: number | undefined = this.asInt(req.query.toLocalTimestamp)
     const from: number | undefined = this.asInt(req.query.from)
     const to: number | undefined = this.asInt(req.query.to)
 
@@ -485,27 +473,13 @@ export class Controller {
       }
     }
 
-    // Validate to and from are valid
-    if (sortingField == SortingField.ENTITY_TIMESTAMP && (fromLocalTimestamp || toLocalTimestamp)) {
-      res.status(400).send({
-        error: 'The filters fromLocalTimestamp and toLocalTimestamp can not be used when sorting by entity timestamp.'
-      })
-      return
-    }
-
-    // TODO: remove this when to/from localTimestamp parameter is deprecated to use to/from
-    const fromFilter =
-      (!sortingField || sortingField == SortingField.LOCAL_TIMESTAMP) && fromLocalTimestamp ? fromLocalTimestamp : from
-    const toFilter =
-      (!sortingField || sortingField == SortingField.LOCAL_TIMESTAMP) && toLocalTimestamp ? toLocalTimestamp : to
-
     const requestFilters = {
       pointers,
       entityTypes: entityTypes as EntityType[],
       entityIds,
       onlyCurrentlyPointed,
-      from: fromFilter,
-      to: toFilter
+      from,
+      to
     }
 
     const deploymentOptions = {
@@ -593,7 +567,7 @@ export class Controller {
     return value ? value === 'true' : undefined
   }
 
-  async getStatus(req: express.Request, res: express.Response) {
+  async getStatus(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /status
 
@@ -613,7 +587,7 @@ export class Controller {
   /**
    * @deprecated
    */
-  async getSnapshot(req: express.Request, res: express.Response) {
+  async getSnapshot(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /snapshot/:type
 
@@ -634,7 +608,7 @@ export class Controller {
     }
   }
 
-  async getAllSnapshots(req: express.Request, res: express.Response) {
+  async getAllSnapshots(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /snapshot
 
@@ -647,15 +621,15 @@ export class Controller {
     }
   }
 
-  async getFailedDeployments(req: express.Request, res: express.Response) {
+  async getFailedDeployments(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /failed-deployments
 
     const failedDeployments = await this.components.deployer.getAllFailedDeployments()
     res.send(failedDeployments)
   }
 
-  async getChallenge(req: express.Request, res: express.Response) {
+  async getChallenge(req: express.Request, res: express.Response): Promise<void> {
     // Method: GET
     // Path: /challenge
 

diff --git a/content/src/service/synchronization/batchDeployer.ts b/content/src/service/synchronization/batchDeployer.ts
@@ -115,8 +115,6 @@ export function createBatchDeployerComponent(
     }
   }
 
-  // TODO: [new-sync] every now and then cleanup the deploymentsMap of old deployments
-
   return {
     async stop() {
       // stop will wait for the queue to end.

diff --git a/content/test/integration/E2ETestEnvironment.ts b/content/test/integration/E2ETestEnvironment.ts
@@ -33,7 +33,6 @@ export class E2ETestEnvironment {
       .setConfig(EnvironmentConfig.LOG_REQUESTS, false)
       .setConfig(EnvironmentConfig.LOG_LEVEL, 'off')
       .setConfig(EnvironmentConfig.BOOTSTRAP_FROM_SCRATCH, false)
-      .setConfig(EnvironmentConfig.METRICS, false)
 
     if (overrideConfigs) {
       for (const key in overrideConfigs) {

diff --git a/content/test/integration/controller/deployment-pagination.spec.ts b/content/test/integration/controller/deployment-pagination.spec.ts
@@ -1,5 +1,4 @@
 import { toQueryParams } from '@catalyst/commons'
-import assert from 'assert'
 import { EntityType, fetchJson, SortingField, SortingOrder, Timestamp } from 'dcl-catalyst-commons'
 import { DeploymentField } from '../../../src/controller/Controller'
 import { EnvironmentConfig } from '../../../src/Environment'
@@ -209,50 +208,6 @@ loadStandaloneTestEnvironment()('Integration - Deployment Pagination', (testEnv)
     expect(nextLink).toContain(`lastId=${E2.entity.id}`)
   })
 
-  // TODO: Remove this test when toLocalTimestamp and fromLocalTimestamp are deleted
-  it('given deprecated filter, when local timestamp filter is set with asc order, then only from is modified in next', async () => {
-    // Deploy E1, E2 and E3 in that order
-    const [E1Timestamp, E2Timestamp, E3Timestamp] = await deploy(E1, E2, E3)
-
-    const url =
-      server.getUrl() +
-      `/deployments?` +
-      toQueryParams({
-        limit: 2,
-        sortingOrder: SortingOrder.ASCENDING,
-        fromLocalTimestamp: E1Timestamp,
-        toLocalTimestamp: E3Timestamp
-      })
-    const actualDeployments = (await fetchJson(url)) as any
-
-    expect(actualDeployments.deployments.length).toBe(2)
-    const nextLink = actualDeployments.pagination.next
-    expect(nextLink).toContain(`from=${E2Timestamp}`)
-    expect(nextLink).toContain(`to=${E3Timestamp}`)
-    expect(nextLink).toContain(`lastId=${E2.entity.id}`)
-  })
-
-  // TODO: Remove this test when toLocalTimestamp and fromLocalTimestamp are deleted
-  it('given deprecated filters, when order is by entity timestamp, then it fails', async () => {
-    // Deploy E1, E2 and E3 in that order
-    const [E1Timestamp, , E3Timestamp] = await deploy(E1, E2, E3)
-
-    try {
-      const url =
-        server.getUrl() +
-        `/deployments?` +
-        toQueryParams({
-          limit: 2,
-          sortingField: SortingField.ENTITY_TIMESTAMP,
-          fromLocalTimestamp: E1Timestamp,
-          toLocalTimestamp: E3Timestamp
-        })
-      await fetchJson(url)
-
-      assert.fail('When using deprecated filters it should fail.')
-    } catch (err) {}
-  })
-
   it('When getting pointer changes then the pagination is correctly done', async () => {
     // Deploy E1, E2 in that order
     const [E1Timestamp, E2Timestamp] = await deploy(E1, E2)
@@ -265,26 +220,6 @@ loadStandaloneTestEnvironment()('Integration - Deployment Pagination', (testEnv)
     expect(pointerChanges.pagination.next).toContain(`lastId=${E2.entity.id}`)
   })
 
-  // TODO: Remove this test when toLocalTimestamp and fromLocalTimestamp are deleted
-  it('When getting pointer changes with deprecated filter then the pagination is correctly done', async () => {
-    // Deploy E1, E2 in that order
-    const [E1Timestamp, E2Timestamp] = await deploy(E1, E2)
-
-    const url =
-      server.getUrl() +
-      `/pointer-changes?` +
-      toQueryParams({ fromLocalTimestamp: E1Timestamp, toLocalTimestamp: E2Timestamp, limit: 1 })
-
-    const pointerChanges = (await fetchJson(url)) as any
-
-    expect(pointerChanges.deltas.length).toBe(1)
-    expect(pointerChanges.pagination.next).not.toContain('toLocalTimestamp=')
-    expect(pointerChanges.pagination.next).not.toContain('fromLocalTimestamp')
-    expect(pointerChanges.pagination.next).toContain(`to=${E2Timestamp}`)
-    expect(pointerChanges.pagination.next).toContain(`from=${E1Timestamp}`)
-    expect(pointerChanges.pagination.next).toContain(`lastId=${E2.entity.id}`)
-  })
-
   async function deploy(...entities: EntityCombo[]): Promise<Timestamp[]> {
     const result: Timestamp[] = []
     for (const { deployData } of entities) {

diff --git a/lambdas/src/Environment.ts b/lambdas/src/Environment.ts
@@ -204,7 +204,6 @@ export class EnvironmentBuilder {
       ms(process.env.PROFILE_WEARABLES_CACHE_TIMEOUT ?? '30m')
     )
     this.registerConfigIfNotAlreadySet(env, EnvironmentConfig.PROOF_OF_WORK, () => process.env.PROOF_OF_WORK === 'true')
-    this.registerConfigIfNotAlreadySet(env, EnvironmentConfig.METRICS, () => process.env.METRICS !== 'false')
 
     this.registerConfigIfNotAlreadySet(
       env,