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.

Sign up for GitHub

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

Add repositories info so the generation of docs TIP and overall TIR have the info they need to render the doc. #100

Draft
wants to merge 26 commits into
base: feature/moveLevaDoc
Choose a base branch
from
Draft

Add repositories info so the generation of docs TIP and overall TIR have the info they need to render the doc. #100

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a6bd3d9
Enables tests previously disabled.
Apr 12, 2022
1a4cf0f
Load repositories test data 4 TIP and overall_TIR
Apr 13, 2022
f29d06b
ProjectRepositoryFixture
Apr 13, 2022
42e8385
Need better examples.
Apr 18, 2022
c87ed12
Need to save generated pdf when not as expected one.
Apr 18, 2022
1e20f00
Generates doInstall info before calling to generate doc.
Apr 18, 2022
b9a871f
Do not update pdfs if they are still valid.
Apr 18, 2022
ba6df8b
Improves example input data.
Apr 18, 2022
e7812ea
Updates expected pdf only if changes detected.
Apr 18, 2022
9f3f3ef
Renames method name.
Apr 18, 2022
8dcb4c8
Generated repos urls in docGen.
Apr 18, 2022
badfc5d
Fixes typo in urls, TIP docs.
Apr 18, 2022
03db887
computeRepositoriesDataNeededInDocs
Apr 18, 2022
9a4a6f9
Expected overall TIR.
Apr 18, 2022
3ee8932
Changed expected pdfs, since input data changed.
Apr 18, 2022
24ebad9
changelog
Apr 18, 2022
aedf1ed
Merge branch 'feature/moveLevaDoc' into feature/add-component-params-…
victorpablosceruelo Apr 18, 2022
c1f6462
Changes method name.
Apr 19, 2022
b26ff81
Merge branch 'feature/add-component-params-to-overall-tir-and-tip' of…
Apr 19, 2022
9952740
Loads repositories metadata from repositories yml files.
Apr 20, 2022
e874179
Fixes download method for templates.
Apr 20, 2022
84746f3
This seems to be a bug inherited from sharedLib
Apr 20, 2022
429d5d0
Change in tmp file name.
Apr 20, 2022
5a7eb06
Needed to ensure we do not use uncontrolled field values.
Apr 20, 2022
7b946bc
Fixes incorrect data.
Apr 20, 2022
2f47b39
Wiremock resources.
Apr 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
- Added IT (Docker tests)
- Added performance tests
- logback.xml can be overridden from command line
- removed unused params from payloads.
- removed param jekins to jenkins
- Removed byte[] use
- removed unused params from payloads, jekins -> jenkins, computes values gitUrl and doInstall for each repository.
- Removed byte[] usage

## [4.0] - 2021-18-11
=======
Expand Down
4 changes: 2 additions & 2 deletions src/main/groovy/org/ods/doc/gen/external/modules/git/BitbucketService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class BitbucketService {
return new JsonSlurperClassic().parseText(response)
}

