Ori Liel has posted comments on this change. Change subject: restapi: Return Additional Info For 400 Messages (#867794) ......................................................................
Patch Set 1: (9 comments) .................................................... File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd Line 117: </xs:sequence> Line 118: </xs:complexType> Line 119: Line 120: Line 121: <xs:element name="usage_message" type="UsageMessage"/> Done Line 122: Line 123: <xs:complexType name="UsageMessage"> Line 124: <xs:sequence> Line 125: <xs:element name="message" type="xs:string"/> Line 125: <xs:element name="message" type="xs:string"/> Line 126: <xs:element ref="detailedLink" minOccurs="0" maxOccurs="1"/> Line 127: </xs:sequence> Line 128: </xs:complexType> Line 129: Done Line 130: <!-- Actions --> Line 131: Line 132: <xs:complexType name="GracePeriod"> Line 133: <xs:sequence> .................................................... File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/validation/UsageFinder.java Line 31: if (isMatch(link, uriInfo, httpMethod)) { Line 32: return link; Line 33: } Line 34: } Line 35: return null; // should never happen If we really want to be 'by the book' the thing to do is to add an assert statement. I'll do that. Line 36: } Line 37: Line 38: private RSDL getRSDL(Application application) { Line 39: for (Object obj : application.getSingletons()) { Line 34: } Line 35: return null; // should never happen Line 36: } Line 37: Line 38: private RSDL getRSDL(Application application) { The only thing is, I'm trying to think how client code would access this object. In our current architecture' we initialize instances of all of our 'Resources' and place them in a list within the Application object (or rather, BackendApplication, which extends it). We would need to add this object to there, but it's different than the other objects in that list, in the sense that it isn't identified with a certain path, and it's not an interceptor. See the constructor of BackendApplication() to understand what I'm talking about. Still, this might be doable. I think I'll play with it and check if it can work. Line 39: for (Object obj : application.getSingletons()) { Line 40: if (obj instanceof BackendApiResource) { Line 41: BackendApiResource resource = (BackendApiResource) obj; Line 42: return resource.getRSDL(); Line 41: BackendApiResource resource = (BackendApiResource) obj; Line 42: return resource.getRSDL(); Line 43: } Line 44: } Line 45: return null; // should never happen Done Line 46: } Line 47: Line 48: private boolean isMatch(DetailedLink link, UriInfo uriInfo, String httpMethod) { Line 49: int baseUriLength = uriInfo.getBaseUri().getPath().length(); .................................................... File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/rsdl/RsdlBuilder.java Line 90: Line 91: private static final String RESOURCES_PACKAGE = "org.ovirt.engine.api.resource"; Line 92: private static final String PARAMS_METADATA = "rsdl_metadata.yaml"; Line 93: Line 94: public RsdlBuilder(UriInfo uriInfo, ApplicationMode applicationMode) { I agree it's nicer to supply 'rels', I'll do that. I don't think it has to do with a new class that manages the RSDL, I think BackendApiResouce will simply initialize the RsdlBuilder with 'rels'. Line 95: this.uriInfo = uriInfo; Line 96: this.applicationMode = applicationMode; Line 97: this.parametersMetaData = loadParametersMetaData(); Line 98: } Line 246: List<DetailedLink> results = new ArrayList<DetailedLink>(); Line 247: List<Class<?>> classes = ReflectionHelper.getClasses(RESOURCES_PACKAGE); Line 248: List<String> paths = Line 249: applicationMode == ApplicationMode.GlusterOnly ? ApiUtils.getGlusterRels(uriInfo) Line 250: : ApiUtils.getAllRels(uriInfo); Done Line 251: for (String path : paths) { Line 252: Class<?> resource = findResource(path, classes); Line 253: String entryPointPath = uriInfo.getBaseUri().getPath(); Line 254: String prefix = entryPointPath.endsWith("/") ? entryPointPath + path : entryPointPath + "/" + path; Line 249: applicationMode == ApplicationMode.GlusterOnly ? ApiUtils.getGlusterRels(uriInfo) Line 250: : ApiUtils.getAllRels(uriInfo); Line 251: for (String path : paths) { Line 252: Class<?> resource = findResource(path, classes); Line 253: String entryPointPath = uriInfo.getBaseUri().getPath(); Agreed, will do (although if I'm not mistaken, compiler optimization implicitly does it anyway in such a case) Line 254: String prefix = entryPointPath.endsWith("/") ? entryPointPath + path : entryPointPath + "/" + path; Line 255: results.addAll(describe(resource, prefix, new HashMap<String, Type>())); Line 256: } Line 257: return results; .................................................... File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/ApiUtils.java Line 13: import org.ovirt.engine.api.model.Link; Line 14: import org.ovirt.engine.api.model.Parameter; Line 15: import org.ovirt.engine.api.model.ParametersSet; Line 16: Line 17: public class ApiUtils { I agree ApiUtils is a terrible name, but I'm not sure about the name you proposed, because not only RSDL uses this, but also BackendApiManager - to display links for: GET .../api. I'll think of a better name and resubmit. Line 18: Line 19: public static Collection<DetailedLink> getLinks(UriInfo uriInfo) { Line 20: Collection<DetailedLink> links = new LinkedList<DetailedLink>(); Line 21: links.add(createLink("capabilities", uriInfo)); -- To view, visit http://gerrit.ovirt.org/23017 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic7a6da4fbb00ea5f1e6919ddfe21dd7b3dd126fd Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches