621: Generic Read Metadata #1019
Conversation
| try { | ||
| PreparedStatement statement = | ||
| connection.prepareStatement( | ||
| "SELECT * FROM metadata where received_message_id = ? OR sent_message_id = ?"); |
There was a problem hiding this comment.
At some point I think we should separate SQL code from Java code. In the meantime I've been placing the sql code in a variable at the start of the method where it's used, but we should probably start putting them in a more centralized place soon. It could be external files or java constants
| connection -> { | ||
| try { | ||
| PreparedStatement statement = | ||
| connection.prepareStatement( |
There was a problem hiding this comment.
This is nice. For some time I've been thinking of having a wrapper class that will handle the prepared statements. Thoughts on having the wrapper class so that both storage classes can use it with the dao?
| * ResultSetIterator iterates over a SQL {@link ResultSet}. This helps stream over a {@link | ||
| * ResultSet}. | ||
| */ | ||
| public class ResultSetIterator implements Iterator<ResultSet> { |
There was a problem hiding this comment.
Thoughts on implementing the AutoCloseable interface to manage the closure of ResultSet in case the iterator is not fully traversed until the end?
There was a problem hiding this comment.
That is handled outside of the iterator in PostgresDao#fetchData and PostgresDao#fetchManyData already. We put the ResultSet inside the try-with-resources immediately as we get the ResultSet.
|
Nice work, the only thing I see that might need attention is the test coverage. Other than that, everything looks good. |
| @@ -14,18 +14,14 @@ class JjwtEngineTest extends Specification { | |||
|
|
|||
| def "readPrivateKey works"() { | |||
| given: | |||
| def expected = """SunRsaSign RSA private CRT key, 2048 bits | |||
There was a problem hiding this comment.
For some reason this test started to fail in GitHub for me. I decided to refactor the test to not depend on toString().
|
Generic Read Metadata
Made reading from the database generic. It used to be highly coupled to the metadata concept. It can now support arbitrary SQL statements and conversions.
Issue
#621.
Checklist
Note: You may remove items that are not applicable