[OPNFLWPLUG-315] OutputActionBuilder.setOutputNodeConnector(new Uri("1")); needs to be replaced with ... setOutputNodeConnector(new NodeConnectorId(...)) Created: 13/Nov/14  Updated: 27/Sep/21  Resolved: 27/May/15

Status: Resolved
Project: OpenFlowPlugin
Component/s: General
Affects Version/s: None
Fix Version/s: None

Type: Bug
Reporter: Flavio Fernandes Assignee: Flavio Fernandes
Resolution: Won't Do Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All

External issue ID: 2382

 Description   

In a number of places of the controller code, I see code that looks like this:

1 private void prepareActionOutput(OutputActionCaseBuilder wrapper)

{ 2 OutputActionBuilder outputActionBuilder = new OutputActionBuilder(); 3 outputActionBuilder.setOutputNodeConnector(new Uri("1")); <==== ?!? 4 wrapper.setOutputAction(outputActionBuilder.build()); 5 }

Line 3 is an issue because while it is legal, it is not a NodeConectorId type.

As mentioned by Martin in core review:

https://git.opendaylight.org/gerrit/#/c/12568/
Martin Bobak Nov 12 2:44 AM
Patch Set 8: -Code-Review
I do believe, that patchset 8 as well as patchset 1 is not responsible for
uri becoming just ‘CONTROLLER’. W/out the 'OF:dpid:’. Method
toADNodeConnectorId always returns only port number part of uri - as a short
number or whatever string it was.

there is quite a bit of confusion on the proper use of
"OutputActionBuilder.setOutputNodeConnector()" because it takes Uri and that
has the potential of run time bugs where the Type:DPID:Port notation is not
provided. A better idea would be to make
OutputActionBuilder.setOutputNodeConnector() require NodeConnectorId as the parameter.

 Comments   
Comment by Timotej Kubas [ 27/Feb/15 ]

Hi Flavio,
we made changes in openflowplugin to make use of setOutputNodeConnector method in class OutputActionBuilder more clear. You can find these changes here:
https://git.opendaylight.org/gerrit/#/c/15824/

For now we don't consider changing of parameters in mentioned method for several reasons:
1. it would require changes in model, that would affect plenty of modules including integretion tests
2. type NodeConnectorId extends Uri and doesn't encapsulate any other logic

Is there any good argument for encapsulation of NodeConnectorId parameter?

Comment by Flavio Fernandes [ 27/Feb/15 ]

(In reply to Timotej Kubas from comment #1)
> Hi Flavio,
> we made changes in openflowplugin to make use of setOutputNodeConnector
> method in class OutputActionBuilder more clear. You can find these changes
> here:
> https://git.opendaylight.org/gerrit/#/c/15824/
>
> For now we don't consider changing of parameters in mentioned method for
> several reasons:
> 1. it would require changes in model, that would affect plenty of modules
> including integretion tests
> 2. type NodeConnectorId extends Uri and doesn't encapsulate any other logic
>
> Is there any good argument for encapsulation of NodeConnectorId parameter?

Thanks Timotej. The only argument for encapsulation is to help callers
using the expected value. Bc using the wrong value causes a very chatty
log (See bugs 2021 and 2757); the caller is never made aware that it used
the expected format.

Generated at Wed Feb 07 20:32:08 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.