Hi reviewers! There were a number of concerns with how the domain objects were implemented in the old neutron and some different concerns with the refactored code. Problems with the old code: Options and Mapbinders (extra code, doesn't utilize GSON enough). Newer code: Create/Update/List objects were confusing.
Latest code: I added some changes to the Router, RouterApi, and RouterApiMockTest RouterApi https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApi.java Router https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/domain/Router.java Tests https://github.com/rackerlabs/jclouds-labs-openstack/blob/neutron-refactoring/openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2/features/RouterApiMockTest.java Main points improved in the Router refactoring: You use createOptions or updateOptions; you can only pass createOptions.build() to the create method; you can only pass updateOptions.build to the update method. You get Router when listing, so you cannot pass it back to the service. You have to use createOptions or updateOptions to instantiate an object as a user. These have different validation paths. They can require different parameters to be present (in this example createOptions requires name and adminStateUp). This also ensures code is relatively dry (attributes of the domain object are not repeated and neither is code). This should also keep the domain objects immutable as before. This seems to be a good pattern going forward for openstack, or at least neutron domain objects. Feedback will be much appreciated. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/111#issuecomment-47837869
