Commit Graph

47 Commits

Author SHA1 Message Date
Gary Russell
324c54d8ef Remove use of deprecated ChannelInterceptorAdapter
Deprecated in favor of default methods on `ChannelInterceptor`.

* Fix race in RouterConcurrencyTest; no assertion on executor shutdown; increase time.
2018-05-24 15:11:11 -04:00
Artem Bilan
027021d6e4 Move XSDs to version 5.1
* Upgrade to Spring AMQP-2.1 and fix compatibility
2018-03-17 09:58:40 -04:00
Gary Russell
82f252dd00 INT-4390: Fix tangles
JIRA: https://jira.spring.io/browse/INT-4390

Phase I: flow context/registration cycle

- extract interfaces
- rename implementations to `Standard...`

Phase II - dsl<->dsl.channel tangles

Move the channel specs to dsl.

Fix missing refactorings

(Not related to DSL) - rename core `event` package to `events`

- avoid collision with event module package
- fix tangle caused by `MessageGroupExpiredEvent`.

Phase III - dsl<->config.dsl

- move classes from config.dsl to dsl

(Not related to DSL) - fix many tangles caused by graph being a sub-package of support.management
 - make `graph` a top-level package

(Not related to DSL) - move `IntegrationManagementConfigurer` from
`management` to `config` - tangle between core and management
* Polishing Copyrights, diamonds, some JavaDocs and What's New
2018-03-16 14:59:47 -04:00
Artem Bilan
cb0d43db6b Fix JDBC tests and some upgrades
https://build.spring.io/browse/INT-FATS5IC-430

Looks like there is a race condition when our polling rate around
data base is too fast and we have access to the DB files even during
application context close.

* Stop polling channel adapter in the tests explicitly after test methods
* Increase some tests performance decreasing timeouts to wait
* Upgrade to Reactor-3.1.5, AssertJ-3.9.1, Derby-10.14.1.0 and some
Gradle plugins

* Upgrade to `reactor-netty-0.7.5`
* Remove workaround from the `StompServerIntegrationTests`

* Upgrade to Spring Data Kay SR5

* Upgrade to Spring Security 5.0.3
* Increase timeouts and performance in the `StreamTransformerParserTests`
2018-02-28 11:57:21 -05:00
Artem Bilan
e46fc9b738 INT-4407: Add merge option to Log4j2LevelAdjuster
JIRA: https://jira.spring.io/browse/INT-4407

To make a `Log4j2LevelAdjuster` more builder friendly, add a `merge`
option to the overloaded `classes()` and `categories()` methods.
This way the existing (or previously configured) `categories` and
`classes` are merged with the provided values and default
`org.springframework.integration` category can be reuse.

Note: this behavior can be reconsidered as default one in the `5.1`
2018-02-20 15:05:46 -05:00
Artem Bilan
f0a6cb7d41 Upgrade to Reactor-3.1.4, Reactor-Netty-0.7.4
* Fix `StompServerIntegrationTests` for disabling connection pool to
workaround the regression in the Reactor-Netty-0.7.4

