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

Jump to bottom

8231640: (prop) Canonical property storage #5372

Closed
wants to merge 28 commits into from
+837 −15
Closed

8231640: (prop) Canonical property storage #5372

Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
85748cf
8231640: (prop) Canonical property storage
jaikiran Aug 29, 2021
641864e
use @implNote to explain the use of the environment variable
jaikiran Sep 7, 2021
64e2065
implement review suggestions:
jaikiran Sep 8, 2021
1ded17f
adjust testcases to verify the new semantics
jaikiran Sep 8, 2021
867ec99
update javadoc @implNote to match latest changes
jaikiran Sep 8, 2021
848ded8
Implement Stuart's suggestion on date format:
jaikiran Sep 9, 2021
5326b7c
update the new tests to match the new date format expectations and al…
jaikiran Sep 9, 2021
7736a8f
Implement Stuart's suggestion to use Collections instead of Arrays fo…
jaikiran Sep 9, 2021
c9d3cb8
update javadoc to match latest changes
jaikiran Sep 9, 2021
a29d0f0
dummy commit to trigger GitHub actions job to try and reproduce an un…
jaikiran Sep 9, 2021
a9b71d2
Implement Roger's suggestions:
jaikiran Sep 10, 2021
06ff3bd
jcheck fix - trailing whitespace in test
jaikiran Sep 10, 2021
1d24a3a
allow free-form text for java.util.Properties.storeDate system property
jaikiran Sep 10, 2021
c1dfb18
Update javadoc based on Stuart's inputs
jaikiran Sep 12, 2021
ff34ad5
Implement Roger's and the agreed upon decision to allow free-form tex…
jaikiran Sep 14, 2021
6447f9b
Merge latest from master branch
jaikiran Sep 14, 2021
9237466
unused imports
jaikiran Sep 14, 2021
315f3c8
update the javadoc to clarify how line terminator characters are hand…
jaikiran Sep 14, 2021
14711a9
Add a @implNote to specify the order in which the properties are writ…
jaikiran Sep 14, 2021
6f5f1be
Introduce a test to make sure backslash character in the system prope…
jaikiran Sep 14, 2021
7098a2c
Address review suggestions:
jaikiran Sep 15, 2021
79d1052
- Clarify how overriden Properties#entrySet() method impacts the orde…
jaikiran Sep 15, 2021
e2effb9
(Only doc/comment changes):
jaikiran Sep 16, 2021
eb31d28
- Implement Mark's suggestion in CSR, to rename java.util.Properties.…
jaikiran Sep 17, 2021
458c1fd
Implement Mark's suggestion in CSR to include the java.properties.dat…
jaikiran Sep 17, 2021
e350721
Roger's suggestion to reword the implSpec text
jaikiran Sep 18, 2021
ed5c221
reduce the line length of code comment
jaikiran Sep 22, 2021
944cbf1
Revert "Implement Mark's suggestion in CSR to include the java.proper…
jaikiran Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 src/java.base/share/classes/java/util/Properties.java
View file
Open in desktop
@@ -815,7 +815,7 @@ public void save(OutputStream out, String comments) {
* character in comments is not character {@code #} or character {@code !} then
* an ASCII {@code #} is written out after that line separator.
Copy link
Member

@dfuch dfuch Sep 14, 2021

Choose a reason for hiding this comment

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

Should something be done for comments ending with \ (backslash) ? It might otherwise suppress the first property assignment that follows.

Copy link
Member Author

@jaikiran jaikiran Sep 14, 2021

Choose a reason for hiding this comment

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

There has been no change in how we deal with this aspect. The existing specification (stated in the load method) says:

* Properties are processed in terms of lines. There are two
* kinds of line, <i>natural lines</i> and <i>logical lines</i>.
* A natural line is defined as a line of
* characters that is terminated either by a set of line terminator
* characters ({@code \n} or {@code \r} or {@code \r\n})
* or by the end of the stream. A natural line may be either a blank line,
* a comment line, or hold all or some of a key-element pair. A logical
* line holds all the data of a key-element pair, which may be spread
* out across several adjacent natural lines by escaping
* the line terminator sequence with a backslash character
* {@code \}.  **Note that a comment line cannot be extended
* in this manner;**

(emphasis on that last sentence).
I'll anyway go ahead and add new tests around this to be sure that this works as advertised.

Copy link
Member

@dfuch dfuch Sep 14, 2021

Choose a reason for hiding this comment

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

Oh - great - thanks - verifying by a test that it also applies to the comment specified by java.util.Properties.storeDate would be good.

Copy link
Member Author

@jaikiran jaikiran Sep 14, 2021

Choose a reason for hiding this comment

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

Done. A new test StoreReproducibilityTest#testBackSlashInStoreDateValue has been added in the latest updated version of this PR. This test passes. Plus I checked the written out properties file, from these tests, for such values in java.util.Properties.storeDate and the content matches what the spec says.
Just for quick reference - a run of that test case with the "newline-plus-backslash...." system property value (cannot paste that exact string value from that test case because GitHub editor is messing up the special characters) generates output like below:

#some user specified comment
#newline-plus-backslash\
#c=d
a=b

* <p>
* If the {@systemProperty java.util.Properties.storeDate} is set on the command line
* If the {@systemProperty java.properties.date} is set on the command line
* and is non-empty (as determined by {@link String#isEmpty() String.isEmpty}),
* a comment line is written as follows.
* First, a {@code #} character is written, followed by the contents
@@ -946,12 +946,12 @@ private void store0(BufferedWriter bw, String comments, boolean escUnicode)
}

private static void writeDateComment(BufferedWriter bw) throws IOException {
// value of java.util.Properties.storeDate system property isn't sensitive
// value of java.properties.date system property isn't sensitive
// and so doesn't need any security manager checks to make the value accessible
// to the callers
String storeDate = StaticProperty.javaUtilPropertiesStoreDate();
if (storeDate != null && !storeDate.isEmpty()) {
writeComments(bw, storeDate);
String sysPropVal = StaticProperty.javaPropertiesDate();
if (sysPropVal != null && !sysPropVal.isEmpty()) {
writeComments(bw, sysPropVal);
} else {
bw.write("#" + new Date());
bw.newLine();
12 src/java.base/share/classes/jdk/internal/util/StaticProperty.java
View file
Open in desktop
@@ -51,7 +51,7 @@ public final class StaticProperty {
private static final String JAVA_IO_TMPDIR;
private static final String NATIVE_ENCODING;
private static final String FILE_ENCODING;
private static final String JAVA_UTIL_PROPERTIES_STOREDATE;
private static final String JAVA_PROPERTIES_DATE;

private StaticProperty() {}

@@ -68,7 +68,7 @@ private StaticProperty() {}
JDK_SERIAL_FILTER_FACTORY = getProperty(props, "jdk.serialFilterFactory", null);
NATIVE_ENCODING = getProperty(props, "native.encoding");
FILE_ENCODING = getProperty(props, "file.encoding");
JAVA_UTIL_PROPERTIES_STOREDATE = getProperty(props, "java.util.Properties.storeDate", null);
JAVA_PROPERTIES_DATE = getProperty(props, "java.properties.date", null);
}

private static String getProperty(Properties props, String key) {
@@ -231,14 +231,14 @@ public static String fileEncoding() {
}

/**
* Return the {@code java.util.Properties.storeDate} system property.
* Return the {@code java.properties.date} system property.
*
* <strong>{@link SecurityManager#checkPropertyAccess} is NOT checked
* in this method.</strong>
*
* @return the {@code java.util.Properties.storeDate} system property
* @return the {@code java.properties.date} system property
*/
public static String javaUtilPropertiesStoreDate() {
return JAVA_UTIL_PROPERTIES_STOREDATE;
public static String javaPropertiesDate() {
return JAVA_PROPERTIES_DATE;
}
}