Skip to content

Commit

Permalink
fix: fix miss MultiInstance proper qualifiers (#241)
Browse files Browse the repository at this point in the history
<!--
Thank you for your pull request. Please review below requirements.
Bug fixes and new features should include tests and possibly benchmarks.
Contributors guide:
https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md

感谢您贡献代码。请确认下列 checklist 的完成情况。
Bug 修复和新功能必须包含测试,必要时请附上性能测试。
Contributors guide:
https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md
-->

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to
[x]. -->

- [ ] `npm test` passes
- [ ] tests and/or benchmarks are included
- [ ] documentation is changed or added
- [ ] commit message follows commit guidelines

##### Affected core subsystem(s)
<!-- Provide affected core subsystem(s). -->


##### Description of change
<!-- Provide a description of the change below this comment. -->

<!--
- any feature?
- close https://github.com/eggjs/egg/ISSUE_URL
-->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Enhanced handling of qualifiers for prototype instances, improving the
prototype resolution process.
- Updated configuration for the `BizManager` module to include distinct
secret values for clients.

- **Bug Fixes**
- Improved logic for managing qualifiers in `ClazzMap` and `ModuleNode`
classes.

- **Documentation**
- Updated comments and documentation to reflect changes in qualifier
handling and prototype management.

- **Refactor**
- Simplified the prototype handling logic by consolidating
multi-instance prototype management into the `ModuleGraph` class.

- **Style**
	- Cleaned up code formatting and organization for better readability.

- **Tests**
- Adjusted tests to accommodate changes in the `BizManager` and
qualifier handling logic.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
killagu authored Sep 30, 2024
1 parent 4790595 commit 15666d3
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 121 deletions.
21 changes: 18 additions & 3 deletions core/core-decorator/src/util/PrototypeUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import {
EggMultiInstancePrototypeInfo,
EggProtoImplClass,
EggPrototypeInfo,
EggPrototypeName,
EggPrototypeName, InitTypeQualifierAttribute,
InjectConstructorInfo,
InjectObjectInfo,
InjectType,
InjectType, LoadUnitNameQualifierAttribute,
MultiInstancePrototypeGetObjectsContext, QualifierAttribute,
} from '@eggjs/tegg-types';
import { MetadataUtil } from './MetadataUtil';
Expand Down Expand Up @@ -141,9 +141,24 @@ export class PrototypeUtil {
}
const callBackMetadata = MetadataUtil.getMetaData<EggMultiInstanceCallbackPrototypeInfo>(PrototypeUtil.MULTI_INSTANCE_PROTOTYPE_CALLBACK_PROPERTY, clazz);
if (callBackMetadata) {
const objects = callBackMetadata.getObjects(ctx);
const defaultQualifier = [{
attribute: InitTypeQualifierAttribute,
value: callBackMetadata.initType,
}, {
attribute: LoadUnitNameQualifierAttribute,
value: ctx.moduleName,
}];
for (const object of objects) {
defaultQualifier.forEach(qualifier => {
if (!object.qualifiers.find(t => t.attribute === qualifier.attribute)) {
object.qualifiers.push(qualifier);
}
});
}
return {
...callBackMetadata,
objects: callBackMetadata.getObjects(ctx),
objects,
};
}
}
Expand Down
19 changes: 18 additions & 1 deletion core/metadata/src/factory/EggPrototypeCreatorFactory.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { PrototypeUtil } from '@eggjs/core-decorator';
import { InitTypeQualifierAttribute, LoadUnitNameQualifierAttribute, PrototypeUtil } from '@eggjs/core-decorator';
import type {
EggProtoImplClass,
EggPrototypeInfo,
Expand Down Expand Up @@ -28,12 +28,29 @@ export class EggPrototypeCreatorFactory {
moduleName: loadUnit.name,
})!;
for (const obj of multiInstanceProtoInfo.objects) {
const defaultQualifier = [{
attribute: InitTypeQualifierAttribute,
value: PrototypeUtil.getInitType(clazz, {
unitPath: loadUnit.unitPath,
moduleName: loadUnit.name,
})!,
}, {
attribute: LoadUnitNameQualifierAttribute,
value: loadUnit.name,
}];
defaultQualifier.forEach(qualifier => {
if (!obj.qualifiers.find(t => t.attribute === qualifier.attribute)) {
obj.qualifiers.push(qualifier);
}
});

properties.push({
name: obj.name,
protoImplType: multiInstanceProtoInfo.protoImplType,
initType: multiInstanceProtoInfo.initType,
accessLevel: multiInstanceProtoInfo.accessLevel,
qualifiers: obj.qualifiers,
properQualifiers: obj.properQualifiers,
className: multiInstanceProtoInfo.className,
});
}
Expand Down
28 changes: 19 additions & 9 deletions core/metadata/src/impl/EggPrototypeBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class EggPrototypeBuilder {
private injectObjects: Array<InjectObject | InjectConstructor> = [];
private loadUnit: LoadUnit;
private qualifiers: QualifierInfo[] = [];
private properQualifiers: Record<string, QualifierInfo[]> = {};
private className?: string;
private multiInstanceConstructorIndex?: number;
private multiInstanceConstructorAttributes?: QualifierAttribute[];
Expand All @@ -56,38 +57,47 @@ export class EggPrototypeBuilder {
...QualifierUtil.getProtoQualifiers(clazz),
...(ctx.prototypeInfo.qualifiers ?? []),
];
console.log('proto: ', ctx.prototypeInfo.properQualifiers);
builder.properQualifiers = ctx.prototypeInfo.properQualifiers ?? {};
builder.multiInstanceConstructorIndex = PrototypeUtil.getMultiInstanceConstructorIndex(clazz);
builder.multiInstanceConstructorAttributes = PrototypeUtil.getMultiInstanceConstructorAttributes(clazz);
return builder.build();
}