Related to https://github.com/reactor/reactor-netty/pull/290
2018-02-20 11:01:41 -05:00
Gary Russell
6df25d3c3f More diagnostics for StompServerTests 2018-02-17 12:16:53 -05:00
Artem Bilan
aca79f842d Fix deprecation from SF-5.0.3
* Upgrade to Spring Data Key SR3, Reactor-3.1.3, Reactor Netty-0.7.3
2018-01-24 11:13:01 -05:00
Artem Bilan
30450c48be INT-4376: Upgage to Log4J 2 (#2321)
* INT-4376: Upgage to Log4J 2

JIRA: https://jira.spring.io/browse/INT-4376

* Deprecate Log4J 1.x components in favor of newly added
a `Log4j2LevelAdjuster` JUnit `@Rule`
* Update all the logging configuration to Log4J 2

* Polishing after testing
2018-01-11 10:59:55 -05:00
Artem Bilan
df109410c3 Fix AbstractStompSessionManager race condition
https://build.spring.io/browse/INT-MJATS41-1150

When we add `addHandler()` to the `CompositeStompSessionHandler`,
there is no guarantee that we will have `session` atomically during
this method invocation or after it relying on the later call to the
`this.delegates`.
In other words the session may be populated in between and, therefore,
our `delegate` loses `afterConnected()` event.
We need to synchronize on the barrier and block the concurrent
`afterConnected()` or wait for it.
This way we atomically ensure that our `delegate` is added to the
existing `session` or will be performed afterwards in the `afterConnected()`

**Cherry-pick to 4.3.x**
2017-10-23 13:45:05 -04:00
Artem Bilan
bf1fef39db INT-4345: ServerWebSocketContainer: Fix Lifecycle
JIRA: https://jira.spring.io/browse/INT-4345

Resolves https://github.com/spring-projects/spring-integration/issues/2238

The `DefaultHandshakeHandler` doesn't implement `SmartLifecycle`
(just `Lifecycle`) there it doesn't delegate its `start()` to the
`RequestUpgradeStrategy`, e.g. `JettyRequestUpgradeStrategy`.
On the other hand `DefaultHandshakeHandler` can be started as a dependant
`Lifecycle` from some other `SmartLifecycle`, like it happens with the
`WebSocketHandlerMapping`

* Implement `SmartLifecycle` for the `ServerWebSocketContainer` and
delegate its lifecycle to the provided `HandshakeHandler`
* Fix `WebSocketInboundChannelAdapter` to properly implement `doStop()`
with the propagation to the provided `webSocketContainer`
* Fix deprecation warning in the `StompMessageHandlerWebSocketIntegrationTests`

**Cherry-pick to 4.3.x**
2017-10-16 17:28:55 -04:00
Artem Bilan
a1c42f65d0 Fix timing issue in test for STOMP Server
https://build.spring.io/browse/INT-MASTER-733

The Netty tries to load `netty-transport-native-epoll` too long, more
then 10 seconds we expect in the `AbstractStompSessionManager`, therefore
 we end up having `StompConnectionFailedEvent` as a first event in the
queue channel for STOMP events.

* Increase connection wait timeout to `30 secs`, like it is in many other
places in Spring
* Provide some Java 8 style refactoring for the `AbstractStompSessionManager`
* Fix `StompServerIntegrationTests` to poll events from queue until
`StompSessionConnectedEvent` any way
* Upgrade to Gradle 4.0 and `spring-io-plugin-0.0.8`
* Bump `org.sonarqube` version to `2.5` - `2.1` isn't compatible with Gradle 4.0
2017-06-23 15:41:37 -04:00
Artem Bilan
fabc5fb725 Fix STOMP and WebSocket modules to the latest SF
* The `@MessageMapping` now requires `@SendTo` for replies
even if we are going to send to the default topic
* Fix assertion for exception message in the `StompInboundChannelAdapterWebSocketIntegrationTests`
2017-06-07 15:26:55 -04:00
Artem Bilan
b3f461e139 INT-4018: Resolve some TODOs
JIRA: https://jira.spring.io/browse/INT-4018

* Resolve some TODOs as expected
* Remove some as logically or physically with useless effort
* Leave others for future considerations
2017-03-14 12:30:47 -04:00
Gary Russell
37683f1b80 INT-4219: Test Polishing
JIRA: https://jira.spring.io/browse/INT-4219

- Use `AdditionalAnswers.returnFirstArgument` instead of lambda to return first arg
- Enable CheckStyle `AvoidStaticImport` for tests
- Fix static import violations in tests
2017-03-06 11:13:39 -05:00
Artem Bilan
2659baa821 Checkstyle ImportOrder for main and fixes 2017-01-23 12:58:36 -05:00
Artem Bilan
b25c95508e INT-4125: Fix STOMP module to the latest SF
JIRA: https://jira.spring.io/browse/INT-4125

* Rename `Reactor2TcpStompSessionManager` to `ReactorNettyTcpStompSessionManager` and make it based on the `ReactorNettyTcpStompClient` from the latest SF
* Revert dependencies to their latest B-S
2016-12-02 12:32:12 -05:00
Artem Bilan
3e8ed942a0 Ignore StompServerIntegrationTests until Reactor 3
Since IO Platform plugin provide its own dependencies configuration we can't enforce Reactor 2.0 usage.
From other side the Rector 2.0 support doesn't make sense for the next IO version.

* `@Ignore` the test class which is based on the Reactor 2.0 foundation until SF STOMP module provides Reactor 3.0 solution
2016-11-28 10:16:21 -05:00
Gary Russell
67d6cd0c89 Lambdas for Remaining Modules JPA -> ZK
Polishing - PR Comments and Closeable Warnings

Eclipse emits bogus warnings with exceptions in lambdas.
Even though the lambda might run on another thread, elipse thinks it could
cause the context to not be closed.

SPR-14854: MessageChannel is now a @FunctionalInterface

* Additional Lambda polishing and some code style fixes
2016-10-28 16:06:11 -04:00
Artem Bilan
12fb590b95 Mainly Checkstyle Violation Fixes
* Upgrade to Checkstyle `7.1`
* Relax `RequireThis` rule a bit. Right now it does the effort only in case of overlapping. See https://github.com/checkstyle/checkstyle/issues/2362 for more info
* Enable some annotation rules and provide fixes for violations
* Enable `tabs indents` rule. This was the biggest fix in this PR
* Resolve `TODO` in the `MessagingMethodInvokerHelper` and fix tests to meet `IllegalStateException` now

Remain Checkstyle version `6.16.1` and reinstate `RequireThis` rule

The latest Checkstyle has a bug with local scope variables if they have the same names as property.

Revert some literals splitting

Fix some line length exceeding and code style
2016-09-22 13:23:51 -04:00
Gary Russell
04a42f54be Rename Schemas to 5.0
Fix Schema Imports

spring.schemas
2016-08-18 14:40:38 -04:00
Artem Bilan
7124136091 Some Fixes and Improvements
* Fix several typos in log messages. And some test on the matter as well
* Add comment to `AbstractPersistentAcceptOnceFileListFilter.rollback()` to clarify the reason of `rollingBack` variable
* Make `RemoteFileTemplate.StreamHolder` as `static` to avoid extra internal variable to outer class instance
* Replace `MessagingException` with `AbstractInboundFileSynchronizingMessageSource` in `init()` method of some components. It isn't Messaging yet in that phase
* Fix `SubscribableRedisChannel.MessageListenerDelegate` to handle `Object` not `String`, because with the `serializer` injection there is no guaranty that incoming is always `String`
* Move `JSch.setLogger(new JschLogger());` in the `DefaultSftpSessionFactory` to `static` block. It really should be done only once
* Remove `Assert.isTrue(this.port >= 0)` from the `DefaultSftpSessionFactory`. The subsequant `initJschSession()` convert it to default `22` port
* Change in the `JschProxyFactoryBean` `UnsupportedOperationException` to `IllegalArgumentException`. Wrong enum is wrong argument. That isn't a problem of operation
* Simplify `stop()` in the `CuratorFrameworkFactoryBean` and mark it as a `this.running = false`. Otherwise it wasn't able to be restarted
* Expose `leaderEventPublisher` in the `LeaderInitiatorFactoryBean`  and fix `stop(Runnable callback)` with propagation `callback` to delegate.

Fix `SubscribableRedisChannelTests` for new `handleMessage(Object)` signature
2016-08-12 12:47:32 -04:00
Artem Bilan
bce851576e INT-4075: Fix NPE in AbstractStompSessionManager
JIRA: https://jira.spring.io/browse/INT-4075

Fixes GH-1854 (https://github.com/spring-projects/spring-integration/issues/1854)

The `catch` block just swallows an Exception with the logger and leaves `stompSessionListenableFuture` as `null`.
The next `stompSessionListenableFuture` usage over two code lines bellow ends up with `NPE`.

* Add reconnect logic into that `catch` block and just return from there, since we don't have anything to go ahead.
* Add `if (e != null) {` around `log.error` in the `scheduleReconnect`
* Add `if...else` around `this.stompClient.getTaskScheduler()` and `log.info()` to notify end-user that there won't be reconnection if there is no `taskScheduler`
* Add `StompSessionManagerTests` mock tests to verify solution

**Cherry-pick to 4.2.x**
2016-07-20 10:59:17 -04:00
Sean Mills
17abf29da1 INT-4044: Fix WebSocketStompSessionManager API
JIRA: https://jira.spring.io/browse/INT-4044

The wrong method `WebSocketStompClient.connect()` has been used before because of `Object` vararg.

* Fix to the proper method invocation
* Modify `StompInboundChannelAdapterWebSocketIntegrationTests` with `origin` handshake header to prove the fix
2016-06-02 19:04:41 -04:00
Gary Russell
57f96bb759 checkstyle Misc Rules
checkstyle Nesting

checkstyle GenericWhitespace

checkstyle MethodParamPad

checkstyle NoWhiteSpace

checkstyle ParenPad Script

checkstyle ParenPad
2016-04-05 17:21:29 -04:00
Gary Russell
05cc7be644 checkstyle WhiteAround
WhiteAroundCheck script
2016-04-05 13:50:11 -04:00
Artem Bilan
2b0598291c RequireThis rule and fixThis Gradle task
* `gradlew clean check -x test --parallel --continue` - to collect reports
* `gradlew fixThis --parallel` - to fix all possible vulnerabilities. With `-Dfile.encoding=UTF-8` on Windows

Since the `RequireThisCheck` doesn't see parents for anonymous classes (e.g. `Runnable` callback), its report doesn't contains the outer class name with `this.`,
therefore we still have to fix those cases manually.
Thanks to the wrong `replacer` just with `this.` we have uncompilable code enough easy to find problems.
Not so easy to fix for good readability though...

* Upgrade to Grade 2.12
* Upgrade to SonarQube native plugin

The fix contains at about 300 files. So, will be done on merge.

Fix `fixThis.gradle` according PR comments

Apply `fixThis` and also `fixModifiers` for test classes.
 Fix some `this.` inner issues manually.
 Make code polishing for long lines after `fixThis`

Fix conflicts and vulnerabilities after the rebase
2016-03-22 20:18:27 -04:00
Artem Bilan
66cd92a6e7 INT-3964: More @EnableIntegration Documentation
JIRA: https://jira.spring.io/browse/INT-3964

Doc Polish

XSD: document classes for element implementations

Fix Redis XSD typo for CDATA
2016-03-22 10:41:07 -04:00
Artem Bilan
ef40a939cf Add check header rule and fixHeaders task
Add "new line" in the header end

* Optimization for the `fixHeaders.gradle`
* Apply the `fixHeaders` for the affected classes
2016-03-07 15:08:51 -05:00
Artem Bilan
c810aaae12 INT-3897: Fix deadlock in the StompMessageHandler
JIRA: https://jira.spring.io/browse/INT-3897

The `StompMessageHandler` synchronized on the `sessionHandler` during `connectIfNecessary()`
performing mutually exclusive connect operation in the `StompSessionManager`.
Since the last one synchronizes on all `sessionHandler`s during its own operations, we end up with the deadlock
waiting for the `connectSemaphore`.

Change synchronization monitor object to the `connectSemaphore` accomplishing the same desired mutually exclusive connect logic
in the `StompMessageHandler`.

**Cherry-pick to 4.2.x**
2015-12-09 10:50:53 -05:00
Gary Russell
d16cd9748c INT-799: Remove Core Deps from s-i-test
JIRA: https://jira.spring.io/browse/INT-799

Some test classes (e.g. `TestUtils`) were duplicated in core to avoid cyclic
dependency.

Now that core messaging has been moved to spring-messaging, it is possible
to remove the dependencies on `spring-integration-core` from `spring-integration-test`.

A few minor test cases have been moved to `spring-integration-core`.

The simple polishing to the `build.gradle` and `ServiceActivatorOnMockitoMockTests`
2015-12-07 20:39:00 -05:00
Artem Bilan
1cc1b2ef79 INT-3895: Fix MonitorTests Race Condition
JIRA: https://jira.spring.io/browse/INT-3895

* `@Ignore` `DelayerUsageTests.testDelayWithCustomScheduler()` as very weak test. (We can consider it to remove at all: doesn't test anything from our side)
* Add `LogAdjustingTestSupport` diagnostic to the `OutboundGatewayFunctionTests`

Tentative commit to see more logs from Travis

Additional diagnostics

More STOMP Diagnostics
2015-11-23 15:02:10 -05:00
Artem Bilan
350355e362 Fix one more STOMP WebSocket test race condition
https://build.spring.io/browse/INT-B43-JOB1-7

We need to wait for the **real** `UNSUBSCRIBE` from server before go ahead.
2015-11-19 19:23:10 -05:00
Artem Bilan
c0d1277b55 INT-3888: Fix STOMP Test for Race Condition
JIRA: https://jira.spring.io/browse/INT-3888

Note sure yet what race condition we have in the `DefaultSubscriptionRegistry`,
but that looks for me better to follow with the standard `SubscriptionRegistry.findSubscriptions()`
to check subscription present, unless fail during `send()` because of `destinationLookup` early exit.

Fix for the `StompServerIntegrationTests`

Looks like we have to `reconnect()` for any `handleTransportError()` activity,
otherwise we are just hang in the Reactor's TCP reconnect loop without freeing resources
for other activities, e.g. start Embedded STOMP Broker.

Increase `receive` timeout in case of wait for the Broker Start
2015-11-19 08:55:13 -05:00
Gary Russell
051b393ed0 Prepare Master for 4.3
Reference to 4.3

Refactor `what's new` for 4.3
2015-11-17 12:12:49 -05:00
Artem Bilan
9d8e2b61f6 Fixes for Sporadic Test Failures
* `ClientWebSocketContainer`: add some synchronization to avoid race conditions: https://build.spring.io/browse/INT-B41-492
* `TomcatWebSocketTestServer`: convert to `0` port to rely on the OS resolution for `localPort`
* Add `LogAdjustingTestSupport` for STOMP test
* `SftpServerTests`: use `0` port to rely on the OS resolution for `localPort`
* `ImapMailReceiver`, `OutboundGatewayFunctionTests` (JMS), `CachingClientConnectionFactoryTests`,
`AsyncGatewayTests`, `AsyncMessagingTemplateTests`, `GatewayParserTests`, `PriorityChannelTests`, `AggregatorIntegrationTests`: increase timeout
* `EnableIntegrationTests`: use `LogAdjustingTestSupport`
* `FileOutboundChannelAdapterParserTests`: rework `Thread.sleep()` with `CountDownLatch`
* `ConnectionToConnectionTests`: increase timeout and count iteration. Previously with `1sec` we may lose some events. And we can't just rely on the `10sec`,
because the last iteration will be so long
* `TcpOutboundGatewayTests`: `500ms` is so big timeout to wait for the `Exception` that in the high load environment we can yield to other Thread so long.
Like in our case to `server` Thread to send the reply for us. Therefore decrease the Exception timeout to the `50ms` and increase server delay to `2sec`
* `StompInboundChannelAdapterWebSocketIntegrationTests`: remove `@Qualifier("taskScheduler")` as a potential candidate to test against latest SF changes.
We're fine with `SF-4.2.2` and it is just a test-case. So, I don't see reason to wait for their fix here.

STOMP: `session = null` in adapters for any transportError

Polishing
2015-11-14 16:12:23 -05:00
Artem Bilan
e45bb36be2 Upgrade to AMQP-1.5.2
Add `@Qualifier("taskScheduler")` to the `StompInboundChannelAdapterWebSocketIntegrationTests`:
https://build.spring.io/browse/INT-B41-JOB1-491
2015-11-13 15:17:08 -05:00
Artem Bilan
b9730cd183 INT-3882: StompSession Reconnection Logic
JIRA: https://jira.spring.io/browse/INT-3882

Add reconnect support for the StompSession:
* Introduce `recoveryInterval` for the `AbstractStompSessionManager`
* Add reconnect scheduled task
* Add handling of the `ConnectionLostException` into the `AbstractStompSessionManager`,
as well as for the `StompInboundChannelAdapter` and `StompMessageHandler`
* Cover adapters reconnection feature with tests
* Documentation polishing

Rework logic according PR comments

* Make `StompMessageHandler` as a "lazy-load" for the connection
* Add "direct" connect interaction for the `AbstractStompSessionManager`
* Polishing tests

Polishing

Fix `StompAdaptersParserTests#testStompSessionManagerReconnect()` to use "fake" server port

Address PR comments

The further polishing

Some further polishing

* `AbstractStompSessionManager`: `reconnectFuture.cancel(true)` on each `scheduleReconnect()` to avoid something like "DDoS attack"
* Add reconnect feature test for the `StompInboundChannelAdapterWebSocketIntegrationTests`, closing and then refreshing again the `serverContext`
2015-11-13 14:19:18 -05:00
Artem Bilan
9918304954 INT-3873: Fix STOMP WebSocket test
JIRA: https://jira.spring.io/browse/INT-3873

Phase I: add `Log4jLevelAdjuster` to the `StompInboundChannelAdapterWebSocketIntegrationTests` to have more info on the next CI failure.
2015-11-03 11:47:56 -05:00
Gary Russell
1ad51b8051 Fix Stomp Test Race Condition
Test wasn't waiting until the adapter was subscribed after restart.
2015-10-07 09:21:51 -04:00
Artem Bilan
d89dabe72f INT-3778: STOMP Namespace and Documentation
JIRA: https://jira.spring.io/browse/INT-3778

* Add Namespace support for STOMP adapters
* Document the STOMP module
* Fix race condition in the `AbstractStompSessionManager`
* Add `handleTransportError` to the `StompInboundChannelAdapter` and `StompMessageHandler` to log errors during STOMP interactions
* Fix `StompMessageHandler` to handle `RECEIPT` in case of `RECEIPT` header existence, not the common `autoReceipt` option
which can be disable, but the `RECEIPT` header may be present in the message to send
* Increase logging level for the STOMP tests to trace sporadic failures on the CI Server
* Fix several typos

INT-3778: Doc Polishing
2015-08-11 09:17:05 -04:00
Artem Bilan
ec12b289c2 INT-3786: Fix Several Sporadic Test Failures
JIRA: https://jira.spring.io/browse/INT-3786

* Increase `receiveTimeout` for several `QueueChannel` and `CountDownLatch` based tests
* Fix `FileSplitterTests` for Windows compatibility - `UTF-8` for bytes conversion
* Make some STOMP tests as `LongRunning`

The next fixing phase
2015-07-30 15:28:02 -04:00
Artem Bilan
9c96a14cd1 INT-3749: Revert setters for EC
JIRA: https://jira.spring.io/browse/INT-3749

Since some target application could inject there is `EvaluationContext` before, removal of
 `setIntegrationEvaluationContext()` is breaking change.

 This fix reverts all those setters.
2015-06-27 12:36:33 -04:00
Artem Bilan
3392d4e1ab INT-3749: Get rid of IECA logic
JIRA: https://jira.spring.io/browse/INT-3749

Since `IntegrationEvaluationContextAware` isn't so "context-free" resource like `BeanFactory`
 and `ApplicationContext`, but just a specific bean in the context, we can't follow with `BeanPostProcessor` logic - bad architecture by level of responsibility.
 Therefore we should follow with standard Dependency Injection mechanism to retrieve `evaluationContext` for the particular component.

 Since we can't rely on the `@Autowired` because SI can be used from the raw XML configuration,
 we use utility method instead. The pattern to get the proper `integrationEvaluationContext` is:
 ```
 @Override
 protected void onInit() throws Exception {
 		super.onInit();
 		this.evaluationContext = ExpressionUtils.createStandardEvaluationContext(getBeanFactory());
 }
 ```
2015-06-24 14:57:00 -04:00
Artem Bilan
b7221938af INT-3729: Fix STOMP test sporadic failures
JIRA: https://jira.spring.io/browse/INT-3729
2015-06-07 20:17:01 -04:00
Artem Bilan
9b1aab0703 Fix STOMP subscription race condition
Even if we receive a SUBSCRIBE event, it doesn't that the subscription has been registered.
 So, fix the test to wait for the real subscription.
2015-05-27 14:23:48 -04:00
Artem Bilan
9e4c7e00db INT-3685: Introduce STOMP Adapters
JIRA: https://jira.spring.io/browse/INT-3685

* Add `StompInboundChannelAdapter` to subscribe to STOMP destination and receive messages from them.
Destinations can be added/removed (hence subscribed/unsubscribed) at runtime.
* Add `stompMessageHandler` to the send messages to STOMP destinations.
* `destination` can be extracted from `MessageHeaders`, the `destination` and `destinationExpression` are also supported.
* `RECEIPT` Frame is also supported and emitted as `StompReceiptEvent`
* `StompExceptionEvent` is emitted when we have a `failure` for `CONNECT` Frame or as a reaction to the `ERROR` Frame.
* Introduce `StompSessionManager` abstraction to manage the single `StompSession` and allow to share it between different adapters.
* Add `WebSocketStompSessionManager` implementation over `WebSocketStompClient`
* Add `Reactor2TcpStompSessionManager` implementation for target Broker connections
* Add `StompHeaderMapper`

Address PR comments and JavaDocs

Polishing according PR comments

Polishing
2015-05-26 14:10:52 -04:00