Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

Feature/simple rules #470

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

etschelp
Copy link
Contributor

@etschelp etschelp commented Jun 11, 2021

@etschelp etschelp marked this pull request as draft June 11, 2021 15:28
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
Signed-off-by: Philipp Etschel <[email protected]>
…-partner-agent into feature/simple-rules

Signed-off-by: Philipp Etschel <[email protected]>

# Conflicts:
#	backend/README.md
#	backend/pom.xml
Signed-off-by: Philipp Etschel <[email protected]>
Copy link
Contributor

@l-wegner l-wegner left a comment

Choose a reason for hiding this comment

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

I made some minor suggestions.


@Inject
@Named("rules")
EventHandler rulesEventHandler;

@Post(WEBHOOK_CONTROLLER_PATH + "/{eventType}")
public void logEvent(
Copy link
Contributor

Choose a reason for hiding this comment

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

rename method? It does not solely log.
accept event? consume?

abstract boolean apply(EventContext ctx);

@SuppressWarnings("unused")
private String type;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this field? It's not used via accessors and builders. The database stores Triggers as jsonb.

How about pulling this field into the subclasses, if there is a purpose for it. And then change Trigger to an interface, which is the also a functional one. Or does that create serialization issues?

abstract void run(EventContext ctx);

@SuppressWarnings("unused")
private String type;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as Trigger.type?


@Override
public boolean apply(EventContext ctx) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add a TODO, that this can be implemented as soon as proof templates are available?

public static final String CONNECTION_TRIGGER_NAME = "connection";
public static final String PROOF_RECEIVED_TRIGGER_NAME = "proof_received";

abstract boolean apply(EventContext ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply is a very generic name.
shallApplyAction
applicableRule

@Override
public boolean apply(EventContext ctx) {
ConnectionRecord connRec = ctx.getConnRec();
boolean apply = connRec != null && ConnectionState.REQUEST.equals(connRec.getState());
Copy link
Contributor

Choose a reason for hiding this comment

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

boolean isRequest = connRec != null && ConnectionState.REQUEST.equals(connRec.getState());
boolean roleMatches = role==null || connRec != null && role.equals(connRec.getTheirRole());
return isRequest && roleMatches;

ConnectionRecord connRec = ctx.getConnRec();
boolean apply = connRec != null && ConnectionState.REQUEST.equals(connRec.getState());
if (role != null) {
apply = apply && role.equals(connRec.getTheirRole());
Copy link
Contributor

Choose a reason for hiding this comment

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

connRec could be null?

private Trigger trigger;
private Action action;

@JsonTypeInfo(
Copy link
Contributor

Choose a reason for hiding this comment

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

I used @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS) to store the type information. Is there a decision, which one to use?


public List<RulesData> getAll() {
List<RulesData> result = new ArrayList<>();
rr.findAll().forEach(active -> result.add(RulesData.fromActive(active)));
Copy link
Contributor

Choose a reason for hiding this comment

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

return rr.findAll().stream().map(RulesData::fromActive).collect(Collectors.toList());

Partner p = ModelBuilder.buildDefaultPartner().setConnectionId(connectionId);
pr.save(p);

rs.add(new RulesData.Trigger.ConnectionTrigger(), new RulesData.Action.TagConnection());
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe put a special action for the rule to verify it's execution? There are mutltiple path, where the ruleExecution is stopped before that.

Signed-off-by: Philipp Etschel <[email protected]>

# Conflicts:
#	backend/business-partner-agent/pom.xml
#	backend/business-partner-agent/src/main/java/org/hyperledger/bpa/controller/AriesWebhookController.java
#	backend/business-partner-agent/src/main/java/org/hyperledger/bpa/impl/aries/ConnectionManager.java
Signed-off-by: Philipp Etschel <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants