Skip to content

Synchronizing changes from Boeing to open source#137

Merged
dlamoris merged 3 commits intoOpen-MBEE:developfrom
BA-MBSE:ba_opensource_sync_20210202
Feb 5, 2021
Merged

Synchronizing changes from Boeing to open source#137
dlamoris merged 3 commits intoOpen-MBEE:developfrom
BA-MBSE:ba_opensource_sync_20210202

Conversation

@BA-MBSE
Copy link
Collaborator

@BA-MBSE BA-MBSE commented Feb 2, 2021

Update ElementsResponse to include commit id when committing changes
Branch id input sanitization
Add option to disable TWC auth delegation & host parsing cleanup
Added TokenService to allow use of tokens in other contexts
Allow password setting for admins (user conversion from external to internal)
Handle setting of null passwords (user conversion from internal to external)

Branch id input sanitization
Add option to disable TWC auth delegation & host parsing cleanup
Added TokenService to allow use of tokens in other contexts
Allow password setting for admins (user conversion from external to internal)
Handle setting of null passwords (user conversion from internal to external)
@BA-MBSE BA-MBSE marked this pull request as ready for review February 2, 2021 15:05
@BA-MBSE BA-MBSE requested review from HuiJun and dlamoris February 2, 2021 15:05
public String getSuffix() {
String refId = ContextHolder.getContext().getBranchId();
return refId.equals(ContextObject.MASTER_BRANCH) ? "" : refId.toLowerCase();
if(Pattern.matches("^[a-zA-Z0-9-_]*$", refId)) {
Copy link
Collaborator

@HuiJun HuiJun Feb 2, 2021

Choose a reason for hiding this comment

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

getSuffix() is used in a number of places multiple times in our DAOs. Recompiling the pattern every time could potentially have an effect on performance, albeit, probably not significantly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

there's a BRANCH_ID_VALID_PATTERN in the BranchesController - may be better if that gets moved to core Formats?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think so

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated to use a common Pattern

}

private static String stripHost(String url) {
Pattern pattern = Pattern.compile("(https?://)?([\\w-\\.]*)(:\\d+)?");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should check this and possibly move it to a static final.

package org.openmbee.mms.core.objects;

public class ElementsCommitResponse extends ElementsResponse {
private String commitId;
Copy link
Collaborator

Choose a reason for hiding this comment

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

add @Schema(nullable = true) for openapi

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

Moved BRANCH_ID_VALID_PATTERN to core Constants & used both places
Added @Schema annotation to ElementsCommitResponse
public String getSuffix() {
String refId = ContextHolder.getContext().getBranchId();
if(Pattern.matches("^[a-zA-Z0-9-_]*$", refId)) {
if(Pattern.matches(BRANCH_ID_VALID_PATTERN, refId)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I understand Pattern.matches(String, String) is a convenience method that does Pattern.matches(Pattern.compile(String), String) for regex patterns that are used only once. The expensive bit is Pattern.compile, which should instead be defined as static final so that it is only compiled once at initialization.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to a static Pattern instead

@dlamoris dlamoris merged commit 8203107 into Open-MBEE:develop Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants