Skip to content

cleanup-jooby-rxjava #1171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jknack merged 1 commit into from
Jun 25, 2018
Merged

cleanup-jooby-rxjava #1171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/jooby-rxjava/src/main/java/org/jooby/rx/ExecSchedulerHook.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@

class ExecSchedulerHook extends RxJavaSchedulersHook {

private Throwing.Function<String, Scheduler> schedulers;
private final Throwing.Function<String, Scheduler> schedulers;

public ExecSchedulerHook(final Map<String, Executor> executors) {
// we don't want eager initialization of Schedulers
Expand Down
4 changes: 2 additions & 2 deletions modules/jooby-rxjava/src/main/java/org/jooby/rx/Rx.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ static class DeferredSubscriber extends Subscriber<Object> {

private Deferred deferred;

private AtomicBoolean done = new AtomicBoolean(false);
private final AtomicBoolean done = new AtomicBoolean(false);

public DeferredSubscriber(final Deferred deferred) {
this.deferred = deferred;
Expand All @@ -366,7 +366,7 @@ public DeferredSubscriber(final Deferred deferred) {
@Override
public void onCompleted() {
if (done.compareAndSet(false, true)) {
deferred.resolve((Object) null);
deferred.resolve(null);

@jknack jknack Jun 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you sure about this? Feel we need the cast to Object

@feffi feffi Jun 25, 2018

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.

Hi, as the function "resolve" in Deferred is specified with "Object", it's the least significant variant anyway. "null" will be resolved to "Object". As long as there are no other overrides, this cast can be removed.

resolve(final Object value)

@feffi feffi Jun 25, 2018

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.

BTW, does PowerMock have a problem with proxying URL Objects? All builds fail with a strange NotFoundException in AssetForwardTest...

@jknack jknack Jun 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You're right about resolve(Object), please ignore my comment.

Are you on linux? I did see that error in Travis, still can't reproduce/fix it

@feffi feffi Jun 25, 2018

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.

no problem :-) Happy to help!

Nope, I'm on macOS (so kinda posix yeah). I found this regarding the error:

https://stackoverflow.com/questions/12190377/getting-javassist-notfoundexception-with-powermock-and-powerrule-in-junit-with-m

maybe we just need to switch to the agent version? I'm not that deep into your testing (yet), but maybe this helps?

@feffi feffi Jun 25, 2018

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.

btw: locally I can't reproduce this error neither. All tests (maven/console and IntelliJ) are fine.

}
deferred = null;
}
Expand Down