private tryFindDefaultPrototype(injectObject: InjectObject): EggPrototype {
const propertyQualifiers = QualifierUtil.getProperQualifiers(this.clazz, injectObject.refName);
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, propertyQualifiers);
const multiInstancePropertyQualifiers = this.properQualifiers[injectObject.refName as string] ?? [];
console.log('multi instance: ', this.properQualifiers, injectObject.refName);
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, [
...propertyQualifiers,
...multiInstancePropertyQualifiers,
]);
}

private tryFindContextPrototype(injectObject: InjectObject): EggPrototype {
let propertyQualifiers = QualifierUtil.getProperQualifiers(this.clazz, injectObject.refName);
propertyQualifiers = [
const propertyQualifiers = QualifierUtil.getProperQualifiers(this.clazz, injectObject.refName);
const multiInstancePropertyQualifiers = this.properQualifiers[injectObject.refName as string] ?? [];
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, [
...propertyQualifiers,
...multiInstancePropertyQualifiers,
{
attribute: InitTypeQualifierAttribute,
value: ObjectInitType.CONTEXT,
},
];
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, propertyQualifiers);
]);
}

private tryFindSelfInitTypePrototype(injectObject: InjectObject): EggPrototype {
let propertyQualifiers = QualifierUtil.getProperQualifiers(this.clazz, injectObject.refName);
propertyQualifiers = [
const propertyQualifiers = QualifierUtil.getProperQualifiers(this.clazz, injectObject.refName);
const multiInstancePropertyQualifiers = this.properQualifiers[injectObject.refName as string] ?? [];
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, [
...propertyQualifiers,
...multiInstancePropertyQualifiers,
{
attribute: InitTypeQualifierAttribute,
value: this.initType,
},
];
return EggPrototypeFactory.instance.getPrototype(injectObject.objName, this.loadUnit, propertyQualifiers);
]);
}