String buildReleaseManagerUrl(String projectId, String releaseManagerRepo) {
URI uri = new URI([getBitbucketURLForDocs(), projectId, releaseManagerRepo].join("/"))
String buildRepositoryUrl(String projectId, String repositoryName) {
URI uri = new URI([getBitbucketURLForDocs(), projectId, repositoryName].join("/"))
return "${uri.normalize().toString()}.git"
}

Expand Down
21 changes: 20 additions & 1 deletion src/main/groovy/org/ods/doc/gen/leva/doc/services/LeVADocumentService.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.ods.doc.gen.leva.doc.services

import groovy.util.logging.Slf4j
import groovy.xml.XmlUtil
import org.ods.doc.gen.external.modules.git.BitbucketService
import org.ods.doc.gen.external.modules.git.BitbucketTraceabilityUseCase
import org.ods.doc.gen.external.modules.jira.CustomIssueFields
import org.ods.doc.gen.external.modules.jira.IssueTypes
Expand Down Expand Up @@ -63,6 +64,7 @@ class LeVADocumentService {
private final LeVADocumentChaptersFileService levaFiles
private final BitbucketTraceabilityUseCase bbt
private final NexusService nexus
private final BitbucketService bitbucketService

@Inject
Clock clock
Expand All @@ -74,14 +76,16 @@ class LeVADocumentService {
JiraUseCase jiraUseCase,
JUnitReportsService junit,
LeVADocumentChaptersFileService levaFiles,
BitbucketTraceabilityUseCase bbt) {
BitbucketTraceabilityUseCase bbt,
BitbucketService bitbucketService) {
this.project = project
this.docGenUseCase = docGenUseCase
this.nexus = nexus
this.jiraUseCase = jiraUseCase
this.junit = junit
this.levaFiles = levaFiles
this.bbt = bbt
this.bitbucketService = bitbucketService
}

@SuppressWarnings('CyclomaticComplexity')
Expand Down Expand Up @@ -522,6 +526,8 @@ class LeVADocumentService {
def keysInDoc = this.computeKeysInDocForTIP(projectData.getComponents())
def docHistory = this.getAndStoreDocumentHistory(documentType, keysInDoc, projectData)

updateRepositoriesData(data)

def data_ = [
metadata: this.getDocumentMetadata(projectData, Constants.DOCUMENT_TYPE_NAMES[documentType]),
data : [
Expand Down Expand Up @@ -945,6 +951,8 @@ class LeVADocumentService {

def watermarkText = this.getWatermarkText(projectData)

updateRepositoriesData(data)

def visitor = { data_ ->
// Prepend a section for the Jenkins build log
data_.sections.add(0, [
Expand Down Expand Up @@ -1639,4 +1647,15 @@ class LeVADocumentService {
junit.getNumberOfTestCases(testData.testResults) - testIssues.count { !it.isUnexecuted }
}

private updateRepositoriesData(Map data) {
String projectId = data.projectId as String

data.repositories.each { Map repo ->
String repoName = repo.id as String
repoName = "${projectId}-${repoName}"
repo["data"]["git"]["url"] = bitbucketService.buildRepositoryUrl(projectId, repoName)

repo["doInstall"] = !Constants.COMPONENT_TYPE_IS_NOT_INSTALLED.contains(repo.type?.toLowerCase())
}
}
}
16 changes: 10 additions & 6 deletions src/main/groovy/org/ods/doc/gen/project/data/ProjectData.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import java.nio.file.Paths
@Service
class ProjectData {

static final String DEFAULT_TEMPLATE_VERSION = '1.2'
static final String BUILD_PARAM_VERSION_DEFAULT = 'WIP'
static final String METADATA_FILE_NAME = 'metadata.yml'

Expand Down Expand Up @@ -69,7 +70,7 @@ class ProjectData {
this.data.jira = [project: [ : ]]
this.data.repo = data.repo
this.data.git = data.git
this.data.git.url = bitbucketService.buildReleaseManagerUrl(
this.data.git.url = bitbucketService.buildRepositoryUrl(
data.projectId as String,
data.git.releaseManagerRepo as String
)
Expand Down Expand Up @@ -190,13 +191,16 @@ class ProjectData {
}

private Map<String, String> loadMetadataRepo(repo) {
if ((! repo) || (! repo.id)) {
throw new RuntimeException("Repository id cannot be blank or null.")
}
return [
id: repo.id,
name: repo.name,
description: "myDescription-A",
supplier: "mySupplier-A",
version: "myVersion-A",
references: "myReferences-A"
name: repo.name ? repo.name : repo.id,
Copy link
Member

Choose a reason for hiding this comment

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

In the other class you said repo.name:
String repoName = repo.id as String
repoName = "${projectId}-${repoName}"
Here is different?
And when is possible to don't have the repo.name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs more work.
Seems to be in the original commit you forgot about this information.
We are now determining how to obtain this information.
Thanks for pointing to it.

description: "myDescription-${repo.id}",
supplier: "mySupplier-${repo.id}",
version: "myVersion-${repo.id}",
references: "myReferences-${repo.id}"
]
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/groovy/org/ods/doc/gen/leva/doc/fixture/DocTypeProjectFixture.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package org.ods.doc.gen.leva.doc.fixture
class DocTypeProjectFixture extends DocTypeProjectFixtureBase {

DocTypeProjectFixture() {
super(["CSD", "DIL", "DTP", "RA", "CFTP", "IVP", "SSDS", "TCP", "TRC"])
super(["CSD", "DIL", "DTP", "RA", "CFTP", "IVP", "SSDS", "TCP", "TIP", "TRC"])
}

DocTypeProjectFixture(docTypes) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/groovy/org/ods/doc/gen/leva/doc/fixture/DocTypeProjectFixturesOverall.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import org.ods.doc.gen.leva.doc.services.Constants
class DocTypeProjectFixturesOverall extends DocTypeProjectFixtureBase {

DocTypeProjectFixturesOverall() {
super([ Constants.DocumentType.DTR.toString() ])
super(Constants.OVERALL_DOC_TYPES)
}

def addDocTypes(Map project, List projects) {
Expand Down
1 change: 1 addition & 0 deletions src/test/groovy/org/ods/doc/gen/leva/doc/fixture/LevaDocDataFixture.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class LevaDocDataFixture {
data.build = buildJobParams(projectFixture)
data.git = buildGitData(projectFixture)
data.openshift = [targetApiUrl:"https://openshift-sample"]
data.repositories = new ProjectRepositoryFixture().load()
Copy link
Member

Choose a reason for hiding this comment

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

Previously we were using getModuleData()
If we don't need it anymor,, please clean the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getModuleData is still used to generate component information sent. Please notice that when documents use component information they do not use repositories information.

Copy link
Member

Choose a reason for hiding this comment

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

1- The data in getModuleDatais the same you put here.
2- Shared is sending this data for everytype of doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1-That is not true. While the previous contains only info for a component, new code cotains different information for each code.
2-If you are asking this, you did not expend enough time reading my code.

return data
}

Expand Down
29 changes: 27 additions & 2 deletions src/test/groovy/org/ods/doc/gen/leva/doc/fixture/LevaDocTestValidator.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import de.redsix.pdfcompare.env.SimpleEnvironment
import groovy.util.logging.Slf4j
import org.apache.commons.io.FileUtils

import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.nio.file.StandardCopyOption

@Slf4j
class LevaDocTestValidator {

Expand All @@ -23,7 +28,12 @@ class LevaDocTestValidator {
boolean validatePDF(String buildId) {
unzipGeneratedArtifact(buildId)
if (GENERATE_EXPECTED_PDF_FILES) {
copyDocWhenRecording(buildId)
boolean comparisonResult = compareFiles(buildId)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove all this part as it will add a lot of time to the test, when if this is GENERATE_EXPECTED_PDF_FILES=true, we expect to copy the results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please take a look at the whole change, not just two lines.
We are updating the docs, but only if we need to.
Pdfs source code can change while the visual result remains the same.
Please comment again if you want us to remove it.

Copy link
Member

@s2oBCN s2oBCN Apr 19, 2022 •
edited by victorpablosceruelo
Loading

Choose a reason for hiding this comment

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

Please, don't put this code, this will add a lot of time and not needed

Victor to so2:
Please do not close conversations if you use soft arguments in your last words. Can't answer them.

It is not needed for you because you did not took time debugging and fixing a single document. Why do I have to update all of them when only one is wrong? Just because you do not like the idea of having two more lines of code? It is nonsense.

if (! comparisonResult) {
log.info("validatePDF - Comparison returned differences.")
log.info("validatePDF - Updating expected pdf since GENERATE_EXPECTED_PDF_FILES=true.")
copyDocWhenRecording(buildId)
}
return true
} else {
return compareFiles(buildId)
Expand All @@ -34,7 +44,7 @@ class LevaDocTestValidator {
String actualPath = actualDoc(buildId).absolutePath
File expectedFile = expectedDoc(buildId, projectFixture.component)
String expectedPath = expectedFile.absolutePath
log.info("validatePDF - Expected pdf:${expectedPath}")
log.info("validatePDF - Expected pdf: ${expectedPath}")

String diffFileName = pdfDiffFileName(expectedFile)

Expand All @@ -49,6 +59,17 @@ class LevaDocTestValidator {
} else {
FileUtils.copyFile(actualDoc(buildId), reportPdfDoc(buildId))
}

if (!filesAreEqual) {
Copy link
Member

Choose a reason for hiding this comment

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

This is the else of the line 59.
Improve the else of the if.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll do that if previous change is accepted. If not, we'll remove this lines because they are related to the previous ones.

String generatedPdfSavedCopyFileName = generatedPdfSavedCopyFileName(actualPath)
Path generatedPdfSource = java.nio.file.Paths.get(actualPath)
Path generatedPdfSavedCopy = Paths.get(generatedPdfSavedCopyFileName)
Files.copy(generatedPdfSource, generatedPdfSavedCopy,
StandardCopyOption.REPLACE_EXISTING)
log.info("validatePDF - Built pdf (saved because different from expected): " +
"${generatedPdfSavedCopyFileName}")
}

return filesAreEqual
}

Expand All @@ -61,6 +82,10 @@ class LevaDocTestValidator {
return "${SAVED_DOCUMENTS}/${expectedFile.name.take(expectedFile.name.lastIndexOf('.'))}-diff"
}

String generatedPdfSavedCopyFileName(String actualPath){
return "${SAVED_DOCUMENTS}/generated-${actualPath.substring(actualPath.lastIndexOf('/') +1)}"
}

private void unzipGeneratedArtifact(String buildId) {
String source = "${tempFolder.absolutePath}/artifacts/${getArtifactName(buildId, projectFixture.component)}.zip"
String destination = "${tempFolder.absolutePath}"
Expand Down
49 changes: 49 additions & 0 deletions src/test/groovy/org/ods/doc/gen/leva/doc/fixture/ProjectRepositoryFixture.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.ods.doc.gen.leva.doc.fixture

class ProjectRepositoryFixture {
Copy link
Member

Choose a reason for hiding this comment

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

This file does not scale to more projects

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yours neither. This makes it possible to test ordgp project.
If we remove them, tests fail.
It is obvious that we'll have to fix your work in the future, but right now we can live with it.

Copy link
Member

@s2oBCN s2oBCN Apr 19, 2022 •
edited by victorpablosceruelo
Loading

Choose a reason for hiding this comment

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

No, this is not true. This data does not scale at all

Victor to so2:
Please do not close conversations if you use soft arguments in your last words. Can't answer them.
Yours code in LevaDocDataFixture scales, right?
Example:
targetEnvironmentToken: "D",

Please, take a look to the code we have to live with just because you can tell us ours is bad while we cannot tell you yours can be improved.


List load() {
return [
[
"id": "backend",
"type": "ods",
"data": [
"git": [
"branch": "master",
"commit": "138495888301232315f5455aeb17cc635982ba2c",
"createdExecutionCommit": "",
"baseTag": "ods-generated-v3.0-3.0-0b11-D",
"targetTag": "ods-generated-v3.0-3.0-0b11-D",
]
]
],
[
"id": "frontend",
"type": "ods",
"data": [
"git": [
"branch": "master",
"commit": "3b5a62fbb2307d95360da386408b7f668f0e89ae",
"createdExecutionCommit": "",
"baseTag": "ods-generated-v3.0-3.0-0b11-D",
"targetTag": "ods-generated-v3.0-3.0-0b11-D",
]
]
],
[
"id": "test",
"type": "ods-test",
"data": [
"git": [
"branch": "master",
"commit": "417c5b12c0af838cf0b843feb16ee5b7b1dab4ec",
"createdExecutionCommit": "",
"baseTag": "ods-generated-v3.0-3.0-0b11-D",
"targetTag": "ods-generated-v3.0-3.0-0b11-D",
]
]
]
]
}

}
Binary file modified src/test/resources/expected/ORDGP/DTP-ORDGP-WIP-2022-01-22_23-59-59.pdf
View file
Open in desktop
Binary file not shown.
Binary file modified src/test/resources/expected/ORDGP/SSDS-ORDGP-WIP-2022-01-22_23-59-59.pdf
View file
Open in desktop
Binary file not shown.
Binary file modified src/test/resources/expected/ORDGP/TIP-ORDGP-WIP-2022-01-22_23-59-59.pdf
View file
Open in desktop
Binary file not shown.
Binary file modified src/test/resources/expected/ORDGP/TIR-ORDGP-WIP-2022-01-22_23-59-59.pdf
View file
Open in desktop
Binary file not shown.