Michael Pasternak has posted comments on this change.
Change subject: core: osinfo - Remove Unassigned and map it to Other
......................................................................
Patch Set 4: (3 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/OsValidator.java
Line 15: @Override
Line 16: public void validateEnums(OperatingSystem os) {
Line 17: if (os != null) {
Line 18: if (os.isSetType()) {
Line 19: validateEnum(OsType.class, OsTypeUtils.getAllValues(),
os.getType(), false);
well, it works for you case you never reach [1] at EnumValidator, because you
pass OsTypeUtils.getAllValues() so you get out from the validator on string
existence in List verification, but if someone else will reuse your code, and
string to check is not in a List of string, he will end up with the
IllegalArgumentException
cause all enums in api are lower case and therefore Enum.valueOf("xxx") will
always fail.
[1] return Enum.valueOf(clz, toUppercase ? name.toUpperCase() : name).name();
Line 20: }
Line 21: if (os.isSetBoot()) {
Line 22: for (Boot boot : os.getBoot()) {
Line 23: bootValidator.validateEnums(boot);
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java
Line 258: entity.getInitrdUrl() != null ||
Line 259: entity.getKernelParams() != null) {
Line 260: OperatingSystem os = new OperatingSystem();
Line 261:
Line 262:
os.setType(SimpleDependecyInjector.getInstance().get(OsRepository.class).getUniqueOsNames().get(entity.getOsId()));
same comment as in VmMapper for corresponding mapping
Line 263:
Line 264: if (entity.getDefaultBootSequence() != null) {
Line 265: for (Boot boot :
VmMapper.map(entity.getDefaultBootSequence(), null)) {
Line 266: os.getBoot().add(boot);
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java
Line 307: entity.getKernelParams() != null) {
Line 308: OperatingSystem os = new OperatingSystem();
Line 309:
Line 310:
os.setType(SimpleDependecyInjector.getInstance().get(OsRepository.class).getUniqueOsNames().get(entity.getVmOsId()));
Line 311:
even 'other' ?....
Line 312: os.setKernel(entity.getKernelUrl());
Line 313: os.setInitrd(entity.getInitrdUrl());
Line 314: os.setCmdline(entity.getKernelParams());
Line 315: model.setOs(os);
--
To view, visit http://gerrit.ovirt.org/16679
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie508a8828caedf4cb8fdf971e14139404f7dded3
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
