Skip to content

Commit

Permalink
feat(core): rename method and improve logging (#225)
Browse files Browse the repository at this point in the history 
- Renamed the `registerProviders` method of the `FeatureServiceRegistry` to `registerFeatureServices`.
- The messages/errors logged by the `FeatureServiceRegistry` have been improved.
- The messages/errors logged by the `FeatureAppManager` have been improved.
  • Loading branch information
@clebert
clebert authored Jan 4, 2019
1 parent ef7a0b9 commit 45c8906
Show file tree
Hide file tree
Showing 13 changed files with 216 additions and 197 deletions.
9 changes: 6 additions & 3 deletions docs/guides/integrating-the-feature-hub.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ const featureServiceDefinitions = [
someFeatureServiceDefinition2
];

registry.registerProviders(featureServiceDefinitions, 'acme:integrator');
registry.registerFeatureServices(featureServiceDefinitions, 'acme:integrator');

const manager = new FeatureAppManager(registry);
```

**Note:** The integrator needs a self-selected but unique consumer ID to
register or [consume][consuming-feature-services] Feature Services (in the
example above it is `'acme:integrator'`). All Feature Services [registered
together][faq-1] using the `registerProviders` method of the
together][faq-1] using the `registerFeatureServices` method of the
`FeatureServiceRegistry` are automatically sorted topologically and therefore do
not need to be registered in the correct order.

Expand Down Expand Up @@ -278,7 +278,10 @@ const featureServiceDefinitions = [
someFeatureServiceDefinition2
];

registry.registerProviders(featureServiceDefinitions, integratorDefinition.id);
registry.registerFeatureServices(
featureServiceDefinitions,
integratorDefinition.id
);

const {featureServices} = registry.bindFeatureServices(integratorDefinition);
const someFeatureService2 = featureServices[someFeatureServiceDefinition2.id];
Expand Down
4 changes: 2 additions & 2 deletions docs/guides/sharing-the-browser-history.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const featureServiceDefinitions = [
defineServerRenderer()
];

registry.registerProviders(featureServiceDefinitions, 'acme:integrator');
registry.registerFeatureServices(featureServiceDefinitions, 'acme:integrator');
```

On the server, the integrator defines the server renderer using the request. The
Expand All @@ -149,7 +149,7 @@ const featureServiceDefinitions = [
defineServerRenderer(request)
];

registry.registerProviders(featureServiceDefinitions, 'acme:integrator');
registry.registerFeatureServices(featureServiceDefinitions, 'acme:integrator');
```

## Root Location Transformer
Expand Down
20 changes: 10 additions & 10 deletions docs/guides/writing-a-feature-service.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const myFeatureServiceDefinition = {
create(env) {
let count = env.config.initialCount || 0;

const v1 = uniqueConsumerId => ({
const v1 = consumerUid => ({
featureService: {
plus() {
count += 1;
Expand Down Expand Up @@ -129,7 +129,7 @@ const myFeatureServiceDefinition = {
create(env) {
let count = env.config.initialCount || 0;

const v1 = uniqueConsumerId => ({
const v1 = consumerUid => ({
featureService: {
plus() {
count += 1;
Expand Down Expand Up @@ -176,7 +176,7 @@ const myFeatureServiceDefinition = {
const decrement = () => void --count;
const increment = () => void ++count;

const v1 = uniqueConsumerId => ({
const v1 = consumerUid => ({
featureService: {
getCount,

Expand All @@ -192,7 +192,7 @@ const myFeatureServiceDefinition = {
}
});

const v2 = uniqueConsumerId => ({
const v2 = consumerUid => ({
featureService: {getCount, increment, decrement}
});

Expand Down Expand Up @@ -220,21 +220,21 @@ const myFeatureServiceDefinition = {
// Shared state lives here.
let consumerCounts = {};

const v1 = uniqueConsumerId => {
const v1 = consumerUid => {
// Consumer-specific state lives here.
consumerCounts[uniqueConsumerId] = 0;
consumerCounts[consumerUid] = 0;

const featureService = {
increment() {
consumerCounts[uniqueConsumerId] += 1;
consumerCounts[consumerUid] += 1;
},

decrement() {
consumerCounts[uniqueConsumerId] -= 1;
consumerCounts[consumerUid] -= 1;
},

getCount() {
return consumerCounts[uniqueConsumerId];
return consumerCounts[consumerUid];
},

getTotalCount() {
Expand All @@ -247,7 +247,7 @@ const myFeatureServiceDefinition = {

const unbind = () => {
// Cleaning up the consumer-specific state.
delete consumerCounts[uniqueConsumerId];
delete consumerCounts[consumerUid];
};

return {featureService, unbind};
Expand Down
6 changes: 3 additions & 3 deletions docs/help/faq.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ sidebar_label: FAQ
## Can the integrator register Feature Services one by one?

Yes, but then you need to guarantee the correct order. On the other hand, all
Feature Services registered **together** using the `registerProviders` method of
the `FeatureServiceRegistry` are automatically sorted topologically and
therefore do not need to be registered in the correct order.
Feature Services registered **together** using the `registerFeatureServices`
method of the `FeatureServiceRegistry` are automatically sorted topologically
and therefore do not need to be registered in the correct order.

## Can Feature Services be dynamically loaded after initial load?

Expand Down
80 changes: 39 additions & 41 deletions packages/core/src/__tests__/feature-app-manager.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import {FeatureAppModule} from '../internal/is-feature-app-module';

interface MockFeatureServiceRegistry extends FeatureServiceRegistryLike {
registerProviders: jest.Mock;
registerFeatureServices: jest.Mock;
bindFeatureServices: jest.Mock;
}

Expand All @@ -34,7 +34,7 @@ describe('FeatureAppManager', () => {
};

mockRegistry = {
registerProviders: jest.fn(),
registerFeatureServices: jest.fn(),
bindFeatureServices: jest.fn(() => mockFeatureServicesBinding)
};

Expand All @@ -60,7 +60,7 @@ describe('FeatureAppManager', () => {
});
});

it('logs an info message when the feature app module was loaded', async () => {
it('logs an info message when the Feature App module was loaded', async () => {
const asyncFeatureAppDefinition = manager.getAsyncFeatureAppDefinition(
'/example.js'
);
Expand All @@ -71,12 +71,12 @@ describe('FeatureAppManager', () => {

expect(spyConsoleInfo.mock.calls).toEqual([
[
'The feature app module for the url "/example.js" has been successfully loaded.'
'The Feature App module at the url "/example.js" has been successfully loaded.'
]
]);
});

it('returns an async value for a feature app definition', async () => {
it('returns an async value for a Feature App definition', async () => {
const asyncFeatureAppDefinition = manager.getAsyncFeatureAppDefinition(
'/example.js'
);
Expand All @@ -98,7 +98,7 @@ describe('FeatureAppManager', () => {
{default: {id: 'testId'}},
{default: {create: jest.fn()}}
]) {
describe(`when an invalid feature app module (${JSON.stringify(
describe(`when an invalid Feature App module (${JSON.stringify(
invalidFeatureAppModule
)}) has been loaded`, () => {
beforeEach(() => {
Expand All @@ -108,7 +108,7 @@ describe('FeatureAppManager', () => {

it('throws an error (and stores it on the async value)', async () => {
const expectedError = new Error(
'The feature app module at url "/example.js" is invalid. A feature app module must have a feature app definition as default export. A feature app definition is an object with at least an `id` string and a `create` method.'
'The Feature App module at the url "/example.js" is invalid. A Feature App module must have a Feature App definition as default export. A Feature App definition is an object with at least an `id` string and a `create` method.'
);

await expect(
Expand All @@ -130,8 +130,8 @@ describe('FeatureAppManager', () => {
});
}

describe('for a known feature app module url', () => {
it('returns the same async feature app definition', () => {
describe('for a known Feature App module url', () => {
it('returns the same async Feature App definition', () => {
const asyncFeatureAppDefinition = manager.getAsyncFeatureAppDefinition(
'/example.js'
);
Expand All @@ -144,7 +144,7 @@ describe('FeatureAppManager', () => {
});

describe('#getFeatureAppScope', () => {
it('creates a feature app with a consumer environment using the service registry', () => {
it('creates a Feature App with a consumer environment using the Feature Service registry', () => {
const mockConfig = {kind: 'test'};
const idSpecifier = 'testIdSpecifier';

Expand All @@ -165,14 +165,14 @@ describe('FeatureAppManager', () => {
]);
});

describe('with a feature app definition with own feature service definitions', () => {
describe('with a Feature App definition with own Feature Service definitions', () => {
let registryMethodCalls: string[];

beforeEach(() => {
registryMethodCalls = [];

mockRegistry.registerProviders.mockImplementation(() => {
registryMethodCalls.push('registerProviders');
mockRegistry.registerFeatureServices.mockImplementation(() => {
registryMethodCalls.push('registerFeatureServices');
});

mockRegistry.bindFeatureServices.mockImplementation(() => {
Expand All @@ -187,10 +187,10 @@ describe('FeatureAppManager', () => {
};
});

it("registers the feature app's own feature service definitions before binding the feature services", () => {
it("registers the Feature App's own Feature Services before binding its required Feature Services", () => {
manager.getFeatureAppScope(mockFeatureAppDefinition, 'testIdSpecifier');

expect(mockRegistry.registerProviders.mock.calls).toEqual([
expect(mockRegistry.registerFeatureServices.mock.calls).toEqual([
[mockFeatureAppDefinition.ownFeatureServiceDefinitions, 'testId']
]);

Expand All @@ -199,25 +199,23 @@ describe('FeatureAppManager', () => {
]);

expect(registryMethodCalls).toEqual([
'registerProviders',
'registerFeatureServices',
'bindFeatureServices'
]);
});
});

describe('for a known feature app definition', () => {
describe('for a known Feature App definition', () => {
describe('and no id specifier', () => {
it('logs an info message after creation', () => {
manager.getFeatureAppScope(mockFeatureAppDefinition);

expect(spyConsoleInfo.mock.calls).toEqual([
[
'The feature app scope for the ID "testId" has been successfully created.'
]
['The Feature App "testId" has been successfully created.']
]);
});

it('returns the same feature app scope', () => {
it('returns the same Feature App scope', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition
);
Expand All @@ -227,8 +225,8 @@ describe('FeatureAppManager', () => {
);
});

describe('when destroy() is called on the feature app scope', () => {
it('returns another feature app scope', () => {
describe('when destroy() is called on the Feature App scope', () => {
it('returns another Feature App scope', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition
);
Expand All @@ -251,12 +249,12 @@ describe('FeatureAppManager', () => {

expect(spyConsoleInfo.mock.calls).toEqual([
[
'The feature app scope for the ID "testId" with the specifier "testIdSpecifier" has been successfully created.'
'The Feature App "testId:testIdSpecifier" has been successfully created.'
]
]);
});

it('returns the same feature app scope', () => {
it('returns the same Feature App scope', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition,
'testIdSpecifier'
Expand All @@ -270,8 +268,8 @@ describe('FeatureAppManager', () => {
).toBe(featureAppScope);
});

describe('when destroy() is called on the feature app scope', () => {
it('returns another feature app scope', () => {
describe('when destroy() is called on the Feature App scope', () => {
it('returns another Feature App scope', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition,
'testIdSpecifier'
Expand All @@ -290,7 +288,7 @@ describe('FeatureAppManager', () => {
});

describe('and a different id specifier', () => {
it('returns another feature app scope', () => {
it('returns another Feature App scope', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition
);
Expand All @@ -306,7 +304,7 @@ describe('FeatureAppManager', () => {
});

describe('#featureApp', () => {
it("is the feature app that the feature app definition's create returns", () => {
it("is the Feature App that the Feature App definition's create returns", () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition
);
Expand All @@ -316,7 +314,7 @@ describe('FeatureAppManager', () => {
});

describe('#destroy', () => {
it('unbinds the bound feature services', () => {
it('unbinds the bound Feature Services', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition
);
Expand All @@ -334,14 +332,14 @@ describe('FeatureAppManager', () => {

featureAppScope.destroy();

expect(() =>
featureAppScope.destroy()
).toThrowErrorMatchingInlineSnapshot(
'"The feature app scope for the ID \\"testId\\" and its specifier \\"testIdSpecifier\\" could not be destroyed."'
expect(() => featureAppScope.destroy()).toThrowError(
new Error(
'The Feature App "testId:testIdSpecifier" could not be destroyed.'
)
);
});

it('fails to destroy an already destroyed feature app scope, even if this scope has been re-created', () => {
it('fails to destroy an already destroyed Feature App scope, even if this scope has been re-created', () => {
const featureAppScope = manager.getFeatureAppScope(
mockFeatureAppDefinition,
'testIdSpecifier'
Expand All @@ -350,17 +348,17 @@ describe('FeatureAppManager', () => {
featureAppScope.destroy();
manager.getFeatureAppScope(mockFeatureAppDefinition, 'testIdSpecifier');

expect(() =>
featureAppScope.destroy()
).toThrowErrorMatchingInlineSnapshot(
'"The feature app scope for the ID \\"testId\\" and its specifier \\"testIdSpecifier\\" could not be destroyed."'
expect(() => featureAppScope.destroy()).toThrowError(
new Error(
'The Feature App "testId:testIdSpecifier" could not be destroyed.'
)
);
});
});
});

describe('#destroy', () => {
it('unbinds the bound feature services for all feature apps', () => {
it('unbinds the bound Feature Services for all Feature Apps', () => {
manager.getFeatureAppScope(mockFeatureAppDefinition, 'test1');
manager.getFeatureAppScope(mockFeatureAppDefinition, 'test2');
manager.destroy();
Expand All @@ -376,7 +374,7 @@ describe('FeatureAppManager', () => {
});
});

it('preloads a feature app so that the scope is synchronously available', async () => {
it('preloads a Feature App definition so that the scope is synchronously available', async () => {
await manager.preloadFeatureApp('/example.js');

const asyncFeatureAppDefinition = manager.getAsyncFeatureAppDefinition(
Expand Down
Loading

0 comments on commit 45c8906

Please sign in to comment.