Skip to content

initial work to support HttpEntity#531

Merged
asfgit merged 7 commits intoapache:masterfrom
andreaturli:feature/http-command-effector
Feb 15, 2017
Merged

initial work to support HttpEntity#531
asfgit merged 7 commits intoapache:masterfrom
andreaturli:feature/http-command-effector

Conversation

@andreaturli
Copy link
Copy Markdown
Contributor

  • add HttpCommnadEffector
  • add CompositeEffector
  • add EntityInitializers util class to resolve DSL injected as params
    into the HttpCommandEffector

It is based on #152 which was not merged as it didn't handle rebind properly, happy to sort it out in this PR but I've not touched rebinding yet.

public void testHttpEffector() throws Exception {
new HttpCommandEffector(ConfigBag.newInstance()
.configure(HttpCommandEffector.EFFECTOR_NAME, "GithubApacheAccount")
.configure(HttpCommandEffector.EFFECTOR_URI, "https://api.github.com/users/apache")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend http://httpbin.org/ for integration testing - it gives very predictable responses based on the inputs, allowing you to test a wide variety of things such as request/response headers, get/put/post/delete etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @aledsage, much better, I'll use it

private EntityLocal entity;

@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can write a unit test (rather than just an integration test) if you use something similar to https://github.com/apache/brooklyn-server/blob/master/core/src/test/java/org/apache/brooklyn/feed/http/HttpFeedTest.java

@aledsage
Copy link
Copy Markdown
Contributor

@aledsage
Copy link
Copy Markdown
Contributor

And rebind tests would be very good (e.g. see https://github.com/apache/brooklyn-server/blob/master/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java and its sub-classes - the important thing is to call rebind() in the test method after creating your entities, and then doing some basic testing that the entity is still functional).


private static final Logger LOG = LoggerFactory.getLogger(CompositeEffector.class);

public static final ConfigKey<List<String>> EFFECTORS = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "effectors",
Copy link
Copy Markdown
Contributor

@aledsage aledsage Jan 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we pass in the same parameters to every effector. I wonder whether we should also accept a list of maps, so that we could define the parameters to the individual effectors. But that would make the code more complicated.

It would be interesting to see what the user's yaml would look like, if they were doing that.

@andreaturli
Copy link
Copy Markdown
Contributor Author

I've added more tests for the effectors:

  • add unit tests: HttpCommandEffectorTest and CompositeEffectorTest
  • add YamlTests: HttpCommandEffectorYamlTest and CompositeEffectorYamlTest.java
  • add YamlRebindTests: HttpCommandEffectorYamlRebindTest and CompositeEffectorYamlRebindTest

please @aledsage can you have another look?

- add HttpCommnadEffector
- add CompositeEffector
- add EntityInitializers util class to resolve DSL injected as params
  into the HttpCommandEffector
- some http calls may not require to parse the output
- add unit test (HttpCommandEffectorTest)
- add YamlTest (HttpCommandEffectorYamlTest)
- add YamlRebindTest (HttpCommandEffectorYamlRebindTest)
@andreaturli andreaturli force-pushed the feature/http-command-effector branch from 50f908a to 8809d5c Compare February 13, 2017 14:16
@andreaturli andreaturli reopened this Feb 14, 2017
@andreaturli andreaturli force-pushed the feature/http-command-effector branch from 98edec7 to 1871a52 Compare February 14, 2017 11:35
@andreaturli
Copy link
Copy Markdown
Contributor Author

@aledsage, @grkvlt can you have another look at this PR, please?

@ahgittin
Copy link
Copy Markdown
Contributor

very nice @andreaturli -- and excellent tests.

is there a corresponding PR against the docs to explain how to use this?

looks good so merging

@asfgit asfgit merged commit 1871a52 into apache:master Feb 15, 2017
asfgit pushed a commit that referenced this pull request Feb 15, 2017
@andreaturli
Copy link
Copy Markdown
Contributor Author

I will open a PR for docs ASAP, thanks @ahgittin for merging it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants