-
Notifications
You must be signed in to change notification settings - Fork 24.6k
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
Deprecation info for joda-java migration #41956
Merged
pgomulka
merged 54 commits into
elastic:6.8
from
pgomulka:feature/deprecation_info_joda
May 29, 2019
Merged
Changes from 16 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
8f99fc7
initial work
pgomulka dbc725c
init version
pgomulka 937fe06
draft working version
pgomulka 92dd79f
cleanup
pgomulka 0ab2a6e
commented code remove
pgomulka 353af6a
joda deprecation keys
pgomulka 21255e1
generic deprecation key
pgomulka 6e85a05
empty lines
pgomulka 8ca0fee
message fix, prefix8 suggestion
pgomulka c94bbe7
fixing test for warnings in headers
pgomulka 5e7bdb3
do not report deprecation when format starts with 8
pgomulka ba913f5
unused improt
pgomulka 7bc3ea7
remove ignore annotation
pgomulka 837dea3
field extraction
pgomulka 23f11f4
changes to support defined format names
pgomulka a758afb
change imports
pgomulka c282f14
compile fix
pgomulka 1c1e5bd
disable warning comment change
pgomulka 160a697
Joda is using format names
pgomulka f6776a4
snake case
pgomulka 9edcc0a
missing snake case name
pgomulka f57e855
all names in a set
pgomulka 03bce11
code style
pgomulka e582d38
typo
pgomulka 8860748
fix failing RangeQueryBuilderTest
pgomulka 73a19ad
fixing docs tests failing due to joda-java warning
pgomulka 7461c0f
cleanup
pgomulka beaf55b
failing tests
pgomulka 6f9c402
cleanup and javadoc
pgomulka ecc304e
Merge branch '6.8' into feature/deprecation_info_joda
pgomulka adf2c49
disable warning check
pgomulka 3afeae8
warning failing tests
pgomulka d61eb70
boostrap file revert
pgomulka a23e6e8
old assertion on warning message change
pgomulka a64c61b
testing combined patterns and removing duplicates
pgomulka c853b72
test failures
pgomulka 56bc22d
space after ; when combined pattern suggestion
pgomulka c4632c8
failing tests
pgomulka 55a233f
unused import
pgomulka 9e77047
Update x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpac…
pgomulka 9b98a79
remove intellij instructions
pgomulka aa36731
pipelines support
pgomulka 3e3c106
failing tests
pgomulka f7ee788
checkstyle
pgomulka eeaf96c
deprecation issue message change
pgomulka 0dd1ae1
disable joda deprecation checks by default
pgomulka 0afb36d
remove failing test cases
pgomulka 40509ec
use constant instead of string for usePrefix8
pgomulka 7d3de05
fix tests with warning assertions
pgomulka 8b82383
remove warning assertion
pgomulka 85fd30e
IT test failing fix
pgomulka fe2f17d
Update x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpac…
pgomulka edd0aa1
unused comment and message change
pgomulka 84608c2
revert resetDeprecationLogger method access
pgomulka File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
85 changes: 85 additions & 0 deletions
85
server/src/main/java/org/elasticsearch/common/joda/JodaDeprecationPatterns.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,85 @@ | ||||||
/* | ||||||
* Licensed to Elasticsearch under one or more contributor | ||||||
* license agreements. See the NOTICE file distributed with | ||||||
* this work for additional information regarding copyright | ||||||
* ownership. Elasticsearch licenses this file to you under | ||||||
* the Apache License, Version 2.0 (the "License"); you may | ||||||
* not use this file except in compliance with the License. | ||||||
* You may obtain a copy of the License at | ||||||
* | ||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||
* | ||||||
* Unless required by applicable law or agreed to in writing, | ||||||
* software distributed under the License is distributed on an | ||||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||
* KIND, either express or implied. See the License for the | ||||||
* specific language governing permissions and limitations | ||||||
* under the License. | ||||||
*/ | ||||||
|
||||||
package org.elasticsearch.common.joda; | ||||||
|
||||||
import org.elasticsearch.common.time.DateFormatter; | ||||||
import org.elasticsearch.common.time.FormatNames; | ||||||
|
||||||
import java.util.LinkedHashMap; | ||||||
import java.util.List; | ||||||
import java.util.Map; | ||||||
import java.util.StringJoiner; | ||||||
import java.util.stream.Collectors; | ||||||
|
||||||
public class JodaDeprecationPatterns { | ||||||
private static Map<String, String> JODA_PATTERNS_DEPRECATIONS = new LinkedHashMap<>(); | ||||||
|
||||||
static { | ||||||
JODA_PATTERNS_DEPRECATIONS.put("Y", "'Y' year-of-era becomes 'y'. Use 'Y' for week-based-year."); | ||||||
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 think we should make this a bit clearer as to what we're recommending be done. Something like this:
Suggested change
And similar for the rest of these. |
||||||
JODA_PATTERNS_DEPRECATIONS.put("y", "'y' year becomes 'u'. Use 'y' for year-of-era."); | ||||||
JODA_PATTERNS_DEPRECATIONS.put("C", "'C' century of era is no longer supported."); | ||||||
JODA_PATTERNS_DEPRECATIONS.put("x", "'x' weak-year becomes 'Y'. Use 'x' for zone-offset."); | ||||||
JODA_PATTERNS_DEPRECATIONS.put("Z", | ||||||
"'Z' time zone offset/id fails when parsing 'Z' for Zulu timezone. Consider using 'X'."); | ||||||
JODA_PATTERNS_DEPRECATIONS.put("z", | ||||||
"'z' time zone text. Will print 'Z' for Zulu given UTC timezone."); | ||||||
} | ||||||
|
||||||
public static final String USE_PREFIX_8_WARNING = "Prefix your date format with '8' to use the new specifier."; | ||||||
|
||||||
/** | ||||||
* Returns true if pattern is deprecated. That is when it was not already prefixed with 8 (meaning already upgraded) | ||||||
* and it is not a predefined pattern from <code>FormatNames</code> like basic_date_time_no_millis | ||||||
* and it uses pattern characters which changed meaning from joda to java like Y->y | ||||||
* @param format | ||||||
* @return | ||||||
*/ | ||||||
public static boolean isDeprecatedFormat(String format) { | ||||||
List<String> patterns = DateFormatter.splitCombinedPatterns(format); | ||||||
|
||||||
for (String pattern : patterns) { | ||||||
boolean isDeprecated = pattern.startsWith("8") == false && FormatNames.exist(pattern) == false && | ||||||
JODA_PATTERNS_DEPRECATIONS.keySet().stream() | ||||||
.filter(s -> pattern.contains(s)) | ||||||
.findAny() | ||||||
.isPresent(); | ||||||
if (isDeprecated) | ||||||
return true; | ||||||
} | ||||||
return false; | ||||||
} | ||||||
|
||||||
public static String formatSuggestion(String format) { | ||||||
List<String> patterns = DateFormatter.splitCombinedPatterns(format); | ||||||
|
||||||
StringJoiner joiner = new StringJoiner("; "); | ||||||
for (String pattern : patterns) { | ||||||
if (isDeprecatedFormat(pattern)) { | ||||||
|
||||||
String suggestion = JODA_PATTERNS_DEPRECATIONS.entrySet().stream() | ||||||
.filter(s -> pattern.contains(s.getKey())) | ||||||
.map(s -> s.getValue()) | ||||||
.collect(Collectors.joining("; ")); | ||||||
joiner.add(suggestion); | ||||||
} | ||||||
} | ||||||
return joiner.toString(); | ||||||
} | ||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I need to figure out how to fix Docs tests. At the moment they fail because of warnings
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.
Normally we would update the docs to not use deprecated functionality. Is that possible here?
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.
it would - actually it would be much easier if I just use a new pattern with 8 prefix. But should this be done across all the docs?
That change is to fix the
DocsClientYamlTestSuiteIT
. The options is to prefix with8, assert about warnings, disable warning checks alltogether?