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

[#IP-84] Change strict configuration to true #109

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

[#IP-84] Change strict configuration to true #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
da61300
(^) set strict=true
fabriziopapi Mar 30, 2021
16471dc
Merge branch 'master' into IP-84--enforce-strict-to-true
balanza Apr 8, 2021
da1d990
Merge branch 'master' into IP-84--enforce-strict-to-true
balanza Apr 8, 2021
14e1dec
(^) fixed withApiRequestWrapper strict=true compatibility
fabriziopapi Apr 8, 2021
2704fcf
Merge branch 'IP-84--enforce-strict-to-true' of https://github.com/pa…
fabriziopapi Apr 8, 2021
cb2283b
(+) add docblock to withApiRequestWrapper function
fabriziopapi Apr 9, 2021
cb34d59
(+) added nw api wrapper function to managen empty-response call
fabriziopapi Apr 9, 2021
ad839f4
(^) fixed trackMessageProcessing return type
fabriziopapi Apr 12, 2021
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
4 changes: 2 additions & 2 deletions CreateMessage/__tests__/handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ describe("createMessageDocument", () => {

expect(mockMessageModel.create).toHaveBeenCalledTimes(1);
expect(response.isRight()).toBeTruthy();
expect(response.getOrElse(undefined)).toMatchObject({
expect(response.fold(l=>undefined,r=>r)).toMatchObject({
fiscalCode,
id: messageId,
indexedId: messageId,
Expand Down Expand Up @@ -242,7 +242,7 @@ describe("forkOrchestrator", () => {
serviceVersion: service.version
})
);
expect(response.getOrElse(undefined)).toEqual("orchestratorId");
expect(response.fold(l=>undefined,r=>r)).toEqual("orchestratorId");
}
)
);
Expand Down
2 changes: 1 addition & 1 deletion CreateMessage/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ export function CreateMessageHandler(
telemetryClient.trackEvent({
name: "api.messages.create",
properties: {
error: isSuccess ? undefined : r.kind,
error: isSuccess ? "" : r.kind,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this?

hasDefaultEmail: Boolean(
messagePayload.default_addresses &&
messagePayload.default_addresses.email
Expand Down
13 changes: 7 additions & 6 deletions CreateService/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ import {
} from "italia-ts-commons/lib/strings";
import { APIClient } from "../clients/admin";
import { Service } from "../generated/api-admin/Service";
import { ServiceMetadata } from "../generated/api-admin/ServiceMetadata";
import { Subscription } from "../generated/api-admin/Subscription";
import { UserInfo } from "../generated/api-admin/UserInfo";
import { ServicePayload } from "../generated/definitions/ServicePayload";
import { ServiceWithSubscriptionKeys } from "../generated/definitions/ServiceWithSubscriptionKeys";
import { withApiRequestWrapper } from "../utils/api";
import { withEmbodimentApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses, IResponseErrorUnauthorized } from "../utils/responses";

Expand Down Expand Up @@ -89,7 +90,7 @@ const createSubscriptionTask = (
subscriptionId: NonEmptyString,
productName: NonEmptyString
): TaskEither<ErrorResponses, Subscription> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.createSubscription({
Expand All @@ -107,7 +108,7 @@ const getUserTask = (
apiClient: APIClient,
userEmail: EmailString
): TaskEither<ErrorResponses, UserInfo> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getUser({
Expand All @@ -124,7 +125,7 @@ const createServiceTask = (
sandboxFiscalCode: FiscalCode,
adb2cTokenName: NonEmptyString
): TaskEither<ErrorResponses, Service> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.createService({
Expand All @@ -135,7 +136,7 @@ const createServiceTask = (
service_metadata: {
...servicePayload.service_metadata,
token_name: adb2cTokenName
}
} as ServiceMetadata
}
}),
200
Expand Down Expand Up @@ -175,7 +176,7 @@ export function CreateServiceHandler(
servicePayload,
subscriptionId,
(sandboxFiscalCode as unknown) as FiscalCode,
userInfo.token_name
(userInfo.token_name as unknown) as NonEmptyString
).map(service => {
telemetryClient.trackEvent({
name: "api.services.create",
Expand Down
2 changes: 1 addition & 1 deletion CreatedMessageOrchestrator/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const trackMessageProcessing = (
properties: event.properties,
tagOverrides: { samplingEnabled: "false" }
})
: null;
: void 0;

export enum MessageProcessingEventNames {
DECODE_INPUT = "api.messages.create.decodeinput",
Expand Down
6 changes: 3 additions & 3 deletions GetService/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { APIClient } from "../clients/admin";
import { Service } from "../generated/api-admin/Service";
import { SubscriptionKeys } from "../generated/api-admin/SubscriptionKeys";
import { ServiceWithSubscriptionKeys } from "../generated/definitions/ServiceWithSubscriptionKeys";
import { withApiRequestWrapper } from "../utils/api";
import { withEmbodimentApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses } from "../utils/responses";
import { serviceOwnerCheckTask } from "../utils/subscription";
Expand Down Expand Up @@ -69,7 +69,7 @@ const getServiceTask = (
apiClient: APIClient,
serviceId: string
): TaskEither<ErrorResponses, Service> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getService({
Expand All @@ -83,7 +83,7 @@ const getSubscriptionKeysTask = (
apiClient: APIClient,
serviceId: string
): TaskEither<ErrorResponses, SubscriptionKeys> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getSubscriptionKeys({
Expand Down
10 changes: 5 additions & 5 deletions GetSubscriptionsFeed/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type TableEntry = Readonly<{
* @see https://docs.microsoft.com/en-us/rest/api/storageservices/query-timeout-and-pagination
*/
export type PagedQuery = (
currentToken: TableService.TableContinuationToken
currentToken: TableService.TableContinuationToken | undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

| undefined actually means the parameter is optional, you can just use

currentToken?: TableService.TableContinuationToken

Besides syntax, is this really optional?

) => Promise<Either<Error, TableService.QueryEntitiesResult<TableEntry>>>;

/**
Expand All @@ -32,7 +32,7 @@ export const getPagedQuery = (tableService: TableService, table: string) => (
tableService.queryEntities(
table,
tableQuery,
currentToken,
currentToken as TableService.TableContinuationToken,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is related to the comment above. If currentToken is possibly undefined, then you cannot cast it but you must handle the undefined case.

(
error: Error,
result: TableService.QueryEntitiesResult<TableEntry>,
Expand All @@ -51,16 +51,16 @@ async function* iterateOnPages(
pagedQuery: PagedQuery
): AsyncIterableIterator<ReadonlyArray<TableEntry>> {
// tslint:disable-next-line: no-let
let token: TableService.TableContinuationToken = null;
let token: TableService.TableContinuationToken | undefined = undefined;
do {
// query for a page of entries
const errorOrResults = await pagedQuery(token);
const errorOrResults : Either<Error, TableService.QueryEntitiesResult<TableEntry>> = await pagedQuery(token);
Copy link
Contributor

Choose a reason for hiding this comment

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

When you manually specify the type of a variable is a subtle form of manual casting that we should avoid.
This is probably happening because pagedQuery is not typed properly, thus TS fails to infer its type.

The takeaway from this comment is: when you need to manually assign a type, it's a ringing bell that of poor typings somwhere else

if (isLeft(errorOrResults)) {
// throw an exception in case of error
throw errorOrResults.value;
}
// call the async callback with the current page of entries
const results = errorOrResults.value;
const results : TableService.QueryEntitiesResult<TableEntry> = errorOrResults.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

yield results.entries;
// update the continuation token, the loop will continue until
// the token is defined
Expand Down
2 changes: 1 addition & 1 deletion GetUserServices/__tests__/handler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe("GetUserServicesHandler", () => {
expect(result.kind).toBe("IResponseSuccessJson");
if (result.kind === "IResponseSuccessJson") {
expect(result.value).toEqual({
items: aUserInfo.subscriptions.map(it => it.id)
items: (aUserInfo.subscriptions as Subscription[]).map(it => it.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto: manual casting

});
}
});
Expand Down
9 changes: 5 additions & 4 deletions GetUserServices/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ import { ContextMiddleware } from "@pagopa/io-functions-commons/dist/src/utils/m
import { identity } from "fp-ts/lib/function";
import { TaskEither } from "fp-ts/lib/TaskEither";
import { APIClient } from "../clients/admin";
import { Subscription } from "../generated/api-admin/Subscription";
import { UserInfo } from "../generated/api-admin/UserInfo";
import { ServiceIdCollection } from "../generated/definitions/ServiceIdCollection";
import { withApiRequestWrapper } from "../utils/api";
import { withEmbodimentApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses } from "../utils/responses";

Expand All @@ -61,7 +62,7 @@ const getUserTask = (
apiClient: APIClient,
userEmail: EmailString
): TaskEither<ErrorResponses, UserInfo> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getUser({
Expand All @@ -84,8 +85,8 @@ export function GetUserServicesHandler(
)
.map(userInfo =>
ResponseSuccessJson({
items: userInfo.subscriptions.map(it =>
ServiceId.encode(it.id as NonEmptyString)
items: (userInfo.subscriptions as readonly Subscription[]).map(it =>
ServiceId.encode((it.id as unknown) as NonEmptyString)
)
})
)
Expand Down
4 changes: 2 additions & 2 deletions RegenerateServiceKey/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { TaskEither } from "fp-ts/lib/TaskEither";
import { APIClient } from "../clients/admin";
import { SubscriptionKeys } from "../generated/definitions/SubscriptionKeys";
import { SubscriptionKeyTypePayload } from "../generated/definitions/SubscriptionKeyTypePayload";
import { withApiRequestWrapper } from "../utils/api";
import { withEmbodimentApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses, IResponseErrorUnauthorized } from "../utils/responses";
import { serviceOwnerCheckTask } from "../utils/subscription";
Expand Down Expand Up @@ -80,7 +80,7 @@ const regenerateServiceKeyTask = (
serviceId: NonEmptyString,
subscriptionKeyTypePayload: SubscriptionKeyTypePayload
): TaskEither<ErrorResponses, IResponseSuccessJson<SubscriptionKeys>> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.RegenerateSubscriptionKeys({
Expand Down
16 changes: 9 additions & 7 deletions UpdateService/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ import { SubscriptionKeys } from "../generated/api-admin/SubscriptionKeys";
import { UserInfo } from "../generated/api-admin/UserInfo";
import { ServicePayload } from "../generated/definitions/ServicePayload";
import { ServiceWithSubscriptionKeys } from "../generated/definitions/ServiceWithSubscriptionKeys";
import { withApiRequestWrapper } from "../utils/api";
import { withEmbodimentApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses, IResponseErrorUnauthorized } from "../utils/responses";
import { serviceOwnerCheckTask } from "../utils/subscription";

import { ServiceMetadata } from "../generated/api-admin/ServiceMetadata";

type ResponseTypes =
| IResponseSuccessJson<ServiceWithSubscriptionKeys>
| IResponseErrorUnauthorized
Expand Down Expand Up @@ -82,7 +84,7 @@ const getSubscriptionKeysTask = (
apiClient: APIClient,
serviceId: NonEmptyString
): TaskEither<ErrorResponses, SubscriptionKeys> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getSubscriptionKeys({
Expand All @@ -96,7 +98,7 @@ const getServiceTask = (
apiClient: APIClient,
serviceId: NonEmptyString
): TaskEither<ErrorResponses, Service> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getService({
Expand All @@ -110,7 +112,7 @@ const getUserTask = (
apiClient: APIClient,
userEmail: EmailString
): TaskEither<ErrorResponses, UserInfo> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.getUser({
Expand All @@ -127,7 +129,7 @@ const updateServiceTask = (
retrievedService: Service,
adb2cTokenName: NonEmptyString
): TaskEither<ErrorResponses, Service> =>
withApiRequestWrapper(
withEmbodimentApiRequestWrapper(
logger,
() =>
apiClient.updateService({
Expand All @@ -138,7 +140,7 @@ const updateServiceTask = (
service_metadata: {
...servicePayload.service_metadata,
token_name: adb2cTokenName
}
} as ServiceMetadata
},
service_id: serviceId
}),
Expand Down Expand Up @@ -172,7 +174,7 @@ export function UpdateServiceHandler(
servicePayload,
serviceId,
retrievedService,
userInfo.token_name
(userInfo.token_name as unknown) as NonEmptyString
)
)
.chain(service => {
Expand Down
4 changes: 2 additions & 2 deletions UploadOrganizationLogo/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { identity } from "fp-ts/lib/function";
import { TaskEither } from "fp-ts/lib/TaskEither";
import { APIClient } from "../clients/admin";
import { Logo } from "../generated/api-admin/Logo";
import { withApiRequestWrapper } from "../utils/api";
import { withEmptyApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import {
ErrorResponses,
Expand Down Expand Up @@ -80,7 +80,7 @@ const uploadOrganizationLogoTask = (
organizationFiscalCode: OrganizationFiscalCode,
logo: Logo
): TaskEither<ErrorResponses, IResponseSuccessAccepted> =>
withApiRequestWrapper(
withEmptyApiRequestWrapper(
logger,
() =>
apiClient.uploadOrganizationLogo({
Expand Down
6 changes: 3 additions & 3 deletions UploadServiceLogo/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { identity } from "fp-ts/lib/function";
import { TaskEither } from "fp-ts/lib/TaskEither";
import { APIClient } from "../clients/admin";
import { Logo } from "../generated/api-admin/Logo";
import { withApiRequestWrapper } from "../utils/api";
import { withEmptyApiRequestWrapper } from "../utils/api";
import { getLogger, ILogger } from "../utils/logging";
import { ErrorResponses, IResponseErrorUnauthorized } from "../utils/responses";
import { serviceOwnerCheckTask } from "../utils/subscription";
Expand Down Expand Up @@ -79,15 +79,15 @@ const uploadServiceLogoTask = (
serviceId: string,
logo: Logo
): TaskEither<ErrorResponses, IResponseSuccessJson<undefined>> =>
withApiRequestWrapper(
withEmptyApiRequestWrapper(
logger,
() =>
apiClient.uploadServiceLogo({
body: logo,
service_id: serviceId
}),
201
).map(_ => ResponseSuccessJson(undefined));
).map(ResponseSuccessJson);

/**
* Handles requests for upload a service logo by a service ID and a base64 logo' s string.
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@types/express": "^4.16.0",
"@types/html-to-text": "^1.4.31",
"@types/jest": "^24.0.15",
"@types/node-fetch": "^2.5.8",
"@types/nodemailer": "^4.6.2",
"auto-changelog": "^2.2.1",
"danger": "^4.0.2",
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"outDir": "dist",
"rootDir": ".",
"sourceMap": true,
"strict": false,
"strict": true,
"resolveJsonModule": true
}, "exclude": [
"Dangerfile.ts",
Expand Down
16 changes: 9 additions & 7 deletions utils/__tests__/arbitraries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@ import * as fc from "fast-check";
import { some } from "fp-ts/lib/Option";
import {
NonNegativeInteger,
WithinRangeInteger
WithinRangeInteger,
IWithinRangeIntegerTag
} from "italia-ts-commons/lib/numbers";
import {
EmailString,
FiscalCode,
NonEmptyString,
PatternString
PatternString,
} from "italia-ts-commons/lib/strings";


//
// custom fastcheck arbitraries
//
Expand Down Expand Up @@ -82,9 +84,9 @@ export const newMessageArb = fc
markdown,
subject
}
}).getOrElse(undefined)
}).fold(l=>undefined, r=>r)
)
.filter(_ => _ !== undefined);
.filter(_ => _ !== undefined) as fc.Arbitrary<any>;

export const newMessageWithDefaultEmailArb = fc
.tuple(newMessageArb, fc.emailAddress())
Expand All @@ -100,14 +102,14 @@ export const messageTimeToLiveArb = fc
// tslint:disable-next-line:no-useless-cast
.map(_ => _ as number & WithinRangeInteger<3600, 604800>);

export const amountArb = fc
export const amountArb = fc
.integer(1, 9999999999)
.map(_ =>
WithinRangeInteger(1, 9999999999)
.decode(_)
.getOrElse(undefined)
.fold(l=>undefined, r=>r)
)
.filter(_ => _ !== undefined);
.filter(_ => _ != undefined) as fc.Arbitrary<number & IWithinRangeIntegerTag<1,9999999999>>;

export const maxAmountArb = fc
.integer(0, 9999999999)
Expand Down
Loading