-
Notifications
You must be signed in to change notification settings - Fork 9
Refactor pricenode #7
Changes from 178 commits
213a8f3
15be897
0dbb878
99c7d8b
cbf2df4
6bc7665
aa86199
4d55e8a
f797162
bf54408
2ad083d
63e9700
948f8f1
2053f1e
3201503
def6da8
e70dcd6
1c32534
5b0edab
832db42
d358bdc
28b9e43
502f1f8
b3cdace
dc4a4b2
19cb583
966dfc2
d681cc5
00f662e
3c3780d
b1d7e5b
f3075bc
b9d4590
9330449
b5d7eb0
d182b49
e551ba7
5f63d96
bb3cede
0cc2376
563a55d
a72bdef
f85c3a7
6721c2f
ba8fb42
cb94448
d6c713d
bae028a
f5b4efa
b75b673
f980047
3cbfb8a
9c02a4a
9daee4d
0373be3
03a628b
de4b98b
f37f4d3
1cc4d07
511edf6
eb1a295
9757b13
c19cefa
1f9b44c
445d782
b6d7fd9
0078cba
aa62aad
c41e4e7
43e5f2a
c962af1
c06acbb
7269369
473969c
4ae8779
c805415
bf1e8a3
6c81d12
bb4602c
b87f4aa
22a96b6
f2aa335
3f61d7f
09afcd7
7f6a431
7a60359
b569464
b9c525f
d732184
a7902a5
afaff31
7dc37aa
8ab9145
2ffc05d
9e8ce07
9de5307
2346085
b14e72c
44ca9ac
5710415
99a50d0
314d4ab
3e32ffb
f7927f9
c1bc990
8d23e93
3e84b0d
c57a1a3
4db05f4
34dd139
f9e8307
ef721eb
24cd258
5e06ec3
3f30f20
3ef1998
dee5b37
fbefa2f
81be033
d69ae4a
5a67f54
4951989
b137048
d741dbc
875b2cf
c0eb50a
5dec7ec
96a98fd
696021b
58cd243
1cb7b31
50e0c81
3adbb24
7e9c9c1
d91f4b1
72c84a1
e5e4a9c
c234628
9c0bcc0
7f2f5a3
79538d6
707b196
2dc30d3
2c77008
2f0b0fb
cd76471
e5bf166
fc79856
5f9541a
1350b47
ae74368
ba51fe5
1228167
aefc452
af5002c
cce8020
9929284
b34e785
efca8c3
5e7989e
1c7e72c
64f51ee
69618cf
52c207d
7de26d7
c858e27
513d9dd
a41d94f
fcb46f5
7a4fd67
30dcaaf
26a0247
ea99368
c14bdf2
34d6909
5f2a7a8
6089480
0af9900
206c398
3df25dd
bf3056a
a87d652
9f5cf6a
4c476ec
b62855f
f42d9fa
5650ae6
d53300d
1a6a601
c61ca98
123f2f3
02edfef
f22814d
9ebf7d6
b8980fc
ca65c30
d46365f
286baf4
f544007
47115f6
a271dcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,17 @@ | ||
# Vim | ||
*.swp | ||
|
||
# IDEA | ||
.idea | ||
*.iml | ||
out | ||
|
||
# Gradle | ||
build | ||
.gradle | ||
out | ||
|
||
# Heroku | ||
.heroku/ | ||
.jdk/ | ||
.profile.d/ | ||
tor/ |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
language: java | ||
jdk: oraclejdk8 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
web: if [ "$HIDDEN" == true ]; then ./tor/bin/run_tor build/install/bisq-pricenode/bin/bisq-pricenode 2 5 3; else build/install/bisq-pricenode/bin/bisq-pricenode 2 5 3; fi | ||
web: if [ "$HIDDEN" == true ]; then ./tor/bin/run_tor java -jar -Dserver.port=$PORT build/libs/bisq-pricenode.jar 2 5 3; else java -jar -Dserver.port=$PORT build/libs/bisq-pricenode.jar 2 5 3; fi |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Refactorings | ||
|
||
The list of stuff remaining to complete the PR at https://github.com/bisq-network/pricenode/pull/7 | ||
|
||
- Document provider implementations w/ links to API docs, etc | ||
- Add integration tests | ||
- Document / discuss how operators should (ideally) operate their pricenodes on a push-to-deploy model, e.g. how it's done on Heroku | ||
|
||
## Non-refactorings | ||
|
||
Most or all of these will become individual issues / PRs. Just capturing them here for convenience now. Not all may make sense. | ||
|
||
- Deprecate existing get* endpoints (e.g. /getAllMarketPrices) in favor of '/exchange-rates', '/fee-estimate; | ||
- Eliminate dependency on bisq-core (only real need now is CurrencyUtil for list of supported coins) | ||
- Remove command line args for fee estimation params; hard-code these values and update them via commits, not via one-off changes by each operator | ||
- Remove 'getParams' in favor of Boot actuator endpoint | ||
- Update bisq-network/exchange to refer to 'provider' as 'pricenode' | ||
- Invert the dependency arrangement. Move 'ProviderRepository' et al from bisq-network/exchange here into | ||
bisq-network/pricenode and have bisq-network/exchange depend on it as a client lib | ||
- Save bandwidth and be idiomatic by not pretty-printing json returned from /getAllMarketPrices et al |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,29 +1,33 @@ | ||
plugins { | ||
id "java" | ||
id "application" | ||
id "org.springframework.boot" version "1.5.10.RELEASE" | ||
} | ||
|
||
sourceCompatibility = 1.8 | ||
targetCompatibility = 1.8 | ||
|
||
mainClassName = "bisq.pricenode.Main" | ||
version = "0.7.0" | ||
version = file("src/main/resources/version.txt").text | ||
|
||
jar.manifest.attributes( | ||
"Implementation-Title": rootProject.name, | ||
"Implementation-Version": version) | ||
"Implementation-Title": rootProject.name, | ||
"Implementation-Version": version) | ||
|
||
jar.archiveName "${rootProject.name}.jar" | ||
|
||
repositories { | ||
jcenter() | ||
maven { url "https://jitpack.io" } | ||
maven { url "https://raw.githubusercontent.com/JesusMcCloud/tor-binary/testrelease/release/" } | ||
} | ||
|
||
dependencies { | ||
compile("com.github.bisq-network.bisq-exchange:core:v0.6.4") | ||
compile("com.sparkjava:spark-core:2.5.2") | ||
compile("com.github.bisq-network.bisq-exchange:common:v0.6.5") | ||
compile("org.knowm.xchange:xchange-bitcoinaverage:4.3.3") | ||
compile("org.knowm.xchange:xchange-coinmarketcap:4.3.3") | ||
compile("org.knowm.xchange:xchange-poloniex:4.3.3") | ||
compile("org.springframework.boot:spring-boot-starter-web:1.5.10.RELEASE") | ||
compile("org.springframework.boot:spring-boot-starter-actuator") | ||
} | ||
|
||
task stage { | ||
dependsOn installDist | ||
dependsOn assemble | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/* | ||
* This file is part of Bisq. | ||
* | ||
* Bisq is free software: you can redistribute it and/or modify it | ||
* under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or (at | ||
* your option) any later version. | ||
* | ||
* Bisq is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public | ||
* License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with Bisq. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package bisq.price; | ||
|
||
import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
import org.springframework.boot.builder.SpringApplicationBuilder; | ||
|
||
import java.io.File; | ||
import java.io.FileInputStream; | ||
import java.io.IOException; | ||
import java.io.UncheckedIOException; | ||
|
||
import java.util.Properties; | ||
|
||
@SpringBootApplication | ||
public class Main { | ||
|
||
public static void main(String[] args) { | ||
new SpringApplicationBuilder(Main.class) | ||
.properties(bisqProperties()) | ||
.run(args); | ||
} | ||
|
||
private static Properties bisqProperties() { | ||
Properties props = new Properties(); | ||
File propsFile = new File(System.getenv("HOME"), ".config/bisq.properties"); | ||
if (propsFile.exists()) { | ||
try { | ||
props.load(new FileInputStream(propsFile)); | ||
} catch (IOException ex) { | ||
throw new UncheckedIOException(ex); | ||
} | ||
} | ||
return props; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* This file is part of Bisq. | ||
* | ||
* Bisq is free software: you can redistribute it and/or modify it | ||
* under the terms of the GNU Affero General Public License as published by | ||
* the Free Software Foundation, either version 3 of the License, or (at | ||
* your option) any later version. | ||
* | ||
* Bisq is distributed in the hope that it will be useful, but WITHOUT | ||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public | ||
* License for more details. | ||
* | ||
* You should have received a copy of the GNU Affero General Public License | ||
* along with Bisq. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
package bisq.price; | ||
|
||
import org.springframework.web.bind.annotation.ModelAttribute; | ||
|
||
import javax.servlet.http.HttpServletRequest; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public abstract class PriceController { | ||
|
||
protected final Logger log = LoggerFactory.getLogger(this.getClass()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why not use Lombok annotations for logger? (@slf4j) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. It's because I've left Lombok out of the new project altogether. We briefly discussed this when I initially extracted the pricenode module here into its own repository. But it bears treating again here: I was the one who originally brought Lombok into the Bisq codebase in its early days. I had been—and in many ways still am—a fan of Lombok annotations. They work wonders for getting rid of line noise, especially with regard to eliminating the need to manually code getters and setters and eliminating the need to generate and maintain The problem is that it violates the principle of least surprise for the majority of Java developers that come across Lombok-annotated classes. They see a strange beast, adorned with Sometimes people are forced abruptly into figuring this out because they try to import Bisq into IDEA or elsewhere, and they see compilation errors. Then they realize, painfully, that they need a Lombok plugin for their IDE. They find and install it, they rebuild and everything is OK. But it was frustrating getting there. I was a fairly big champion of Lombok coming from the world of mostly closed-source enterprise projects, where it's a reasonable tradeoff to use something like Lombok widely within your organization, to document it in your new employee onboarding docs and be done with it. The cost of getting a closed, well-paid team on the same page about this is low, and the benefits fairly high. I think that cost/benefit ratio changes with a project like Bisq. I think it's important that Bisq's components be as simple, understandable and idiomatic as possible. I want any highly experienced Java developer to take one look at a repository like this and feel right at home, instantly able to reason about what's going on, instantly able to be productive. To me, that means using stock-standard Java. It means using widely accepted naming conventions and patterns like those found in Domain Driven Design. It means using well-known, well-supported, thriving open source infrastructure like Spring Boot and the rest of the Spring family provide. It means striving for a situation where any competent Java developer can clone any of our repositories, run a single build command or import a project directly into their IDE of choice and have everything "just work". Keeping Lombok around in new Bisq repositories just doesn't live up to this standard for me. Lombok is something that's great for a closed, fixed-size team, but is a barrier to entry if you're trying to build an open, global, amorphous team like the Bisq DAO. We want to make the process of contribution as smooth as possible for everyone—especially new people. I just don't see the convenience Lombok offers as being worth it anymore with these goals in mind. P.S. Other issues can crop up with Lombok, too, especially where new language-level Java features are involved. I can't cite specific examples off the top of my head, but more than once, I've had to deal with Lombok being unable to modify or otherwise deal with certain bytecode. I remember problems coming up around Java 5 as well as around Java 8 if memory serves. We want to adopt Java 9 here as soon as possible, and the fewer annotation processors and agents, etc we have is probably for the better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree to most points and I also prefer to stick with plain java instead of too much magical frameworks where someone who is not familiar with need to first learn about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got that, thanks. And to be clear, I am not suggesting that we back Lombok out of the existing bisq-network/exchange repository. This is just where new stuff (or newly extracted) stuff is involved. |
||
|
||
@ModelAttribute | ||
public void logRequest(HttpServletRequest request) { | ||
log.info("Incoming {} request from: {}", request.getServletPath(), request.getHeader("User-Agent")); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dogecoin is not supported anymore, Code is still left just in case, but no plans to re-enable it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git revert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'm deferring this to a separate issue, as it's not part of refactoring, per se. See #9.