-
Notifications
You must be signed in to change notification settings - Fork 145
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
Feat/get registry from uris #592
base: main
Are you sure you want to change the base?
Changes from all commits
26773dc
68f8e9c
acc43df
7346cf1
bcc30f7
cda809b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@hyperlane-xyz/registry': minor | ||
--- | ||
|
||
Add getRegistry and getMergedRegistry functions | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import type { Logger } from 'pino'; | ||
import { GithubRegistry } from './GithubRegistry.js'; | ||
import { FileSystemRegistry } from './FileSystemRegistry.js'; | ||
import { IRegistry } from './IRegistry.js'; | ||
import { DEFAULT_GITHUB_REGISTRY, PROXY_DEPLOYED_URL } from '../consts.js'; | ||
import { MergedRegistry } from './MergedRegistry.js'; | ||
|
||
const isHttpsUrl = (value: string): boolean => { | ||
try { | ||
if (!value) return false; | ||
const url = new URL(value); | ||
return url.protocol === 'https:'; | ||
} catch { | ||
return false; | ||
} | ||
}; | ||
|
||
const isCanonicalRepoUrl = (url: string): boolean => { | ||
return url === DEFAULT_GITHUB_REGISTRY; | ||
}; | ||
|
||
export function getRegistry( | ||
registryUris: string[], | ||
enableProxy: boolean, | ||
logger?: Logger, | ||
): IRegistry { | ||
const registryLogger = logger?.child({ module: 'MergedRegistry' }); | ||
const registries = registryUris | ||
.map((uri) => uri.trim()) | ||
.filter((uri) => !!uri) | ||
.map((uri, index) => { | ||
const childLogger = registryLogger?.child({ uri, index }); | ||
if (isHttpsUrl(uri)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fyi we are working on a HTTP registry #464, we we probably need a different way of identifying that it is supposed to be a github registry. I personally was in favor of using protocol prefix for that, but i got overruled |
||
return new GithubRegistry({ | ||
uri, | ||
logger: childLogger, | ||
proxyUrl: enableProxy && isCanonicalRepoUrl(uri) ? PROXY_DEPLOYED_URL : undefined, | ||
}); | ||
} else { | ||
return new FileSystemRegistry({ | ||
uri, | ||
logger: childLogger, | ||
}); | ||
} | ||
}); | ||
return new MergedRegistry({ | ||
registries, | ||
logger, | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import { expect } from 'chai'; | |
import sinon from 'sinon'; | ||
|
||
import type { ChainMetadata } from '@hyperlane-xyz/sdk'; | ||
import type { Logger } from 'pino'; | ||
import fs from 'fs'; | ||
import { CHAIN_FILE_REGEX } from '../../src/consts.js'; | ||
import { FileSystemRegistry } from '../../src/registry/FileSystemRegistry.js'; | ||
|
@@ -11,6 +12,8 @@ import { RegistryType } from '../../src/registry/IRegistry.js'; | |
import { MergedRegistry } from '../../src/registry/MergedRegistry.js'; | ||
import { PartialRegistry } from '../../src/registry/PartialRegistry.js'; | ||
import { ChainAddresses } from '../../src/types.js'; | ||
import { getRegistry } from '../../src/registry/registry-utils.js'; | ||
import { DEFAULT_GITHUB_REGISTRY, PROXY_DEPLOYED_URL } from '../../src/consts.js'; | ||
|
||
const GITHUB_REGISTRY_BRANCH = 'main'; | ||
|
||
|
@@ -262,3 +265,99 @@ describe('Warp routes file structure', () => { | |
expect(foundPath, foundPath ? `Found addresses.yaml at: ${foundPath}` : '').to.be.null; | ||
}); | ||
}); | ||
|
||
describe('Registry Utils', () => { | ||
// Mock logger | ||
const logger: Logger = { | ||
child: () => ({ info: () => {}, child: () => ({ info: () => {} }) }), | ||
} as any; | ||
|
||
const localPath = './'; | ||
const githubUrl = 'https://github.com/hyperlane-xyz/hyperlane-registry'; | ||
|
||
describe('getRegistry', () => { | ||
it('creates MergedRegistry with FileSystemRegistry for local path', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional Nit: For all these tests, i wonder if its nice to just have a mapping of |
||
const registry = getRegistry([localPath], false, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(1); | ||
expect(registry.registries[0]).to.be.instanceOf(FileSystemRegistry); | ||
expect(registry.registries[0].uri).to.equal(localPath); | ||
}); | ||
|
||
it('creates MergedRegistry with GithubRegistry for HTTPS URLs', () => { | ||
const registry = getRegistry([githubUrl], false, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(1); | ||
expect(registry.registries[0]).to.be.instanceOf(GithubRegistry); | ||
expect(registry.registries[0].uri).to.equal(githubUrl); | ||
}); | ||
|
||
it('creates MergedRegistry with proxied GithubRegistry for canonical repo', () => { | ||
const registry = getRegistry([DEFAULT_GITHUB_REGISTRY], true, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(1); | ||
expect(registry.registries[0]).to.be.instanceOf(GithubRegistry); | ||
expect(registry.registries[0].uri).to.equal(DEFAULT_GITHUB_REGISTRY); | ||
expect((registry.registries[0] as GithubRegistry).proxyUrl).to.equal(PROXY_DEPLOYED_URL); | ||
}); | ||
|
||
it('creates MergedRegistry with non-proxied GithubRegistry for non-canonical repos', () => { | ||
const registry = getRegistry(['https://github.com/test'], true, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(1); | ||
expect(registry.registries[0]).to.be.instanceOf(GithubRegistry); | ||
expect(registry.registries[0].uri).to.equal('https://github.com/test'); | ||
expect((registry.registries[0] as GithubRegistry).proxyUrl).to.be.undefined; | ||
}); | ||
|
||
it('throws error for empty URIs array', () => { | ||
expect(() => getRegistry([], true, logger)).to.throw('At least one registry URI is required'); | ||
expect(() => getRegistry([''], true, logger)).to.throw( | ||
'At least one registry URI is required', | ||
); | ||
expect(() => getRegistry([' '], true, logger)).to.throw( | ||
'At least one registry URI is required', | ||
); | ||
}); | ||
|
||
it('creates MergedRegistry with FileSystemRegistry for non-HTTPS URLs', () => { | ||
const nonHttpsUrl = 'http://example.com'; | ||
const registry = getRegistry([nonHttpsUrl], false, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(1); | ||
expect(registry.registries[0]).to.be.instanceOf(FileSystemRegistry); | ||
expect(registry.registries[0].uri).to.equal(nonHttpsUrl); | ||
}); | ||
|
||
it('creates MergedRegistry with multiple URIs', () => { | ||
const uris = [githubUrl, localPath]; | ||
const registry = getRegistry(uris, false, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(2); | ||
expect(registry.registries[0]).to.be.instanceOf(GithubRegistry); | ||
expect(registry.registries[1]).to.be.instanceOf(FileSystemRegistry); | ||
}); | ||
|
||
it('creates MergedRegistry with mixed registry types and proxy settings', () => { | ||
const uris = [DEFAULT_GITHUB_REGISTRY, localPath, 'https://github.com/test']; | ||
const registry = getRegistry(uris, true, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(3); | ||
expect(registry.registries[0]).to.be.instanceOf(GithubRegistry); | ||
expect((registry.registries[0] as GithubRegistry).proxyUrl).to.equal(PROXY_DEPLOYED_URL); | ||
expect(registry.registries[1]).to.be.instanceOf(FileSystemRegistry); | ||
expect(registry.registries[2]).to.be.instanceOf(GithubRegistry); | ||
expect((registry.registries[2] as GithubRegistry).proxyUrl).to.be.undefined; | ||
}); | ||
|
||
it('properly initializes all registries with logger', () => { | ||
const uris = [githubUrl, localPath]; | ||
const registry = getRegistry(uris, false, logger) as MergedRegistry; | ||
expect(registry).to.be.instanceOf(MergedRegistry); | ||
expect(registry.registries.length).to.equal(2); | ||
expect(registry).to.have.property('logger'); | ||
expect(registry.registries[0]).to.have.property('logger'); | ||
expect(registry.registries[1]).to.have.property('logger'); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer have
getMergedRegistry
in here yeah?