private findInjectObjectPrototype(injectObject: InjectObject): EggPrototype {
Expand Down
109 changes: 33 additions & 76 deletions core/metadata/src/impl/ModuleLoadUnit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,17 @@ class ProtoNode implements GraphNodeObj {
readonly qualifiers: QualifierInfo[];
readonly initType: ObjectInitTypeLike;

constructor(clazz: EggProtoImplClass, objName: EggPrototypeName, unitPath: string, moduleName: string) {
constructor(
clazz: EggProtoImplClass,
objName: EggPrototypeName,
initType: ObjectInitTypeLike,
qualifiers: QualifierInfo[],
) {
this.name = objName;
this.id = '' + (id++);
this.clazz = clazz;
this.qualifiers = QualifierUtil.getProtoQualifiers(clazz);
this.initType = PrototypeUtil.getInitType(clazz, {
unitPath,
moduleName,
})!;
this.qualifiers = qualifiers;
this.initType = initType;
}

verifyQualifiers(qualifiers: QualifierInfo[]): boolean {
Expand All @@ -65,77 +67,21 @@ class ProtoNode implements GraphNodeObj {
}
}

class MultiInstanceProtoNode implements GraphNodeObj {
readonly clazz: EggProtoImplClass;
readonly name: EggPrototypeName;
readonly id: string;
readonly qualifiers: QualifierInfo[];
readonly initType: ObjectInitTypeLike;
readonly unitPath: string;
readonly moduleName: string;

constructor(clazz: EggProtoImplClass, objName: EggPrototypeName, unitPath: string, moduleName: string) {
this.name = objName;
this.id = '' + (id++);
this.clazz = clazz;
this.qualifiers = QualifierUtil.getProtoQualifiers(clazz);
this.initType = PrototypeUtil.getInitType(clazz, {
unitPath,
moduleName,
})!;
this.unitPath = unitPath;
this.moduleName = moduleName;
}

verifyQualifiers(qualifiers: QualifierInfo[]): boolean {
const property = PrototypeUtil.getMultiInstanceProperty(this.clazz, {
unitPath: this.unitPath,
moduleName: this.moduleName,
});
if (!property) {
return false;
}
for (const obj of property.objects) {
const selfQualifiers = [
...this.qualifiers,
...obj.qualifiers,
];
if (this.verifyInstanceQualifiers(selfQualifiers, qualifiers)) {
return true;
}
}
return false;
}

verifyInstanceQualifiers(selfQualifiers: QualifierInfo[], qualifiers: QualifierInfo[]): boolean {
for (const qualifier of qualifiers) {
if (!selfQualifiers.find(t => t.attribute === qualifier.attribute && t.value === qualifier.value)) {
return false;
}
}
return true;
}

toString(): string {
return `${this.clazz.name}@${PrototypeUtil.getFilePath(this.clazz)}`;
}
}

export class ModuleGraph {
private graph: Graph<ProtoNode | MultiInstanceProtoNode>;
private graph: Graph<ProtoNode>;
clazzList: EggProtoImplClass[];
readonly unitPath: string;
readonly name: string;

constructor(clazzList: EggProtoImplClass[], unitPath: string, name: string) {
this.clazzList = clazzList;
this.graph = new Graph<ProtoNode | MultiInstanceProtoNode>();
this.graph = new Graph<ProtoNode>();
this.unitPath = unitPath;
this.name = name;
this.build();
}

private findInjectNode(objName: EggPrototypeName, qualifiers: QualifierInfo[], parentInitTye: ObjectInitTypeLike): GraphNode<ProtoNode | MultiInstanceProtoNode> | undefined {
private findInjectNode(objName: EggPrototypeName, qualifiers: QualifierInfo[], parentInitTye: ObjectInitTypeLike): GraphNode<ProtoNode> | undefined {
let nodes = Array.from(this.graph.nodes.values())
.filter(t => t.val.name === objName)
.filter(t => t.val.verifyQualifiers(qualifiers));
Expand All @@ -156,7 +102,7 @@ export class ModuleGraph {
return nodes[0];
}

const temp: Map<EggProtoImplClass, GraphNode<ProtoNode | MultiInstanceProtoNode>> = new Map();
const temp: Map<EggProtoImplClass, GraphNode<ProtoNode>> = new Map();
for (const node of nodes) {
temp.set(node.val.clazz, node);
}
Expand All @@ -170,17 +116,28 @@ export class ModuleGraph {
}

private build() {
const protoGraphNodes: GraphNode<ProtoNode | MultiInstanceProtoNode>[] = [];
const protoGraphNodes: GraphNode<ProtoNode>[] = [];
for (const clazz of this.clazzList) {
const objNames = PrototypeUtil.getObjNames(clazz, {
unitPath: this.unitPath,
moduleName: this.name,
});
for (const objName of objNames) {
if (PrototypeUtil.isEggMultiInstancePrototype(clazz)) {
protoGraphNodes.push(new GraphNode(new MultiInstanceProtoNode(clazz, objName, this.unitPath, this.name)));
} else {
protoGraphNodes.push(new GraphNode(new ProtoNode(clazz, objName, this.unitPath, this.name)));
if (PrototypeUtil.isEggMultiInstancePrototype(clazz)) {
const properties = PrototypeUtil.getMultiInstanceProperty(clazz, {
unitPath: this.unitPath,
moduleName: this.name,
});
if (properties) {
const qualifiers = QualifierUtil.getProtoQualifiers(clazz);
for (const obj of properties.objects || []) {
const instanceQualifiers = [
...qualifiers,
...obj.qualifiers,
];
protoGraphNodes.push(new GraphNode(new ProtoNode(clazz, obj.name, properties.initType, instanceQualifiers)));
}
}
} else {
const qualifiers = QualifierUtil.getProtoQualifiers(clazz);
const property = PrototypeUtil.getProperty(clazz);
if (property) {
protoGraphNodes.push(new GraphNode(new ProtoNode(clazz, property.name, property.initType, qualifiers)));
}
}
}
Expand Down
35 changes: 19 additions & 16 deletions core/metadata/src/model/AppGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,18 @@ export class ClazzMap {
});
assert(property, `multi instance property not found for ${clazz.name}`);
for (const info of property.objects) {
const instanceQualifiers = [
...qualifiers,
...info.qualifiers,
];
clazzMap[info.name] = clazzMap[info.name] || [];
clazzMap[info.name].push({
name: info.name,
accessLevel: PrototypeUtil.getAccessLevel(clazz, {
unitPath: instanceNode.val.moduleConfig.path,
moduleName: instanceNode.val.moduleConfig.name,
}) as AccessLevel,
qualifiers: [
...qualifiers,
...info.qualifiers,
],
qualifiers: instanceQualifiers,
properQualifiers: info.properQualifiers || {},
instanceModule: instanceNode,
ownerModule: ownerNode,
Expand Down Expand Up @@ -179,18 +180,20 @@ export class ModuleNode implements GraphNodeObj {
if (!this.clazzList.includes(clazz)) {
this.clazzList.push(clazz);
}
const defaultQualifier = [{
attribute: InitTypeQualifierAttribute,
value: PrototypeUtil.getInitType(clazz, {
unitPath: this.moduleConfig.path,
moduleName: this.moduleConfig.name,
})!,
}, {
attribute: LoadUnitNameQualifierAttribute,
value: this.name,
}];
for (const qualifier of defaultQualifier) {
QualifierUtil.addProtoQualifier(clazz, qualifier.attribute, qualifier.value);
if (!PrototypeUtil.isEggMultiInstancePrototype(clazz)) {
const defaultQualifier = [{
attribute: InitTypeQualifierAttribute,
value: PrototypeUtil.getInitType(clazz, {
unitPath: this.moduleConfig.path,
moduleName: this.moduleConfig.name,
})!,
}, {
attribute: LoadUnitNameQualifierAttribute,
value: this.name,
}];
for (const qualifier of defaultQualifier) {
QualifierUtil.addProtoQualifier(clazz, qualifier.attribute, qualifier.value);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
BizManager:
clients:
foo: {}
bar: {}
foo:
secret: '1'
bar:
secret: '2'

secret:
keys:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Inject, SingletonProto } from '@eggjs/core-decorator';
import { Inject, ModuleQualifier, SingletonProto } from '@eggjs/core-decorator';
import { Secret, SecretQualifier } from '../foo/Secret';

@SingletonProto()
export class App2 {
@Inject()
@SecretQualifier('app2')
@ModuleQualifier('app2')
@SecretQualifier('1')
secret: Secret;
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import {
MultiInstanceInfo,
} from '@eggjs/tegg';
import { ModuleConfigUtil } from '@eggjs/tegg-common-util';
import { EggProtoImplClass, QualifierUtil } from '@eggjs/core-decorator';
import { EggProtoImplClass, LoadUnitNameQualifierAttribute, QualifierUtil } from '@eggjs/core-decorator';
import { Secret, SecretQualifierAttribute } from '../foo/Secret';

export const BizManagerQualifierAttribute = Symbol.for('Qualifier.ChatModel');
export const BizManagerQualifierAttribute = Symbol.for('Qualifier.BizManager');
export const BizManagerInjectName = 'bizManager';

export function BizManagerQualifier(chatModelName: string) {
Expand Down Expand Up @@ -41,6 +41,9 @@ export function BizManagerQualifier(chatModelName: string) {
properQualifiers: {
secret: [{
attribute: SecretQualifierAttribute,
value: clients[clientName].secret,
}, {
attribute: LoadUnitNameQualifierAttribute,
value: name,
}],
},
Expand Down
Loading

0 comments on commit 15666d3

Please sign in to comment.