New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internal services #45

Merged
merged 13 commits into from May 20, 2015

Conversation

Projects
None yet
2 participants
@mawolf

mawolf commented May 18, 2015

Adding support for internal services

}
else if (this.internalServiceProgram != null) {
program = this.internalServiceProgram;
OLParseTreeOptimizer optimizer = new OLParseTreeOptimizer( program );

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

Looks like this code is duplicated with the one in the next branch.

this.type = type;
}

public Constants.EmbeddedServiceType getType()

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

Please rename getType() to type()

null,
protocolConfiguration,
new InputPortInfo.AggregationItemInfo[] {},
new HashMap<String, String>() );

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

Use Collections.emptyMap()

//initialize internal interface and interface list
List< InterfaceDefinition > interfaceList = new ArrayList< InterfaceDefinition >();

OLSyntaxNode internal_main = null;

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

internal_main -> internalMain

List< InterfaceDefinition > interfaceList = new ArrayList< InterfaceDefinition >();

OLSyntaxNode internal_main = null;
SequenceStatement internal_init = null;

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

internal_init -> internalInit

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

internal_init could probably just be OLSyntaxNode, SequenceStatement seems redundant.

@@ -30,6 +32,8 @@
private final String servicePath;
private final String portId;
private final Constants.EmbeddedServiceType type;

private final List< OLSyntaxNode > children = new ArrayList< OLSyntaxNode > ();

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

Why a generic children list, and not just store a Program reference?

@@ -0,0 +1,458 @@
/***************************************************************************

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

Where is this class used? Please remove it if unused (we could move it to an external library though maybe... but let's leave it for another patch).

if ( child instanceof TypeInlineDefinition ||
child instanceof InterfaceDefinition ||
child instanceof OutputPortInfo ||
child instanceof EmbeddedServiceNode )

This comment has been minimized.

@fmontesi

fmontesi May 18, 2015

Member

This does not look like the right way of implementing the first requirement in Semantics of #27 .
It's going to duplicate embedded services, which could be a problem in case of sub-services written in Jolie (and even Java, although many are harmless, like Console).

What we should do is implement some mechanism for the internal service to wait for the bindings for the output ports at runtime. This doesn't look too easy, we need to think of the lifecycle defined in:

https://github.com/jolie/jolie/blob/master/jolie/src/jolie/process/InitDefinitionProcess.java

Embedded services are loaded before the init procedure starts, so the internal service can't wait for the parent's init to end before "copying over" the bindings for the output ports or we'll create a deadlock.

@fmontesi

This comment has been minimized.

Member

fmontesi commented May 18, 2015

Great patch, thanks. :-)

Indentation looks weird sometimes. Please follow the style of the rest of the code (spaces in parentheses) and use tabs instead of spaces.

@jolie/developers More eyeballs on the patch are welcome.

@fmontesi fmontesi self-assigned this May 18, 2015

@fmontesi fmontesi added this to the v1.4 milestone May 18, 2015

@fmontesi

This comment has been minimized.

Member

fmontesi commented May 18, 2015

The formatting rules I used (updated to last year I think, but netbeans should not have changed so much hopefully?..)

http://filebin.ca/22Ay89ym7QDR/formatting.zip

@mawolf

This comment has been minimized.

mawolf commented May 19, 2015

Again, thank you for the comments and recommendations!

The latest commits should fix the concerns that you had. Let me know if further changes are needed!

@fmontesi fmontesi merged commit c18194a into jolie:master May 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment