Skip to content
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.

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

Updated JavaDocs across driver - Part 1 #754

Merged
merged 10 commits into from
Jul 25, 2018

Conversation

lilgreenbird
Copy link
Member

No description provided.

@codecov-io
Copy link

codecov-io commented Jul 21, 2018

Codecov Report

Merging #754 into dev will increase coverage by 0.02%.
The diff coverage is 60%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #754      +/-   ##
============================================
+ Coverage     48.35%   48.38%   +0.02%     
- Complexity     2771     2778       +7     
============================================
  Files           116      116              
  Lines         27871    27871              
  Branches       4631     4631              
============================================
+ Hits          13476    13484       +8     
+ Misses        12212    12207       -5     
+ Partials       2183     2180       -3
Flag Coverage Δ Complexity Δ
#JDBC42 47.81% <60%> (-0.02%) 2731 <0> (+4)
#JDBC43 48.27% <60%> (+0.06%) 2773 <0> (+7) ⬆️
Impacted Files Coverage Δ Complexity Δ
src/main/java/microsoft/sql/DateTimeOffset.java 42.46% <ø> (ø) 11 <0> (ø) ⬇️
...QLServerColumnEncryptionAzureKeyVaultProvider.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...oft/sqlserver/jdbc/SQLServerCallableStatement.java 28.16% <ø> (ø) 96 <0> (ø) ⬇️
...sqlserver/jdbc/SQLServerBulkBatchInsertRecord.java 43.04% <ø> (ø) 27 <0> (ø) ⬇️
...crosoft/sqlserver/jdbc/dns/DNSKerberosLocator.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...SQLServerColumnEncryptionJavaKeyStoreProvider.java 63.63% <ø> (ø) 10 <0> (ø) ⬇️
.../com/microsoft/sqlserver/jdbc/SQLServerDriver.java 80.18% <ø> (ø) 25 <0> (ø) ⬇️
...icrosoft/sqlserver/jdbc/SQLServerConnection43.java 42.85% <ø> (ø) 3 <0> (ø) ⬇️
...icrosoft/sqlserver/jdbc/SQLServerXAConnection.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...com/microsoft/sqlserver/jdbc/dns/DNSUtilities.java 0% <ø> (ø) 0 <0> (ø) ⬇️
... and 56 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fef350...beccd2b. Read the comment docs.

@@ -1705,7 +1704,7 @@ private void validateStringBinaryLengths(Object colValue, int srcCol, int destCo
}

/*
* Retrieves the column metadata for the destination table (and saves it for later)
* Returns the column metadata for the destination table (and saves it for later)
*/
private void getDestinationMetadata() throws SQLServerException {
Copy link
Member

Choose a reason for hiding this comment

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

I know this is private method, but its with single asterisk, and so is considered as comment, not Javadoc. Might as well fix it here & in next method.

@@ -21,7 +21,7 @@


/**
* SQLServerParameterMetaData provides JDBC 3.0 meta data for prepared statement parameters.
* Provides JDBC 3.0 meta data for prepared statement parameters.
Copy link
Member

@cheenamalhotra cheenamalhotra Jul 21, 2018

Choose a reason for hiding this comment

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

We can remove 'JDBC 3.0' from here. Its simply metadata for PS params. It was done to track 3.0/4.0 API differences. Same applied to Savepoints implemented class.

* if an exception occurs
* Well-Known Text (WKT) provided by the user.
* @param srid
* Spatial Reference Identifier (SRID) provided by the user.
Copy link
Member

Choose a reason for hiding this comment

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

SRID = Spatial Reference System Identifier

@@ -401,7 +411,7 @@ protected void parseWkb() {
}
}
}

Copy link
Member

Choose a reason for hiding this comment

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

I hope the entire project is formatted.

@@ -531,7 +531,7 @@ static JavaType of(Object obj) {
return JavaType.OBJECT;
}

// Retrieve JDBC to use with this Java type. By default we use the static JDBC type
// Returns the JDBC type to use with this Java type. By default we use the static JDBC type
// associated with the Java type, ignoring the JDBC type specified by the application.
// But this behavior is overridden for certain Java types, like InputStream, which
// require the JDBC type to be specified externally to be able to distinguish between
Copy link
Member

@rene-ye rene-ye Jul 23, 2018

Choose a reason for hiding this comment

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

4 lines of comments should be wrapped with /* */ instead of 4 //. The comment is also a little off grammatically. Maybe it should be changed to something along the lines of the following:

Returns the JDBC type to use with this Java type. We use the static JDBC type associated with the Java type by default, ignoring the JDBC type specified by the application. This behavior is overridden for certain Java types, such as InputStream, which requires the JDBC type to be specified externally in order to distinguish between...

@@ -9,22 +9,22 @@


/**
* The ISQLServerBulkRecord interface can be used to create classes that read in data from any source (such as a file)
* Provides an interface used to create classes that read in data from any source (such as a file)
* and allow a SQLServerBulkCopy class to write the data to SQL Server tables.
Copy link
Member

Choose a reason for hiding this comment

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

and allows

@@ -644,7 +650,7 @@
public boolean getEnablePrepareOnFirstPreparedStatementCall();

/**
* This setting controls how many outstanding prepared statement discard actions (sp_unprepare) can be outstanding
* Sets the value that setting controls how many outstanding prepared statement discard actions (sp_unprepare) can be outstanding
Copy link
Member

Choose a reason for hiding this comment

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

Delete "setting"?

*
* @return true if statement pooling is disabled, false if it is enabled.
*/
public boolean getDisableStatementPooling();

/**
* Setting the socket timeout
* Sets the socket timeout
Copy link
Member

Choose a reason for hiding this comment

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

No period at the end. Also consider "Sets the socket timeout value."?

@@ -704,7 +708,7 @@
public void setSocketTimeout(int socketTimeout);

/**
* Getting the socket timeout
* Returns the socket timeout
Copy link
Member

Choose a reason for hiding this comment

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

Same as setter.

@@ -752,7 +756,7 @@
public void setSSLProtocol(String sslProtocol);

/**
* Retrieves value of connection property 'sslProtocol'
* Returns the value of connection property 'sslProtocol'
Copy link
Member

@rene-ye rene-ye Jul 23, 2018

Choose a reason for hiding this comment

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

This change and the ones below are all missing periods at the end. Not necessarily an issue but just bringing attention to these changes to check for consistency.

Edit: need to view the whole file for changes for "below" context.

Copy link
Contributor

@ulvii ulvii Jul 24, 2018

Choose a reason for hiding this comment

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

We have 'sslProtocol' here(single quotes) and "fips" (double quotes) above. Please refer to the guideline to make these(there are a few more below) consistent.

@@ -646,7 +644,7 @@ public void setSmallDateTime(int parameterIndex, java.sql.Timestamp x,
boolean forceEncrypt) throws SQLServerException;

/**
* Populates a table valued parameter with a data table
* Sets the data table to populates a table valued parameter
Copy link
Member

Choose a reason for hiding this comment

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

Periods at the end for this and 2 other setters below.

@@ -1563,7 +1565,7 @@ public void updateObject(String columnName, Object x, int precision, int scale,
boolean forceEncrypt) throws SQLServerException;

/**
* Exposes Data Classification information for the current ResultSet For SQL Servers that do not support Data
* Returns the Data Classification information for the current ResultSet For SQL Servers that do not support Data
* Classification or results that do not fetch any classified columns, this data can be null
Copy link
Member

Choose a reason for hiding this comment

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

Period at the end.

*
* Specifies the spatial data types values
*
*/
public enum InternalSpatialDatatype {
Copy link
Member

Choose a reason for hiding this comment

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

Extra lines to be removed..

@@ -57,7 +57,7 @@
private static final java.util.logging.Logger loggerExternal = java.util.logging.Logger.getLogger(loggerClassName);

/**
* Creates a simple reader to parse data from a delimited file with the given encoding.
* Contructs a simple reader to parse data from a delimited file with the given encoding.
Copy link
Member

Choose a reason for hiding this comment

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

Constructs

/**
* this method is called against jdbc41, but it require jdbc42 to work
* therefore, we will throw exception.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Needs rewording.
We are now JDBC 42 compliant, so this function no longer throws exception. We can simply generate Javadocs and specify its usage in Bulk Copy operation.

Copy link
Contributor

Choose a reason for hiding this comment

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

"this method is called against jdbc41, but it require jdbc42 to work therefore, we will throw exception." - Terrible.

@@ -52,7 +52,7 @@


/**
* SQLServerConnection implements a JDBC connection to SQL Server. SQLServerConnections support JDBC connection pooling
* Provides implementation a JDBC connection to SQL Server. SQLServerConnections support JDBC connection pooling
Copy link
Member

Choose a reason for hiding this comment

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

Provides an implementation of java.sql.Connection Interface that assists creating a JDBC Connection to SQL Server.

@@ -10,8 +10,8 @@


/**
* SQLServerConnection43 extends {@link SQLServerConnection43} class and implements {@link ISQLServerConnection43} with
* methods introduced in JDBC 4.3 Specifications. This class is used by the drdiver when initializing a class with 43
* Extends {@link SQLServerConnection43} class and implements {@link ISQLServerConnection43} with
Copy link
Member

Choose a reason for hiding this comment

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

Extends SQLServerConnection (looks like it was wrongly added)

*
* Provides methods to connect to a SQL Server database and to obtain information about the JDBC driver.
*
*/
public final class SQLServerDriver implements java.sql.Driver {
Copy link
Member

Choose a reason for hiding this comment

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

Please truncate extra lines..

@@ -34,7 +34,7 @@


/**
* SQLServerPreparedStatement provides JDBC prepared statement functionality. SQLServerPreparedStatement provides
* Provides JDBC prepared statement functionality. SQLServerPreparedStatement provides
Copy link
Member

Choose a reason for hiding this comment

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

Provides an implementation of java.sql.PreparedStatement Interface that assists in preparing Statements for SQL Server.

@@ -10,13 +10,13 @@


/**
* Provides an implementation of the result set metadata to the SQL Server.
Copy link
Member

Choose a reason for hiding this comment

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

Provides an implementation of java.sql.ResultSetMetadata JDBC Interface to assist in working with ResultSet metadata for SQL Server

@@ -9,7 +9,7 @@


/**
* SQLServerSavepoint implements JDBC 3.0 savepoints. A savepoint is checkpoint to which a transaction can be rolled
* Provides an implementation of the JDBC 3.0 savepoints. A savepoint is checkpoint to which a transaction can be rolled
Copy link
Member

Choose a reason for hiding this comment

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

JDBC 3.0 savepoints can be replaced by JDBC Interface java.sql.Savepoint

@@ -29,7 +29,7 @@


/**
* SQLServerStatment provides the basic implementation of JDBC statement functionality. It also provides a number of
* Provides the basic implementation of JDBC statement functionality. It also provides a number of
Copy link
Member

Choose a reason for hiding this comment

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

Provides an implementation of java.sql.Statement JDBC Interface to assist in creating Statements against SQL Server.

(might be able to word better but basic idea is to specify implemented interface, and the basic functionality of class against SQL Server)

@@ -311,22 +313,22 @@
public void setSendStringParametersAsUnicode(boolean sendStringParametersAsUnicode);

/**
* Returns a boolean value that indicates if sending string parameters to the server in UNICODE format is enabled.
* Returns whether string parameters to the server in UNICODE format is enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

"whether sending"

@cheenamalhotra cheenamalhotra added this to the 7.0.0 milestone Jul 24, 2018
*
* @return login configuration file name
*/
public String getJASSConfigurationName();

/**
* Enables Fips Mode on the connection For FIPS enabled JVM this property should be true.
* Sets Fips Mode on the connection For FIPS enabled JVM this property should be true.
Copy link
Contributor

Choose a reason for hiding this comment

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

"Sets Fips Mode" does not make much sense. I would change it to "Sets the value to enable/disable FIPS Mode..."

@@ -367,7 +367,6 @@ public void setBigDecimal(int parameterIndex, BigDecimal x, int precision, int s
public void setLong(int parameterIndex, long x, boolean forceEncrypt) throws SQLServerException;

/**
* <p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

why is it necessary? this isn't in any of the other methods


/**
* Populates a table valued parameter with a data table
* Sets the result set to populate a table valued parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

please use "table-valued" instead of "table valued"

*
* @return the label for Savepoint
*/
public String getLabel();

/**
* Checks if the savepoint label is null
* Returns if the savepoint label is null
Copy link
Contributor

Choose a reason for hiding this comment

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

Please go through the changes and make sure use of periods is consistent.

@@ -38,7 +38,7 @@
public String getResponseBuffering() throws SQLServerException;

/**
* Retrieves the <code>cancelQueryTimeout</code> property set on this SQLServerStatement object.
* Returns the <code>cancelQueryTimeout</code> property set on this SQLServerStatement object.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another inconsistency when highlighting properties.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you want to be done here

Copy link
Contributor

@ulvii ulvii Jul 24, 2018

Choose a reason for hiding this comment

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

I was just pointing out there are 3 different ways we highlight code.

  1. ''
  2. ""
  3. code tag

@@ -268,7 +268,7 @@ public void setFireTriggers(boolean fireTriggers) {
}

/**
* Indicates if allowEncryptedValueModifications option is enabled or not
* Returns if allowEncryptedValueModifications option is enabled or not
Copy link
Contributor

Choose a reason for hiding this comment

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

No period at the end. Also property is not highlighted.

@@ -72,8 +72,8 @@ public String getName() {
}

/**
* Constructor that takes a callback function to authenticate to AAD. This is used by KeyVaultClient at runtime to
* authenticate to Azure Key Vault.
* Constructs a SQLServerColumnEncryptionAzureKeyVaultProvider with a callback function to authenticate to AAD and an executor service..
Copy link
Contributor

Choose a reason for hiding this comment

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

".."?

@@ -294,7 +297,7 @@ else if (val instanceof OffsetTime)
}

/**
* Retrieves <code>java.util.Map</code> object type of columnMetaData for all columns where column indexes are
* Returns the <code>java.util.Map</code> object type of columnMetaData for all columns where column indexes are
Copy link
Contributor

@ulvii ulvii Jul 24, 2018

Choose a reason for hiding this comment

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

We should probably use code tag instead of quotes when highlighting properties, like here.

@@ -193,7 +192,7 @@ public SQLServerException(String errText, Throwable cause) {
}

/**
* Build a new SQL Exception from an error detected by the driver.
* Builds a new SQL Exception from an error detected by the driver.
Copy link
Contributor

Choose a reason for hiding this comment

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

Builds or constructs?

@@ -505,7 +505,7 @@ private void verifyResultSetIsUpdatable() throws SQLServerException {
}

/**
* Checks whether the result set has a current row.
* Returns if whether the result set has a current row.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "if"

@@ -964,7 +964,7 @@ private void updateCurrentRow(int rowsToMove) {
}

/**
* Initially moves the cursor to the first row of this ResultSet object, with subsequent calls moving the cursor to
* Moves the cursor to the first row of this ResultSet object initially, then subsequent calls moves the cursor to
Copy link
Contributor

Choose a reason for hiding this comment

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

"ubsequent calls move"

@@ -129,7 +128,7 @@ void setBaseJDBCType(JDBCType baseJDBCType) {
}

/**
* retrieves the base type as jdbc type
* Returns the base type as jdbc type
Copy link
Contributor

Choose a reason for hiding this comment

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

JDBC

* attempts to connect to the first IP address available. If the first attempt fails, the driver tries to connect to
* all IP addresses in parallel until the timeout expires, discarding any pending connection attempts when one of
* them succeeds.
* Sets the value to enable/disable Transparent Netowrk IP Resolution (TNIR) Beginning in version 6.0 of the
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a missing period after (TNIR)

@@ -264,7 +264,7 @@ private CertificateDetails getCertificateDetailsByAlias(KeyStore keyStore, Strin
}

/**
* Encrypt plainText with the certificate provided
* Encrypt. plainText with the certificate provided.
Copy link
Member

Choose a reason for hiding this comment

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

Extra period?

@@ -334,7 +329,7 @@ SqlFedAuthToken getAuthenticationResult() {
}

/**
* Struct encapsulating the data to be sent to the server as part of Federated Authentication Feature Extension.
* Eencapsulates the data to be sent to the server as part of Federated Authentication Feature Extension.
Copy link
Member

Choose a reason for hiding this comment

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

Typo

@@ -325,7 +325,7 @@ private void parseQueryMetaFor2008(ResultSet rsQueryMeta) throws SQLServerExcept
}

/**
* Escape parser, using the tokenizer tokenizes escaped strings properly e.g.[Table Name, ]
* Parses escaped strings properly e.g.[Table Name, ] using tokenizer/
Copy link
Member

Choose a reason for hiding this comment

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

Weird backslash at the end? Not sure if intended, bringing attention.

@@ -336,7 +335,7 @@ final void closeInternal() {
}

/**
* Intialize the statement parameters.
* Intializes the statement parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Missing an "I" -> Initializes?

@@ -2340,7 +2331,7 @@ public final String getResponseBuffering() throws SQLServerException {


/**
* Helper class that does some basic parsing work for SQL statements that are stored procedure calls.
* Provides a help class that does some basic parsing work for SQL statements that are stored procedure calls.
Copy link
Member

Choose a reason for hiding this comment

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

"Helper class" is the correct term.

* SQLServerPooledConnection represents a database physical connection in a connection pool. If provides methods for the
* connection pool manager to manage the connection pool. Applications typically do not instantiate these connections
* directly.
* Represents a database physical connection in a connection pool. If provides methods for the connection pool manager
Copy link
Member

Choose a reason for hiding this comment

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

database physical connection -> physical database connection

@lilgreenbird lilgreenbird merged commit 2d0f906 into microsoft:dev Jul 25, 2018
@lilgreenbird lilgreenbird deleted the javadocs branch July 25, 2018 21:28
@cheenamalhotra cheenamalhotra changed the title updated javadocs Updated JavaDocs across driver - Part 1 Jul 26, 2018
cheenamalhotra added a commit that referenced this pull request Jul 31, 2018
* Update Snapshot for upcoming RTW release v7.0.0

* Change order of logic for checking the condition for using Bulk Copy API (#736)

Fix | Change order of logic for checking the condition for using Bulk Copy API (#736)

* Update CHANGELOG.md

* Merge pull request #732 from cheenamalhotra/module (Export driver in automatic module)

Introduce Automatic Module Name in POM.

* Update CHANGELOG.md

* Change Sha1HashKey to CityHash128Key for generating PreparedStatement handle and metadata cache keys (#717)

* Change Sha1HashKey to CityHash128Key

* Formatted code

* Prepared statement performance fixes

1) Further speedups to prepared statement hashing

2) Caching of '?' chararacter positiobs in prepared statements to speed parameter substitution

* String compare for hash keys
added missing line for bulkcopy tests.

* comment change

* Move CityHash class to a separate file

* spacings fixes
cleaner code & logic

* Add | Adding useBulkCopyForBatchInsert property to Request Boundary methods (#739)

* Apply the collation name change to UTF8SupportTest

* Package changes for CityHash with license information (#740)

* Reformatted Code + Updated formatter (#742)

* Reformatted Code + Updated formatter

* Fix policheck issue with 'Country' keyword (#745)

* Adding a new test for beginRequest()/endRequest() (#746)

* Add | Adding a new test to notify the developers to consider beginRequest()/endRequest() when adding a new Connection API

* Fix | Fixes for issues reported by static analysis tools (SonarQube + Fortify) (#747)

* handle buffer reading

for invalid buffer input by user

* Revert "handle buffer reading"

This reverts commit 11e2bf4.

* updated javadocs (#754)

* fixed some typos in javadocs (#760)

* API and JavaDoc changes for Spatial Datatypes (#752)

Add | API and JavaDoc changes for Spatial Datatypes (#752)

* Disallow non-parameterized queries for Bulk Copy API for batch insert (#756)

fix | Disallow non-parameterized queries for Bulk Copy API for batch insert (#756)

* Formatting | Change scope of unwanted Public APIs + Code Format (#757)

* Fix unwanted Public APIs.
* Updated formatter to add new line to EOF + formatted project

* Release | Release 7.0 changelog and version update (#748)

* Updated Changelog + release version changes

* Changelog entry updated as per review.

* Updated changelog for new changes

* Terminology update: request boundary declaration APIs

* Trigger Appveyor

* Update Samples and add new samples for new features (#761)

* Update Samples and add new Samples for new features

* Update samples from Peter

* Updated JavaDocs

* Switch to block comment

* Update License copyright (#767)
@lilgreenbird lilgreenbird added this to Closed/Merged PRs in MSSQL JDBC Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

None yet

5 participants