This pattern repeats:

https://github.com/bpxe/bpxe/blob/0828554a14ebbdc2c38b7564443c0036b0f05ab0/pkg/clock/mock.go#L53-L59

	m.Lock()
	ch := make(chan time.Time, 1)
	if duration.Nanoseconds() <= 0 {
		ch <- m.now
		close(ch)
		return ch
	}

This leaves the lock taken. Should use

m.Lock()
defer m.Unlock()

https://github.com/bpxe/bpxe/blob/master/CODEREVIEWS.md#how-do-i-submit-reviews

Simply open an issue and assign the code review label to it.

Without extra access to the repository, people cannot do that.

https://github.com/bpxe/bpxe/blob/0828554a14ebbdc2c38b7564443c0036b0f05ab0/pkg/clock/host_generic.go#L28

				if t1.Before(t1) {

You can be pretty darn sure that t1 is never before itself.

Your FOSSA integration was successful! Attached in this PR is a badge and license report to track scan status in your README.

Below are docs for integrating FOSSA license checks into your CI:

• CircleCI
• TravisCI
• Jenkins
• Other

Problem: passing instance information into traces

Currently, traces produced by many components don't point to particular process instances they are produced in. This will make results of future analysis or querying of such information ambiguous.

Solution: allow traces to be wrapped and unwrapped

This change introduces the concept of trace wrapping (primarily through the newly introduced WrappedTrace interface and unwrapping (though WrappedTrace.Unwrap() and tracing.Unwrap() functions).

Tracers can be contextualized by creating a new tracer that will transform produced traces (see tracing.NewTraceTransformingTracer) and passing it to components in a given context.

Also, there's another problem that have been observed during the development of this feature:

Problem: process instance observing traces from outside

Currently, there's no reason for process instance components to observe traces from outside of its own scope; in fact, it'll likely mistreat traces from other instances or other processes as its own.

Solution: compartmentalize tracers

This way, instance gets its own tracer and its tracer are relayed to process's tracer and so on. This removes transforming tracer as it is no longer necessary. Instead, tracer relay does its job.

I am still not 100% confident this is the right approach but it is better than leaving things as they are with scopes bleeding into other scopes. I am trying to think if there's any reason for observing traces outside of tracing compartment. If there will be such a need, we'll need to wire it through accordingly.

Greetings, fire here!

@yrashk recently invited @fire-bot to this repository. Before fire is enabled, @yrashk needs to complete a few steps:

Complete fire setup now

Only @yrashk will be able to enable fire using the above link. If someone added fire by mistake, feel free to remove @fire-bot from your repo's collaborators.

Some of the code has branching with its own unlocking of mutexes. This code is error-prone as it's easy to forget to unlock.

Solution: use deferred unlocks where feasible

Previous implementation was based on a now-outdated knowledge that deferrals are slower. As it was pointed out in #68, this is no longer true as of Go 1.14. My own measurements seem to confirm this as well.

So, where it was reasonable, I've rewritten unlocking to happen in deferred statements.

It suggests to change time within a docker container, but it'll actually change system time, because we give that capability to the container.

Solution: rewrite the advice to use a virtual machine and provide a Vagrant setup for that, as well as instructions.

When After takes the fast path of returning when the duration is zero or negative, it doesn't unlock its mutex.

Solution: unlock it (just like Until does)

Addresses #68

There's a test that tests whether a certain time is before itself. That's clearly impossible.

Solution: check whether it's before previously known time

While we're at it, update current time on every iteration of the loop.

The reason for this mishap is that this piece of code is not well tested yet (since it can be only tested manually)

Addresses #64

The error seems to indicate that something is being received from the channel when it's not expected to.

Solution: handle channel closure

Since timers close channels when it is safe to do so, sometimes tests receive another (zero) message that indicates channel closure.

This change handles this behavior by observing optional flag that indicates whether the channel is closed.

Hi,

I really appreciate your work in building such a BPMN engine in Go. I'm quite impressed by your problem statement because it's exactly my thinking. Really great job done 👍

One sentence caught my attention - the usage one: "BPXE does not have an executable server of its own and can be only used as a Go library." Frankly said that's exactly, what I'm looking for, an engine as a library.

Let me explain, why I think splitting BPXE into lib and server as separate projects would be a better idea. Every point you mentioned about business is interested in transparency and wants to know what is implemented is right. So the benefits of BPMN having a modeler and thus some "live documentation" is great. On the other hand, when you want to deploy a (or any) workflow engine into a server, you're faced with questions about partitioning, consistency, availability (CAP theorem). What I mean: as the author of this project, you would need to support the most common database, fiddle with their quirks, ensure multiple server instances can run (high availability), and make this overall very responsive. That's a lot of work, which not everyone needs.

Think for a moment, bpxe would have no DB persistency implemented and just allows other developers to load BPMN files, and execute them in memory only - period. This would lower the barrier to include such a lib instead of developing homebrew state machines or what-not, with having full documentation and modeling support available.

That said, the bpxe engine would be focused on being a lib without DB support, one would certainly need two handfuls of callbacks/hooks so that application developers can react to state changes of the BPMN process state. They would follow their existing paradigms of putting the state into the DB with their preferred DB technology.

Bare with me, when I share this example from the Javascript space ... https://github.com/paed01/bpmn-engine/blob/HEAD/docs/Examples.md This engine is built as lib only.

Considering the lib offering such interfaces, you're not blocked and still be able to develop another project, which then would/could focus on being a server incl. database support, UI etc.

I hope my thought is not offending, because I really love Open Source and collaboration around it. And I would be very glad to get some reflections from you and this idea.

Sincerely Martin

https://github.com/bpxe/bpxe/blob/0828554a14ebbdc2c38b7564443c0036b0f05ab0/pkg/bpmn/schema.go#L19-L29

type QName = string
type Id = string

This lets callers intermingle the types freely, removing any benefit of using the types. Should use

type QName string
type Id string

and so on.

https://play.golang.org/p/k8vgBkqdmwr

package main

import (
	"fmt"
)

type Foo = string
type Bar = string

func demo(b Bar) {
	fmt.Printf("i'm happy to have receiver a bar: %v\n", b)
}

func main() {
	var f Foo = "i'm a foo"
	demo(f)
}

Also, ID not Id: https://github.com/golang/go/wiki/CodeReviewComments#initialisms

Currently, testing BPXE against different cases / BPMN documents is a bit verbose, and it's sometimes hard to debug.

Proposed solution (1): extract more primitives to simplify testing

Proposed solution (2): externalize testing by creating a common tracing format as well as live querying / event injection.

This is a larger undertaking than (1) but it has the following benefits:

1. [Live] trace querying is one of the most important features BPXE will provide, so it needs to be done anyway
2. It will allow to test any BPMN engine that can produce the same tracing format.

I think proposed solutions (1) and (2) aren't exactly mutually exclusive.

As mentioned in 460473db4420e2c14185afe65dcba3f8a8e2a1a6, one can't instantiate a process using a Receive Task, simply because Receive Task hasn't been implemented.

As mentioned in 460473db4420e2c14185afe65dcba3f8a8e2a1a6, one can't instantiate a process using an Event-Based Gateway, simply because it's not a handled use case yet.

Currently (as of 0154b48a964c93f02947029be367730974cea4f1) https://github.com/bpxe/bpxe/blob/0154b48a964c93f02947029be367730974cea4f1/pkg/flow_node/gateway/event_based_gateway/tests/event_based_gateway_test.go#L18-L25 only tests event-based gateway that it'll follow a certain path in presence of just one event, but it doesn't test what will happen if both events will occur.

Proposed solution: figure out how to improve the test to support this case

Naive implementation like this one below blocks somewhere:

testEventBasedGateway(t, func(reached map[string]int) {
    assert.True(t, reached["task1"] == 1 || reached["task2"] == 1)
    assert.Equal(t, 1, reached["end"])
}, event.NewMessageEvent("Msg1", nil), event.NewSignalEvent("Sig1"))

Not yet sure if this is due to how the test function works or due to a bug in the implementation.