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.

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

fix(credentials): typing if no modules provided #1188

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Agent } from '../../core/src/agent/Agent'
import type { ConnectionRecord } from '../../core/src/modules/connections'
import type { JsonCredential, JsonLdCredentialDetailFormat } from '../../core/src/modules/credentials/formats/jsonld'
import type { Wallet } from '../../core/src/wallet'
import type { CredentialTestsAgent } from '../../core/tests/helpers'

import { InjectionSymbols } from '../../core/src/constants'
import { KeyType } from '../../core/src/crypto'
Expand All @@ -18,8 +18,8 @@ import testLogger from '../../core/tests/logger'

import { describeSkipNode17And18 } from './util'

let faberAgent: Agent
let aliceAgent: Agent
let faberAgent: CredentialTestsAgent
let aliceAgent: CredentialTestsAgent
let aliceConnection: ConnectionRecord
let aliceCredentialRecord: CredentialExchangeRecord
let faberCredentialRecord: CredentialExchangeRecord
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/agent/AgentModules.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Module, DependencyManager, ApiModule } from '../plugins'
import type { IsAny } from '../types'
import type { Constructor } from '../utils/mixins'
import type { AgentConfig } from './AgentConfig'

Expand Down Expand Up @@ -101,7 +102,9 @@ export type AgentApi<Modules extends ModulesMap> = {
export type CustomOrDefaultApi<
CustomModuleType,
DefaultModuleType extends ApiModule
> = CustomModuleType extends ApiModule
> = IsAny<CustomModuleType> extends true
? InstanceType<DefaultModuleType['api']>
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

This confuses me a bit.

It seems like we are checking if the CustomModuleType is supplied and if so, we use the DefaultModuleType['api']. Should that not be CustomModuleType['api'] and if not, maybe we should rename that type if we also use it for custom modules.

Small note, I am not very familiar with the generic module type setup so I might have missed something ;).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is a bit weird. If the custom module type is of type any (which is the case when you provide no modules key) it will pick type any for the credentials module. So what we do here is check if the CustomModuleType is any, and if that is the case we don't want to use it, because it will type the agent.credentials object as any, and we pick the default one. Hope that makes sense?

: CustomModuleType extends ApiModule
? InstanceType<CustomModuleType['api']>
: CustomModuleType extends Module
? never
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ let wallet
let signCredentialOptions: JsonLdCredentialDetailFormat

