Skip to content
This repository has been archived by the owner on Apr 29, 2022. It is now read-only.

Commit

Permalink
Merge pull request #116 from applandinc/fix/sql-transaction-scope
Browse files Browse the repository at this point in the history
Correctly enumerate transaction events
  • Loading branch information
kgilpin authored Mar 17, 2022
2 parents 93d57c5 + ecc9bfc commit 22fb0a0
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/scope/commandScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const Command = 'command';
const Job = 'job';

export default class CommandScope extends ScopeIterator {
*scopes(events: Generator<Event>): Generator<Scope> {
*scopes(events: IterableIterator<Event>): Generator<Scope> {
for (const event of events) {
if (
event.isCall() &&
Expand Down
2 changes: 1 addition & 1 deletion src/scope/rootScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ScopeImpl from './scopeImpl';
import ScopeIterator from './scopeIterator';

export default class RootScope extends ScopeIterator {
*scopes(events: Generator<Event>): Generator<Scope> {
*scopes(events: IterableIterator<Event>): Generator<Scope> {
for (const event of events) {
if (event.isCall() && !event.parent) {
yield new ScopeImpl(event);
Expand Down
4 changes: 2 additions & 2 deletions src/scope/scopeIterator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { Event } from '@appland/models';
import { Scope } from 'src/types';

export default abstract class ScopeIterator {
abstract scopes(events: Generator<Event>): Generator<Scope>;
abstract scopes(events: Iterable<Event>): Generator<Scope>;

// Scan ahead past the return event of the yielded scope.
protected advanceToReturnEvent(scopeEvent: Event, events: Generator<Event>): void {
protected advanceToReturnEvent(scopeEvent: Event, events: Iterator<Event>): void {
// Don't use for...of events here, it doesn't work with an outer for...of on the same events generator.
let eventResult = events.next();
while (!eventResult.done) {
Expand Down
4 changes: 2 additions & 2 deletions src/scope/sqlTransactionScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function iterateTransaction(
): Scope {
// since we can only go through the tail once,
// we have to keep the list of events in the transaction
const transaction: Event[] = [];
const transaction = [begin];
for (let next = tail.next(); !next.done; next = tail.next()) {
const event = next.value;
if (!event.isCall()) continue;
Expand Down Expand Up @@ -87,7 +87,7 @@ function iterateTransaction(

return {
scope: begin,
events: transaction[Symbol.iterator] as () => Generator<Event>,
events: transaction[Symbol.iterator].bind(transaction) as () => Generator<Event>,
};
}

Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/appmaps/sqlTransactionScopeTest.appmap.json
Git LFS file not shown
48 changes: 0 additions & 48 deletions test/scope.spec.ts

This file was deleted.

16 changes: 16 additions & 0 deletions test/scope/commandScope.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import CommandScope from '../../src/scope/commandScope';
import { fixtureAppMap } from '../util';

describe('command scope', () => {
it('emits http_server_request command with child events', async () => {
const appmap = await fixtureAppMap(
'org_springframework_samples_petclinic_owner_OwnerControllerTests_testInitCreationForm.appmap.json'
);
const scopes = [...new CommandScope().scopes(appmap.events[Symbol.iterator]())];
expect(scopes.map((scope) => scope.scope.id)).toEqual([1]);
expect(scopes.map((scope) => scope.scope.toString())).toEqual(['GET /owners/new']);

const requestEvents = Array.from(scopes[0].events());
expect(requestEvents.length).toEqual(4);
});
});
22 changes: 22 additions & 0 deletions test/scope/rootScope.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import RootScope from '../../src/scope/rootScope';
import { fixtureAppMap } from '../util';

describe('root scope', () => {
it('emits a scope for each root event', async () => {
const appmap = await fixtureAppMap(
'org_springframework_samples_petclinic_owner_OwnerControllerTests_testInitCreationForm.appmap.json'
);
const scopes = [...new RootScope().scopes(appmap.events[Symbol.iterator]())];
expect(scopes.map((scope) => scope.scope.id)).toEqual([1, 9]);
expect(scopes.map((scope) => scope.scope.toString())).toEqual([
'GET /owners/new',
'org/springframework/samples/petclinic/model/BaseEntity#isNew',
]);

const requestEvents = Array.from(scopes[0].events());
expect(requestEvents.length).toEqual(4);

const fnEvents = Array.from(scopes[1].events());
expect(fnEvents.length).toEqual(1);
});
});
40 changes: 40 additions & 0 deletions test/scope/sqlTransactionScope.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import assert from 'node:assert';
import SQLTransactionScope, { hasTransactionDetails } from '../../src/scope/sqlTransactionScope';
import { Scope } from '../../src/types';
import { fixtureAppMap } from '../util';

function next<T>(iter: Iterator<T>): T {
const result = iter.next();
assert(!result.done);
return result.value;
}

describe('SQL transaction scope', () => {
it('emits a scope for each SQL transaction', async () => {
const appMap = await fixtureAppMap('sqlTransactionScopeTest.appmap.json');

const scopes = new SQLTransactionScope().scopes(appMap.events[Symbol.iterator]());

const committed: Scope = next(scopes);
assert(hasTransactionDetails(committed.scope));
expect(committed.scope.transaction.status).toBe('commit');
expect([...committed.events()].length).toBe(3);

const rolledBack: Scope = next(scopes);
assert(hasTransactionDetails(rolledBack.scope));
expect(rolledBack.scope.transaction.status).toBe('rollback');
expect([...rolledBack.events()].length).toBe(3);

const mockWarn = jest.spyOn(console, 'warn').mockImplementation();
const rest: Scope = next(scopes);
assert(hasTransactionDetails(rest.scope));
expect(rest.scope.transaction.status).toBe('rollback');
expect([...rest.events()].length).toBe(2);
expect(mockWarn).toHaveBeenCalledWith(
expect.stringContaining('transaction started within a transaction')
);
mockWarn.mockRestore();

expect(scopes.next().done).toBeTruthy();
});
});

0 comments on commit 22fb0a0

Please sign in to comment.