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

Reply via email to