Bug 427907

Created attachment 291488 [details]
new WebService module 

as discussed with upstream ,, it is preferable that we have a WebService module
called Component.pm to hold all the component xmlrpc functions. one thing they
use is components and products ids as function arguments, in our version of
xmlrpc we use names I believe it is more user friendly, but using id is safer
as product and components names might change , just as how we encountered
before some issue when changed the name of product SV3 to EIT. I have written
an initial function that takes a list of component ids and returns the
component information. one problem in the attached patch is the following line:


initial_cc => [map $_->{login_name},  @{$_->initial_cc}],

the usage of $_ will create errors as each $_ refers to different value. I
wasn't sure how to solve this issue .. Any suggestions?

also we can easily change this function to receive list of product names and
component names instead of ids. it will work the same as the component object
can be instantiated either using component id or hash with product name and
component name.

> one problem in the attached patch is the following line:
> 
> 
> initial_cc => [map $_->{login_name},  @{$_->initial_cc}],
> 
> the usage of $_ will create errors as each $_ refers to different value. I
> wasn't sure how to solve this issue .. Any suggestions?

That should work as is. Did you test it? I think perl does the right thing. 
Read `perldoc -f map`

       map BLOCK LIST
       map EXPR,LIST
               Evaluates the BLOCK or EXPR for each element of LIST 
              (locally setting $_ to each element) ...

Created attachment 291508 [details]
perl test script of the map function and $_

Test script for the map function to prove that it will local'ise $_ for each
map.

Hi Kevin,,

I think the test you attached will work fine and also 
initial_cc => [map $_->{login_name},  @{$_->initial_cc}], 

on its own will work fine but ,, the problem i found with my patch was that
every time i call the function it will produce a different output ,, I think
that the reason that everytime it runs the order of the hash key will be
different and the values of $_ in the above line will get confused as the following:

       map {{
               id          => type('int')->value($_->id),
               name        => type('string')->value($_->name),
               description => type('string')->value($_->description),
               product_id  => type('int')->value($_->product_id),
               product_name     => type('string')->value($_->product->name),
               default_assignee   =>
type('string')->value($_->default_assignee->{login_name}),
               default_qa_contact =>
type('string')->value($_->default_qa_contact->{login_name}),
               initial_cc => [map $_->{login_name},  @{$_->initial_cc}],

             }
        } @components_objs;


so maybe key default_assignee got processed after the key initial_cc then the
value of  $_->default_assignee->{login_name} will not be from @components_objs.

what do you think?

Thanks,
Noura

hmmm ,, Kevin I retested it today and it looks like it is working fine, there
must've been something wrong before .. It looks good now. 

Created attachment 291677 [details]
New bugzilla module with functions to get component info

The attached patch now has 2 xmlrpc functions :

1-Component.get => gets the components info by a list of its ids
2-Component.get_by_name => gets single component info by its component and
product name. 

Noura

1) nice looking code

2) get_by_name assumes that the user always supplies a valid product & 
component combination. error handling?

3) Any reason for limiting get_by_name to only return on product/component 
combination? why not an array or product/components? If you go that route then 
you could reuse the code from 'get'.

4) Lastly, if WebService::Bug has get_bugs what should WebService::Component 
have? ;)

Kev

(In reply to comment #7)
> 1) nice looking code
> 
> 2) get_by_name assumes that the user always supplies a valid product & 
> component combination. error handling?
> 
> 3) Any reason for limiting get_by_name to only return on product/component 
> combination? why not an array or product/components? If you go that route then 
> you could reuse the code from 'get'.
Yeah I thought about that , I will change it to accept a list of hashes instead
of single hash.

> 
> 4) Lastly, if WebService::Bug has get_bugs what should WebService::Component 
> have? ;)
I think that mkanat said that they would want to change get_bugs in Bug.pm to
get and he was saying that we would create in Component.pm also a function
called get(), I refer to that from: 

https://bugzilla.mozilla.org/show_bug.cgi?id=385282#c7

Noura

> 
> Kev

I have always thought it should eventually be a simple API like Bug.get()
Bug.create() and Bug.update(). Same for Component, etc.

LOC estimation:

Inputs: hasref with ids/(component-product names) = 1
Outputs: input validation errors, logging messages,
         hashref with components info = 3
Inquiries: component data , user data, product data = 3
Logical files: noneero
External files = Zero

FP total count(simple weighting factor)= (3*1)+(4*3)+(3*3)+(7*0)+(5*0)= 25

FP = 25 * 1.11 = 28

LOC = 28 * 60 = 1680


For Unit testing:

100 LOC for one xmlrpc testcase

total LOC = 1680+100 - 1780 LOC

added code to force returning only components info for products that the user
can access. also added POD section and did few little modifications that were
suggested by upstream. submitted new patch version to the upstream for review.

made some modifications to the function code Component.get(), moved the code to
return accessible unique component objetcs from passed component ids or
component/product names to a separate subroutine to make it usable for other
functions.

This feature is included now in Milestone 2. Any bugs found with this feature
should be file as a new report and set to block the 3.2 final release tracker.