Forum OpenACS Q&A: Service contract Tcl API

Collapse
Posted by Lars Pind on
As promised, a few thoughts on the design of a Tcl API for defining service contracts.

The first thing to find out is the style of definition. We have a number of conventions for defining structured data in Tcl code: ad_page_contract and ad_form both have things like this, form builder uses a very different approach with procedure calls, and we're going to come up with something for defining workflows as well.

So I thought it was a good idea to step back for a second and take a look at what the options are. I'd like something clean, since different syntactical styles will only confuse matters.

STYLE ONE: ARRAY LISTS

acs_sc::contract::new \
        -contract_name [workflow::service_contract::role_default_assignee] \
        -contract_desc "Service contract to get the default assignees for a role from parameters case_id, object_id and role_id" \
        -operations {

    GetObjectType {
        operation_desc "Get the object type for which this operation is valid."
        inputspec {}
        outputspec {object_type:string}
        nargs 0
        iscachable_p "t"
    }

    GetPrettyName {
        operation_desc "Get the pretty name. May contain #...#, so should be localized."
        inputspec {}
        outputspec {pretty_name:string}
        nargs 0
        iscachable_p "t"
    }

    GetAssignees {
        operation_desc "Get the assignees as a Tcl list of party_ids, of the default assignees for this case, object, role"
        inputspec {case_id:integer,object_id:integer,role_id:integer}
        outputspec {party_ids:[integer]}
        nargs 3
        iscachable_p "f"
    }
}

The value to the -operations switch is an array list of names of operations, and their definitions. The definition is again an array list of parameter names and values. acs_sc::contract::new would need to parse this list and create the contract, msg_types, and operations.

One particularly annoying thing about this style is that if your definition isn't entirely static text, but contains code in certain places, then you're going to have to use [list] instead of {}, and your pretty Emacs indentation goes awry. Or you're going to have to build the thing up dynamically using lappend. Regardless of the approach, this breaks the abstraction.

STYLE TWO: PROCEDURES IN PROCEDURES

The other approach I thought of was to use Tcl procedures, sort-of what the form builder already does, but with a little cleverness to aid notation. Defining a service contract would look like this:

acs_sc::contract::new \
        -contract_name [workflow::service_contract::role_default_assignee] \
        -contract_desc "Service contract to get the default assignees for a role from parameters case_id, object_id and role_id" \
        -operations {

    operation \
            -operation_name GetObjectType \
            -operation_desc "Get the object type for which this operation is valid." \
            -inputspec {} \
            -outputspec {object_type:string} \
            -nargs 0 \
            -iscachable_p "t"

    operation \
            -operation_name GetPrettyName \
            -operation_desc "Get the pretty name. May contain #...#, so should be localized." \
            -inputspec {} \
            -outputspec {pretty_name:string} \
            -nargs 0 \
            -iscachable_p "t"

    operation \
            -operation_name GetAssignees \
            -operation_desc "Get the assignees as a Tcl list of party_ids, of the default assignees for this case, object, role" \
            -inputspec {case_id:integer,object_id:integer,role_id:integer} \
            -outputspec {party_ids:[integer]} \
            -nargs 3 \
            -iscachable_p "f"
}

Here, the trick is that the value to -operations is actually a code chunk, which gets executed in a special namespace, which makes it possible to use if, else, switch, as well as $- and []-substitution.

The "operation" call used in the block is simply a procedure which is defined in a special namespace. You don't have to tell that procedure which contract you're defining, because it already knows, thanks to a variable set in its namespace, which acs_sc::contract::new sets up for it.

The service contract API to implement what's shown here would look like this:

ad_proc -public acs_sc::contract::new {
    {-contract_name:required}
    {-contract_dec {}}
    {-oprations}
} {
    insert -contract_name $contract_name -contract_desc $contract_desc

    if { [exists_and_not_null operations] } {

        namespace eval ::acs_sc::contract::define variable contract_name $contract_name

        namespace eval ::acs_sc::contract::define $operations
    }
}

ad_proc -public acs_sc::contract::define::operation {
    {-operation_name:required}
    {-operation_desc {}}
    {-inputspec {}}
    {-outputspec {}}
    {-nargs 0}
    {-iscachable_p "f"}
} {
    variable contract_name

    set inputtype "${contract_name}.${operatoin_name}.InputType"

    acs_sc::msg_type::insert \
            -msg_type_name $inputtype \
            -msg_type_spec $inputspec

    set outputtype "${contract_name}.${operation_name}.OutputType"

    acs_sc::msg_type::insert \
            -msg_type_name $outputtype \
            -msg_type_spec $outputspec

    acs_sc::operation::insert \
            -contract_name $contract_name \
            -operation_desc $operation_desc \
            -inputtype $inputtype \
            -outputtype $outputtype \
            -nargs $nargs \
            -iscachable_p $iscachable_p
}

If you want, the API doesn't have to create everything in the database right away. In situations where that's appropriate, it could just as well build up a data structure in memory in its own namespace, and then leave it to acs_sc::contract::new to actually do the inserts when everything's done.

Let me know what you think about these styles. Which do you prefer? Other ideas? Modifications to those styles?

Is this something we should make a recommended practice for?

/Lars

Collapse
Posted by Jeff Davis on
I like the first form since it does not include an almost infinite number of backslashes. I also don't really think there will be that many cases where the operation list will be built dynamically and hence need interpolation etc.) The first form is much easier to read.

I think a few modifications would be desirable. Lose nargs (computers are pretty good at counting :) ). Also the arg names are less clear than they could be.


acs_sc::contract::new 
    -name [workflow::service_contract::role_default_assignee] \
    -description "Service contract to get the default assignees for a role from parameters case_id, object_id and role_id" 
    -operations {
        GetAssignees {
            description "Get the assignees as a Tcl list of party_ids, of the default assignees for this case, object, role"
            inputs {case_id:integer,object_id:integer,role_id:integer}
            outputs {party_ids:[integer]}
            iscachable_p "f"
        }

        ...
    }
Collapse
Posted by Jun Yamog on
2nd one looks good for me.  Although take that as not so intelligent vote :)  Whichever is agreed hopefully it will be the standard on the toolkit if applicable.
Collapse
Posted by Tilmann Singer on
A few unordered remarks:

Personally I don't mind lots of backslashes after I got used to them in the formbuilder (I remember Jerry even mentioning an emacs trick to get them automatically but I never found out exactly), but I don't have anything against the second form either, except for the fact that you can't insert tcl comments. I wonder if it would be possible to get Dan's hack into this, but it's definitely a minor issue.

How would an implementation of a service contract look like? What about an extension to ad_form, so that I can simply write it like this:

ad_proc -public -implements workflow::sc::role_default_assignee::GetPrettyName my_pretty_name_proc {
    ...
    return $result
}

Furthermore I think making at least -inputspec the same format as the args list of ad_proc would make it clearer:

   GetAssignees {
        operation_desc "Get the assignees as a Tcl list of party_ids, of the default assignees for this case, object, role"
        inputspec {case_id:integer
                   object_id:integer
                   role_id:integer}
        outputspec {party_ids:integer,multiple}
        nargs 3
        iscachable_p "f"
    }

No reason not to do the same for outputspec actually.

It's great that you are picking this up, thanks! This will make service contracts much more feasible to understand and write.

Collapse
Posted by Don Baccus on
I like the first style, myself, and in fact it's similar in concept to some ideas I'd sketched out months ago when Neophytos and I started talking about getting rid of the need to laboriously write Oracle and PG DML statements to make use of service contracts.  It seems more in the style of today's ad_page_contract.

As Jon says, I don't think there will be much need to build service contract definitions dynamically.  Even if it is a pain it's still going to be easier than it would be in today's SQL-based approach.  And AFAIK no one's ever attempted to generate SC defs dynamically using the current implementation!

I think Tilmann's idea of extending ad_proc (not ad_form!) with an "-implements" switch is an interesting one ...

Collapse
Posted by Dave Bauer on
Don,

I am working on a dynamic sc implementation for ETP. When a new ETP application type (content_type) is introduced it will create a new search implementation for that type.

I am going to use the same Tcl procs for each type implementation and then write callbacks into ETP. So with the way search is currently implemented, you do need a seperate implementation for each content type.

Besides that I prefer the first approach.

Collapse
Posted by Jeff Davis on
A great thing with a -implements flag to ad_proc is that we could check that the argument signatures match when bound and could also add pre and post conditions efficiently. The complicating factor would be we would have to make some decision on how to deal with forward references.

In general though I like it and think it has value both in terms of making the code self documenting and providing hooks for doing more sophisticated things with the contracts.

Collapse
Posted by Lars Pind on
Thanks for the feedback. We'll start to do something about this today.

I take it that the community's voted in favor of style #1, which is consequently the style that we also adopted for the new workflow package. That's decided, then.

And I'll take Jeff's and Tillman's suggestions to heart and tigthen up the args a bit, including making the input/output specs use ad_page_contract style. The reason I didn't originally was that I wanted to stay 100% close to the current PL/SQL API, but I suppose that's not really a valid concern. We're trying to simplify and improve things, anyway.

Regarding making ad_proc also be able to implement, an alternative to throwing more work at ad_proc would be to have a separate acs_sc::impl::proc procedure for defining implementation procs, which, in turn, does call ad_proc to actually define the proc.

How does that sound?

/Lars