luckymarmot / Paw-SwaggerImporter

Swagger/OpenAPI 2.0 Importer for Paw
https://paw.cloud/extensions/SwaggerImporter
MIT License
38 stars 10 forks source link

Feature Request: Group by Path #12

Closed letsbyteit closed 8 years ago

letsbyteit commented 8 years ago

Is there any way to get a proper grouping of the requests by their path?

Example imported with Paw-SwaggerImporter: screen shot 2016-03-14 at 05 10 50 (Taken from: http://petstore.swagger.io/v2/swagger.json)

I'd like to see groups per path-component. I was wondering, that Paw doesn't do it itself. Maybe I'm doing something wrong?

I'd expect now three main-groups:

And the whole thing recursive, so that for example

would be grouped under the main-group

nosyjoe commented 8 years ago

I had a patch for this lying on my disk and created a pull request (#13) from it. You might want to check it out if it works for you!

letsbyteit commented 8 years ago

@nosyjoe Just a short question: Your implementation just groups the first path-segment, right? I'm struggling with this since hours… The problem is the tree-building. I found some working solutions, but these require previous sorting and filtering. For PHP I found something actually working (except root-nodes must be omitted before): http://thehighcastle.com/blog/38/php-dynamic-access-array-elements-arbitrary-nesting-depth

Just an example I found: screen shot 2016-03-15 at 22 04 51

Without omitting root-nodes: screen shot 2016-03-15 at 22 05 02

Maybe you have some ideas?

nosyjoe commented 8 years ago

oh you want sub-groups? it's only grouping top-level path segments, yes

letsbyteit commented 8 years ago

Ah okay, I'm looking into it right now. Yeah, I thought it would be very useful to have a recursive attempt on this or so. But it seems a bit complicated to build up the tree. It would be super easy with such array-access:

var string = "[1][2][3][4]";
array{string} = "42";

But I think, grouping the first segment is a huge improvement for now. We've got > 200 endpoints and it's kind of messy with Paw.

nosyjoe commented 8 years ago

recursion to the rescue, I agree. the API makes it a bit complicated since subgroups are handled differently from root-level groups, though. I'm playing around with it a little

letsbyteit commented 8 years ago

Thanks, I didn't really look into the Paw-Grouping before. My problem was more the grouping-algorithm. Maybe CoffeeScript handles this better than vanilla JavaScript, I'm not pro with CS/JS at all.

This is my current JavaScript attempt:

build_path_tree = function (paths, path_separator) {
    path_separator = typeof path_separator !== 'undefined' ? path_separator : "/";

    var separated_paths = split_paths(paths, path_separator);

    return group_by_paths(separated_paths, path_separator);
}

group_by_paths = function (paths, path_separator) {

    var result_array = [];

    for (var current_path in paths) {
        if (paths.hasOwnProperty(current_path)) {
            array_set_path(result_array, paths[current_path], current_path);
        }
    }

    return result_array;
}

split_paths = function (paths, path_separator) {

    var result_array = [];

    for (index = 0; index < paths.length; ++index) {
        result_array[paths[index]] = split_path(paths[index], path_separator); 
    }

    return result_array;
}

split_path = function (path, path_separator) {
    var path_components = path.split(path_separator);   
    return path_components.filter(function(n){ return n != undefined && n != "" }); 
}

array_set_path = function (object, path, value) {
    var o = object;
    for (var i = 0; i < path.length - 1; i++) {
        var n = path[i];
        if (n in o) {
            o = o[n];
        } else {
            o[n] = {};
            o = o[n];
        }
    }
    o[path[path.length - 1]] = value;
}

console.log(build_path_tree(["/test/12/", "/test/13/", "/test/13/14/", "/app/users/", "/app/users/test", "/app/files/"]));

This is my PHP attempt (I'm just more used to prototype with PHP):

<?php
    $paths = ["/app/test", "/app/files/123", "/app/users/123",  "/app/users/", "/web/"];

    var_dump(build_path_tree($paths));

    function build_path_tree($paths, $path_separator = "/")
    {
        $separated_paths = split_paths($paths, $path_separator);

        return group_by_paths($separated_paths, $path_separator);
    }

    function group_by_paths($paths, $path_separator = "/")
    {
        $result_array = [];

        foreach($paths as $current_path => $current_path_components)
        {
            array_set_path($current_path, $result_array, $current_path_components);
        }   

        return $result_array;
    }

    function split_paths($paths, $path_separator = "/")
    {
        $result_array = [];

        foreach ($paths as $current_path)
        {
            $result_array[$current_path] = split_path($current_path, $path_separator);
        }

        return $result_array;
    }

    function split_path($path, $path_separator = "/")
    {
        return array_filter(explode($path_separator, $path));
    }

    function &array_get_path( &$array, $path, $delim = NULL, $value = NULL, $unset = false ) {

          $num_args = func_num_args();

          $element = &$array;

          if ( ! is_array( $path ) && strlen( $delim = (string) $delim ) ) {

            $path = explode( $delim, $path );

          }
          // if

          if ( ! is_array( $path ) ) {

            // Exception?

          }
          // if

          while ( $path && ( $key = array_shift( $path ) ) ) {

            if ( ! $path && $num_args >= 5 && $unset ) {

              unset( $element[ $key ] );

              unset( $element );

              $element = NULL;

            }
            // if

            else {
                //echo "Element: " . $element . " Key: " . $key . "\n\n";
              $element =& $element[ $key ];

            }
            // else

          }
          // while

          if ( $num_args >= 4 && ! $unset ) {

            $element = $value;

          }
          // if

          return $element;

        }
        // array_get_path

        function array_set_path( $value, &$array, $path, $delimiter = NULL ) {

          array_get_path( $array, $path, $delimiter, $value );

          return;

        }
        // array_set_path

        function array_unset_path( &$array, $path, $delimiter = NULL ) {

          array_get_path( $array, $path, $delimiter, NULL, true );

          return;

        }
        // array_unset_path

        function array_has_path( $array, $path, $delimiter = NULL ) {

          $has = false;

          if ( ! is_array( $path ) ) {

            $path = explode( $delimiter, $path );

          }
          // if

          foreach ( $path as $key ) {

            if ( $has = array_key_exists( $key, $array ) ) {

              $array = $array[ $key ];

            }
            // if

          }
          // foreach

          return $has;

        }
        // array_has_path;

?>

Part of the PHP code was taken from this URL: http://thehighcastle.com/blog/38/php-dynamic-access-array-elements-arbitrary-nesting-depth

nosyjoe commented 8 years ago

you can write it way easier. e.g. split the path segments and omit empty parts:

pathSegments = swaggerRequestPathName.split('/').filter (s) -> s && s != ""
console.log(pathSegments)
nosyjoe commented 8 years ago

ok, after diving into it a little bit more, I managed to come up with a solution, it is not perfect but it does do the creating of the recursive groups:

https://github.com/nosyjoe/Paw-SwaggerImporter/tree/recursive-request-groups

hope that helps!

letsbyteit commented 8 years ago

@nosyjoe Wow, thank you! That's definitely a huge improvement! I'm forking the Swagger-Importer for a specific Swagger-using framework, to make things more easy with the framework. For me, I think, I will add a breakpoint for path-segments containing placeholders like /{user_id}/. I'm currently unsure whether the placeholder syntax is specified by Swagger. But if so, it should also be considered for the Default-Swagger-Importer to stop there. Or would you recommend (a more complex way, I think) to just group these placeholder-segments just, if there are other such segments? I mean:

https://api.my-product.com/my/path/{user_id}/{my_value_1}/
https://api.my-product.com/my/path/{user_id}/{my_value_2}/
https://api.my-product.com/my/path/{file_id}/

=>
- my (group)
  - path (group)
    - {user_id} (group)
      - {my_value_1} (NO group)
      - {my_value_2} (NO group) 
  - {file_id} (NO group) 

This is an example with many empty parsed paths screen shot 2016-03-16 at 00 07 10

So, for us it's currently as good as it could get. But for the future, someone might think about better breakpoint-handling for the sub-groups.

Thanks for your help!

UPDATE: It looks like the sub-grouping creates sub-groups in wrong main-groups. I'm going to provide an example tomorrow, I think. Maybe something like a substring-match before adding to a sub-group would be needed. So, take the current parent's complete path and check whether the current child's path is a complete substring or so. Looking into it tomorrow…

mittsh commented 8 years ago

Nice work on this! Thank you for that. I'll have a look at this code quickly. Thought, I don't know if we should merge this to master yet: some users may not like the per-path-component grouping, and an update to Paw is coming soon that will bring options to importers. We will be able to set a checkbox and ask if the user wants to group per path component.

If you guys still have some questions on how this can be implemented, and about the use of the API, I'm happy to have a Skype call tomorrow (we're in Western Europe).

nosyjoe commented 8 years ago

@letsbyteit glad I could help. I think the placeholders in curly braces are swagger-standard. I was thinking about excluding them like in /pet/{petId} but then there were cases with nested placeholders (like /pet/{petId}/uploadImage) and things were starting to get too complicated :)

also, as a next step I would like to map these placeholders to environment variables through dynamic values...

nosyjoe commented 8 years ago

@mittsh yeah, offering the user an option to choose sounds great. although I don't think groups with 1 child each, like it is currently implemented, makes sense as a default. but choosing how many level or if there should be recursive groups would be a good configuration option, I agree!

mittsh commented 8 years ago

@nosyjoe yeah :) when we have options for importers it will all be much nicer!

nosyjoe commented 8 years ago

@mittsh any ETA on the 2.3.3 release? do you guys have a alpha/beta program?

JonathanMontane commented 8 years ago

@letsbyteit This behavior is now supported in Paw.