describe('credentials', () => {
let faberAgent: Agent
let aliceAgent: Agent
let faberAgent: Agent<typeof faberAgentOptions['modules']>
let aliceAgent: Agent<typeof aliceAgentOptions['modules']>
let faberReplay: ReplaySubject<CredentialStateChangedEvent>
let aliceReplay: ReplaySubject<CredentialStateChangedEvent>
const seed = 'testseed000000000000000000000001'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Agent } from '../../../../../agent/Agent'
import type { CredentialTestsAgent } from '../../../../../../tests/helpers'
import type { Wallet } from '../../../../../wallet'
import type { ConnectionRecord } from '../../../../connections'
import type { JsonCredential, JsonLdCredentialDetailFormat } from '../../../formats/jsonld/JsonLdCredentialFormat'
Expand Down Expand Up @@ -26,8 +26,8 @@ const TEST_LD_DOCUMENT: JsonCredential = {
}

describe('credentials', () => {
let faberAgent: Agent
let aliceAgent: Agent
let faberAgent: CredentialTestsAgent
let aliceAgent: CredentialTestsAgent
let faberConnection: ConnectionRecord
let aliceConnection: ConnectionRecord
let aliceCredentialRecord: CredentialExchangeRecord
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,8 @@ export type FlatArray<Arr> = Arr extends ReadonlyArray<infer InnerArr> ? FlatArr
* Get the awaited (resolved promise) type of Promise type.
*/
export type Awaited<T> = T extends Promise<infer U> ? U : never

/**
* Type util that returns `true` or `false` based on whether the input type `T` is of type `any`
*/
export type IsAny<T> = unknown extends T ? ([keyof T] extends [never] ? false : true) : false
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at this makes me think that this should not work, but it does haha. Quite interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this comes straight from Stack overflow. No idea why it works. I can add a link to th so post though

13 changes: 9 additions & 4 deletions packages/core/tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import type { SubjectMessage } from '../../../tests/transport/SubjectInboundTransport'
import type {
AcceptCredentialOfferOptions,
AgentDependencies,
BasicMessage,
BasicMessageStateChangedEvent,
ConnectionRecordProps,
Expand All @@ -13,10 +14,11 @@ import type {
SchemaTemplate,
Wallet,
} from '../src'
import type { AgentModulesInput } from '../src/agent/AgentModules'
import type { AgentModulesInput, EmptyModuleMap } from '../src/agent/AgentModules'
import type { IndyOfferCredentialFormat } from '../src/modules/credentials/formats/indy/IndyCredentialFormat'
import type { ProofAttributeInfo, ProofPredicateInfo } from '../src/modules/proofs/formats/indy/models'
import type { AutoAcceptProof } from '../src/modules/proofs/models/ProofAutoAcceptType'
import type { Awaited } from '../src/types'
import type { CredDef, Schema } from 'indy-sdk'
import type { Observable } from 'rxjs'

Expand Down Expand Up @@ -83,11 +85,11 @@ const taaVersion = (process.env.TEST_AGENT_TAA_VERSION ?? '1') as `${number}.${n
const taaAcceptanceMechanism = process.env.TEST_AGENT_TAA_ACCEPTANCE_MECHANISM ?? 'accept'
export { agentDependencies }

export function getAgentOptions<AgentModules extends AgentModulesInput>(
export function getAgentOptions<AgentModules extends AgentModulesInput | EmptyModuleMap>(
name: string,
extraConfig: Partial<InitConfig> = {},
modules?: AgentModules
) {
): { config: InitConfig; modules: AgentModules; dependencies: AgentDependencies } {
const config: InitConfig = {
label: `Agent: ${name}`,
walletConfig: {
Expand All @@ -111,7 +113,8 @@ export function getAgentOptions<AgentModules extends AgentModulesInput>(
logger: new TestLogger(LogLevel.off, name),
...extraConfig,
}
return { config, modules, dependencies: agentDependencies } as const

return { config, modules: (modules ?? {}) as AgentModules, dependencies: agentDependencies } as const
}

export function getPostgresAgentOptions(name: string, extraConfig: Partial<InitConfig> = {}) {
Expand Down Expand Up @@ -675,6 +678,8 @@ export function mockProperty<T extends {}, K extends keyof T>(object: T, propert
Object.defineProperty(object, property, { get: () => value })
}

// Helper type to get the type of the agents (with the custom modules) for the credential tests
export type CredentialTestsAgent = Awaited<ReturnType<typeof setupCredentialTests>>['aliceAgent']
export async function setupCredentialTests(
faberName: string,
aliceName: string,
Expand Down
4 changes: 2 additions & 2 deletions packages/tenants/src/TenantsApi.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { CreateTenantOptions, GetTenantAgentOptions, WithTenantAgentCallback } from './TenantsApiOptions'
import type { EmptyModuleMap, ModulesMap } from '@aries-framework/core'
import type { DefaultAgentModules, ModulesMap } from '@aries-framework/core'

import { AgentContext, inject, InjectionSymbols, AgentContextProvider, injectable, Logger } from '@aries-framework/core'

import { TenantAgent } from './TenantAgent'
import { TenantRecordService } from './services'

@injectable()
export class TenantsApi<AgentModules extends ModulesMap = EmptyModuleMap> {
export class TenantsApi<AgentModules extends ModulesMap = DefaultAgentModules> {
private agentContext: AgentContext
private tenantRecordService: TenantRecordService
private agentContextProvider: AgentContextProvider
Expand Down